Crash while creating second MqttConnection

[elad-ep]

Hi, we are using SDK version 1.12.3. we have SDK crash in the following scenario:
we try to connect to AWS using MqttConnection:

std::shared_ptr<Aws::Crt::Mqtt::MqttConnection> mConnection;

in proxy configured environments we don't manage to connect in our first attempt. so we query the proxy and try again to connect again - we overwrite the mConnection variable when creating the second new connection:

mConnection = mMqttClient->NewConnection(*mConnectionConfig);

at this point the first connection shared_ptr is released and the SDK is crashing - see second stack below. in addition see the first stack below for additional crash in the same scenario.

logs sent to jkl@amazon.com.

Thanks

00 0323f104 77e2e4e3 ntdll!RtlFailFast2
01 0323f134 77de4a81 ntdll!RtlpHandleInvalidUserCallTarget+0x93
02 0323f1bc 002e7b72 ntdll!LdrpValidateUserCallTargetBitMapRet+0x44
03 0323f1d0 002e9ac4 	  !aws_mem_acquire+0x22 [aws-iot-device-sdk-cpp-v2\crt\aws-crt-cpp\crt\aws-c-common\source\allocator.c @ 120] 
04 0323f1ec 0047d8b6 	  !aws_string_new_from_array+0x34 [aws-iot-device-sdk-cpp-v2\crt\aws-crt-cpp\crt\aws-c-common\source\string.c @ 19] 
05 0323f240 002ef693 	  !aws_mqtt_client_connection_connect+0x106 [aws-iot-device-sdk-cpp-v2\crt\aws-crt-cpp\crt\aws-c-mqtt\source\client.c @ 1373] 
06 0323f2c4 0027876c 	  !Aws::Crt::Mqtt::MqttConnection::Connect+0x143 [aws-iot-device-sdk-cpp-v2\crt\aws-crt-cpp\source\mqtt\mqttclient.cpp @ 405] 




0383eee8 77e2e4e3 ntdll!RtlFailFast2
01 0383ef18 77dfd9f9 ntdll!RtlpHandleInvalidUserCallTarget+0x93
02 0383efa8 77dd2426 ntdll!RtlDispatchException+0x4dac1
03 0383efa8 77de4a2b ntdll!KiUserExceptionDispatcher+0x26
04 0383f41c 00487cf2 ntdll!LdrpValidateUserCallTargetBitMapCheck
05 0383f430 002ef264 	  !aws_ref_count_release+0x22 [aws-iot-device-sdk-cpp-v2\crt\aws-crt-cpp\crt\aws-c-common\source\ref_count.c @ 27] 
06 0383f460 002f0160 	  !Aws::Crt::Mqtt::MqttConnection::~MqttConnection+0x44 [aws-iot-device-sdk-cpp-v2\crt\aws-crt-cpp\source\mqtt\mqttclient.cpp @ 293] 
07 0383f480 00278307 	  !std::_Ref_count_resource<Aws::Crt::Mqtt::MqttConnection *,<lambda_3f159fec8052f10884a89e7bcc65f7ba> >::_Destroy+0x30 [c:\program files (x86)\microsoft visual studio\2017\professional\vc\tools\msvc\14.16.27023\include\memory @ 925] 
08 (Inline) -------- 	  !std::_Ref_count_base::_Decref+0x1e [c:\program files (x86)\microsoft visual studio\2017\professional\vc\tools\msvc\14.16.27023\include\memory @ 847] 
09 (Inline) -------- 	  !std::_Ptr_base<Aws::Crt::Mqtt::MqttConnection>::_Decref+0x20 [c:\program files (x86)\microsoft visual studio\2017\professional\vc\tools\msvc\14.16.27023\include\memory @ 1120] 
0a (Inline) -------- 	  !std::shared_ptr<Aws::Crt::Mqtt::MqttConnection>::{dtor}+0x20 [c:\program files (x86)\microsoft visual studio\2017\professional\vc\tools\msvc\14.16.27023\include\memory @ 1403] 
0b (Inline) -------- 	  !std::shared_ptr<Aws::Crt::Mqtt::MqttConnection>::operator=+0x7a [c:\program files (x86)\microsoft visual studio\2017\professional\vc\tools\msvc\14.16.27023\include\memory @ 1421

[jmklix]

Can you include a minimum code sample showing how you are using the shared_ptr and steps on how to reproduce the error mentioned above?

[elad-ep]

The issue is not frequent and cannot be reproduced on demand, all I can provide are crash stacks above and captured logs...

[elad-ep]

The issue happens consistently during automation testing, but couldn't reproduce manually.
Tested with latest SDK version 1.12.8 -> issue still happens.
Tested with SDK version 1.10.3 -> issues doesn't happen.

[elad-ep]

any update please? have you found anything relevant in the logs?

[anatolyaliev]

Hi,
I am a colleague of the issue starter. The issue is very urgent for us, and prevents from going to production. We'll be happy if someone will handle it in the timely manner.
I have a consistent reproduction of the issue and during the debugging of AWS-IOT SDK v1.12.3 came to the following conclusions:
The symptom :

• AWS-IOT SDK causes memory corruption, and once any other component of the program tries to access the corrupted memory it crashes.
  The reproduction scenario:
• Disable networking
• Create and initialize MqttConnection class . Use Websockets.
• Run MqttConnection.Connect()
• Enable networking
• Destroy the first instance of MqttConnection
• Your test program should be compiled with address sanitizer (on Mac) or any other memory validation tool.
• At this stage the program will crash. Here is the stack
  ==95==ERROR: AddressSanitizer: SEGV on unknown address 0x000138000016 (pc 0x00010ebe0759 bp 0x70000ee9f500 sp 0x70000ee9f500 T17)
  ==95==The signal is caused by a READ memory access.
  #0 0x10ebe0759 in aws_mem_release+0x9 (fpneocommond:x86_64+0x100434759)
  Fully generated clients #1 0x10ec35adf in s_mqtt_client_connection_destroy_final+0x60f (fpneocommond:x86_64+0x100489adf)
  Readme updates for dev-preview #2 0x10ebedae1 in aws_ref_count_release+0x21 (fpneocommond:x86_64+0x100441ae1)
  Tag update #3 0x10ec531be in Aws::Crt::Mqtt::MqttConnection::~MqttConnection()+0x1e (fpneocommond:x86_64+0x1004a71be)

The root cause:

1. aws_mqtt_client_connection.host_name is a pointer, compared with NULL, but neither initialized with NULL in aws_mqtt_client_connection_new, nor set to null while being freed in s_mqtt_client_connection_destroy_final function
2. same for several other pointers (like reconnect_task, etc.) in aws_mqtt_client_connection struct. Please fix the initialization and destroy code.
3. Double free upon failed connection with websockets attempt:
   a) if there is no internet connection while calling aws_mqtt_client_connection_connect it will call aws_mqtt_client_connection_release which will call s_mqtt_client_connection_destroy_final (ref_count is 0) which will destroy the aws_mqtt_client_connection struct itself.
   b) Then the aws_mqtt_client_connection_acquire(connection) will be called, despite the connection was already freed.
   c) So the ref count will be increased on freed memory (BUG)
   d) The MqttConnection.m_UnderlyingConnection is not NULL despite was released by the C interface
   e) If one tries to get rid of such MqttConnection instance the destructor will try to release the freed memory.

I hope this information will help to analyze and fix the issue.

[bretambrose]

Thank you for reporting this. I cannot reproduce it locally (system behavior on "network disconnect" isn't particularly consistent) but using the information you've provided, I can see execution pathways in the connect code that will absolutely result in a decref (from 1 to 0) on the connection before the "pinning" acquire is called in aws_mqtt_client_connection_connect().

I believe the correct fix is to move the acquire up to before the call to s_mqtt_client_connect() (and add a release on an error return from s_mqtt_client_connect()).

With respect to point (2) I don't think there's anything that needs to be done. The connection memory is allocated in a zeroed state, so there should not be a need to assign anything to NULL. When it comes to final destruction where everything is being freed, setting back to NULL isn't much help either. If you end up double deleting the object, a previous set to NULL isn't going to help since the memory could have been modified in the meantime and you will most likely crash on the double free of the original connection memory block anyways.

[anatolyaliev]

Thank you for the quick reply.
I think the fix and the comments are satisfying. I'll check it. When there will be a release tag with this fix?

[bretambrose]

There's a separate tagging issue with the C++ CRT that I need to fix and then I'll do a new SDK release. Hopefully no later than Friday.