[x] I added or modified the existing tests to cover the change (we do not allow our test coverage to go down).
If this is a modification that impacts the behavior of a public API
[x] I edited the corresponding document in the devdoc folder and added or modified requirements.
I submitted this PR against the correct branch:
[x] This pull-request is submitted against the main branch.
[x] I have merged the latest main branch prior to submission and re-merged as needed after I took any feedback.
[x] I have squashed my changes into one with a clear description of the change.
Reference/Link to the issue solved with this PR (if any)
Description of the problem
messagesender may fire the send complete callback for a message already destroyed at the calling layer (iothubtransport_amqp_messenger), causing a memory access/read error.
Description of the solution
There are instances where a message will be destroyed at the iothubtransport amqp messeger layer, but be still in flight at the messagesender (uamqp) layer, running at the risk of messagesender invoking the send complete callback and resulting in a memory read error over an address previously freed. Using the async operation feature of uamqp adds an extra layer of safety to cancel the existing send operation, preventing the callback to be fired improperly.
It intends to address the crash in the following stack:
Error: Time:Sat Mar 16 04:04:26 2024 Fil==256952== Invalid read of size 8
==256952== at 0x136B85: on_send_complete_callback (iothubtransport_amqp_messenger.c:862)
==256952== by 0x1798DD: indicate_all_messages_as_error (message_sender.c:703)
==256952== by 0x179D5C: messagesender_close (message_sender.c:848)
==256952== by 0x179BBB: messagesender_destroy (message_sender.c:797)
==256952== by 0x136196: destroy_message_sender (iothubtransport_amqp_messenger.c:596)
==256952== by 0x137987: amqp_messenger_stop (iothubtransport_amqp_messenger.c:1232)
==256952== by 0x1343F7: twin_messenger_stop (iothubtransport_amqp_twin_messenger.c:2000)
==256952== by 0x124B72: internal_device_stop (iothubtransport_amqp_device.c:832)
==256952== by 0x12512C: amqp_device_do_work (iothubtransport_amqp_device.c:956)
==256952== by 0x11F475: IoTHubTransport_AMQP_Common_Device_DoWork (iothubtransport_amqp_common.c:1109)
==256952== by 0x120274: IoTHubTransport_AMQP_Common_DoWork (iothubtransport_amqp_common.c:1470)
==256952== by 0x11CFF7: IoTHubTransportAMQP_DoWork (iothubtransportamqp.c:56)
==256952== by 0x1ABA7A: IoTHubClientCore_LL_DoWork (iothub_client_core_ll.c:2115)
==256952== by 0x1AF1D0: IoTHubDeviceClient_LL_DoWork (iothub_device_client_ll.c:91)
==256952== by 0x11CD3F: main (iothub_ll_client_shared_sample.c:1299)
==256952== Address 0x7b92820 is 48 bytes inside a block of size 64 in arena "client"
Checklist
devdocfolder and added or modified requirements.mainbranch.mainbranch prior to submission and re-merged as needed after I took any feedback.Reference/Link to the issue solved with this PR (if any)
Description of the problem
messagesender may fire the send complete callback for a message already destroyed at the calling layer (iothubtransport_amqp_messenger), causing a memory access/read error.
Description of the solution
There are instances where a message will be destroyed at the iothubtransport amqp messeger layer, but be still in flight at the messagesender (uamqp) layer, running at the risk of messagesender invoking the send complete callback and resulting in a memory read error over an address previously freed. Using the async operation feature of uamqp adds an extra layer of safety to cancel the existing send operation, preventing the callback to be fired improperly. It intends to address the crash in the following stack: