segfault when reconnecting after disconnect

[larsonmpdx]

I get a segfault if I run my perpetual logging application and simulate a network disconnect followed by a reconnect. on reconnect, it sometimes segfaults with this valgrind output:

==66725== Process terminating with default action of signal 11 (SIGSEGV)
==66725== Access not within mapped region at address 0x0
==66725== at 0x758364: awsiotsdk::mqtt::KeepaliveActionRunner::PerformAction(std::shared_ptrawsiotsdk::NetworkConnection, std::shared_ptrawsiotsdk::ActionData) (Connect.cpp:369)

here's a link to the offending line:
https://github.com/aws/aws-iot-device-sdk-cpp/blob/master/src/mqtt/Connect.cpp#L369

I have no subscriptions so this function called on the previous line will return nullptr when given an empty subscription vector:
https://github.com/aws/aws-iot-device-sdk-cpp/blob/master/src/mqtt/Subscribe.cpp#L57

then nullptr is dereferenced in this bit:
p_subscribe_packet->ToString()

I think that's what's happening. I'm working on a fix and PR later today

[chaurah]

Hi @larsonmpdx,
Thanks for pointing this out. We have already fixed this internally and it will be included in the next release of the SDK. I will close this when the release is out. Please do keep pointing out any other issues you notice.

Rahul

[larsonmpdx]

Rahul,

Thanks. I know you're bound by internal processes and schedules, but I think you should consider a process change to allow hot fix releases for issues like these. This is a bug that affects a wide range of use cases under fairly normal network conditions, and results in a segfault and full program crash. If you have any similar issues being held back I'd appreciate a release ASAP so I can avoid spending time on them myself.

Michael

[chaurah]

Hi @larsonmpdx,
I totally understand. We are actively working on modifying our release process to do exactly what you have asked. We want to make it extremely easy to release small bug fixes and take in pull requests. The turnaround time that we have currently is definitely not helping. In the meantime, for this issue the fix is to simply add a check if(!p_client_state_->subscription_map_.empty()) at line 351 and run lines 351-370 only if it is true. I will definitely add your comments to our internal tracking for the release process changes.

Rahul

[chaurah]

Hi @larsonmpdx,
This issue has been resolved with v1.1 of the SDK. Please reopen if you have further issues.

Rahul