From e5e02e9f0d6b1df0bcd04c768be34ea56a440055 Mon Sep 17 00:00:00 2001 From: ewertons Date: Fri, 19 May 2017 12:15:46 -0700 Subject: [PATCH] Fix for TFS bug 1224526 (possible A/V due race around desired_state_callback) --- iothub_client/src/iothub_client.c | 30 ++++++++++++++----- .../tests/iothubclient_ut/iothubclient_ut.c | 4 +++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/iothub_client/src/iothub_client.c b/iothub_client/src/iothub_client.c index 0dd8eca99f..0ef0f2943f 100644 --- a/iothub_client/src/iothub_client.c +++ b/iothub_client/src/iothub_client.c @@ -414,15 +414,31 @@ static void dispatch_user_callbacks(IOTHUB_CLIENT_INSTANCE* iotHubClientInstance switch (queued_cb->type) { case CALLBACK_TYPE_DEVICE_TWIN: - if (iotHubClientInstance->desired_state_callback) + { + IOTHUB_CLIENT_DEVICE_TWIN_CALLBACK desired_state_callback; + + if (Lock(iotHubClientInstance->LockHandle) != LOCK_OK) + { + LogError("failed locking for dispatch_user_callbacks"); + desired_state_callback = NULL; + } + else { - iotHubClientInstance->desired_state_callback(queued_cb->iothub_callback.dev_twin_cb_info.update_state, queued_cb->iothub_callback.dev_twin_cb_info.payLoad, queued_cb->iothub_callback.dev_twin_cb_info.size, queued_cb->userContextCallback); + desired_state_callback = iotHubClientInstance->desired_state_callback; + (void)Unlock(iotHubClientInstance->LockHandle); } + + if (desired_state_callback) + { + desired_state_callback(queued_cb->iothub_callback.dev_twin_cb_info.update_state, queued_cb->iothub_callback.dev_twin_cb_info.payLoad, queued_cb->iothub_callback.dev_twin_cb_info.size, queued_cb->userContextCallback); + } + if (queued_cb->iothub_callback.dev_twin_cb_info.payLoad) { free(queued_cb->iothub_callback.dev_twin_cb_info.payLoad); } break; + } case CALLBACK_TYPE_EVENT_CONFIRM: if (iotHubClientInstance->event_confirm_callback) { @@ -1406,11 +1422,6 @@ IOTHUB_CLIENT_RESULT IoTHubClient_SetDeviceTwinCallback(IOTHUB_CLIENT_HANDLE iot } else { - if (iotHubClientInstance->created_with_transport_handle == 0) - { - iotHubClientInstance->desired_state_callback = deviceTwinCallback; - } - /*Codes_SRS_IOTHUBCLIENT_10_020: [** `IoTHubClient_SetDeviceTwinCallback` shall be made thread - safe by using the lock created in IoTHubClient_Create. ]*/ if (Lock(iotHubClientInstance->LockHandle) != LOCK_OK) { @@ -1420,6 +1431,11 @@ IOTHUB_CLIENT_RESULT IoTHubClient_SetDeviceTwinCallback(IOTHUB_CLIENT_HANDLE iot } else { + if (iotHubClientInstance->created_with_transport_handle == 0) + { + iotHubClientInstance->desired_state_callback = deviceTwinCallback; + } + if (iotHubClientInstance->created_with_transport_handle != 0 || deviceTwinCallback == NULL) { /*Codes_SRS_IOTHUBCLIENT_10_005: [** `IoTHubClient_LL_SetDeviceTwinCallback` shall call `IoTHubClient_LL_SetDeviceTwinCallback`, while passing the `IoTHubClient_LL handle` created by `IoTHubClient_LL_Create` along with the parameters `reportedStateCallback` and `userContextCallback`. ]*/ diff --git a/iothub_client/tests/iothubclient_ut/iothubclient_ut.c b/iothub_client/tests/iothubclient_ut/iothubclient_ut.c index be066d7a9d..2b0bf8490a 100644 --- a/iothub_client/tests/iothubclient_ut/iothubclient_ut.c +++ b/iothub_client/tests/iothubclient_ut/iothubclient_ut.c @@ -2950,6 +2950,10 @@ TEST_FUNCTION(IoTHubClient_ScheduleWork_Thread_device_twin_succeed) STRICT_EXPECTED_CALL(VECTOR_size(IGNORED_PTR_ARG)).SetReturn(1); STRICT_EXPECTED_CALL(VECTOR_element(IGNORED_PTR_ARG, 0)); + STRICT_EXPECTED_CALL(Lock(IGNORED_PTR_ARG)) + .IgnoreArgument_handle(); + STRICT_EXPECTED_CALL(Unlock(IGNORED_PTR_ARG)) + .IgnoreArgument_handle(); STRICT_EXPECTED_CALL(test_device_twin_callback(DEVICE_TWIN_UPDATE_COMPLETE, NULL, 0, NULL)); STRICT_EXPECTED_CALL(VECTOR_destroy(IGNORED_PTR_ARG));