Fix for TFS bug 1224526 (possible A/V due race around desired_state_c…

…allback)
Azure · May 19, 2017 · e5e02e9 · e5e02e9
1 parent 1dd70de
commit e5e02e9
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 7 deletions.
diff --git 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
@@ -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));