Set the queue handle to null before stop & destroy. (#525) 59/208359/1
authorsenthil.gs@samsung.com <senthil.gs@samsung.com>
Thu, 20 Jun 2019 14:02:45 +0000 (19:32 +0530)
committerDoHyun Pyun <dh79.pyun@samsung.com>
Mon, 24 Jun 2019 00:51:14 +0000 (09:51 +0900)
g_sendQueueHandle is set to NULL to prevent new requests from being enqueued.
It avoids a race condition which could occur when adding elements to QueueingThread if it's already stopped.

Race condition scenario:-
Background:-
-> If a thread has called CAQueueingThreadStop(), it signals the condition variable
to wake up the CAQueueingThreadBaseRoutine.

-> After signaling, it waits on the same condition variable
to wait for the completion of CAQueueingThreadBaseRoutine.

-> CAQueueingThreadBaseRoutine will finish its current task and signals the condition variable.

-> CAQueueingThreadStop should wake up upon signal and return back to caller.

Issue:-
-> CAQueueingThreadAddData also signals the condition variable which can wake up the thread that called CAQueueingThreadStop.
-> CAQueueingThreadStop returns back to caller assuming that the CAQueueingThreadBaseRoutine has stopped. But it could still be running.

https://github.sec.samsung.net/RS7-IOTIVITY/IoTivity/pull/525
(cherry picked from eb98e19b49ab4d4475449f99f3a4a53607dbea01)

Change-Id: I5b22c034b30b00fc1ae7f08c37c278c29910345c
Signed-off-by: Senthil Kumar G S <senthil.gs@samsung.com>
Signed-off-by: DoHyun Pyun <dh79.pyun@samsung.com>
resource/csdk/connectivity/src/tcp_adapter/catcpadapter.c

index 0e33b46..ea55c74 100644 (file)
@@ -111,7 +111,7 @@ static void CATCPErrorHandler(const CAEndpoint_t *endpoint, const void *data,
 
 static CAResult_t CATCPInitializeQueueHandles();
 
-static void CATCPDeinitializeQueueHandles();
+static void CATCPDeinitializeQueueHandles(CAQueueingThread_t *queueHandle);
 
 static void CATCPSendDataThread(void *threadData);
 
@@ -152,11 +152,10 @@ CAResult_t CATCPInitializeQueueHandles()
     return CA_STATUS_OK;
 }
 
-void CATCPDeinitializeQueueHandles()
+void CATCPDeinitializeQueueHandles(CAQueueingThread_t *queueHandle)
 {
-    CAQueueingThreadDestroy(g_sendQueueHandle);
-    OICFree(g_sendQueueHandle);
-    g_sendQueueHandle = NULL;
+    CAQueueingThreadDestroy(queueHandle);
+    OICFree(queueHandle);
 }
 
 void CATCPConnectionStateCB(const char *ipAddress, CANetworkStatus_t status)
@@ -665,11 +664,18 @@ CAResult_t CAStopTCP()
 
 #ifndef SINGLE_THREAD
     // Stop send queue thread.
-    if (g_sendQueueHandle && g_sendQueueHandle->threadMutex)
+    if (g_sendQueueHandle != NULL)
     {
-        CAQueueingThreadStop(g_sendQueueHandle);
+        // g_sendQueueHandle is set to NULL to prevent new requests from being enqueued.
+        CAQueueingThread_t *queueHandle = g_sendQueueHandle;
+        g_sendQueueHandle = NULL;
+
+        if (queueHandle->threadMutex)
+        {
+            CAQueueingThreadStop(queueHandle);
+        }
+        CATCPDeinitializeQueueHandles(queueHandle);
     }
-    CATCPDeinitializeQueueHandles();
 #endif
 
     // Close TCP servers and established connections.