check socket status before adding to select() list. When TurnConnection goes through failed state, go to cleanup state to cleanup the socket then back to failed state.

Author: chehefen

Diff for: src/source/Ice/ConnectionListener.c

@@ -136,9 +136,7 @@ STATUS connectionListenerRemoveConnection(PConnectionListener pConnectionListene
     while (!hasConnection && pCurNode != NULL) {
         pCurrSocketConnection = (PSocketConnection) pCurNode->data;
         pCurNode = pCurNode->pNext;
-        if (pCurrSocketConnection == pSocketConnection) {
-            hasConnection = TRUE;
-        }
+        hasConnection = pCurrSocketConnection == pSocketConnection;
     }
 
     /* If connection is not found then return early */
@@ -302,8 +300,12 @@ PVOID connectionListenerReceiveDataRoutine(PVOID arg)
 
         for (i = 0; i < socketCount; ++i) {
             pSocketConnection = socketList[i];
-            FD_SET(pSocketConnection->localSocket, &rfds);
-            nfds = MAX(nfds, pSocketConnection->localSocket);
+            if (socketConnectionIsClosed(pSocketConnection)) {
+                updateSocketList = TRUE;
+            } else {
+                FD_SET(pSocketConnection->localSocket, &rfds);
+                nfds = MAX(nfds, pSocketConnection->localSocket);
+            }
         }
 
         nfds++;

Diff for: src/source/Ice/TurnConnection.c

@@ -218,7 +218,7 @@ STATUS turnConnectionHandleStun(PTurnConnection pTurnConnection, PBYTE pBuffer,
 
     switch (stunPacketType) {
         case STUN_PACKET_TYPE_ALLOCATE_SUCCESS_RESPONSE:
-            /* If shutdown has been initiated, ignore the alloaction response */
+            /* If shutdown has been initiated, ignore the allocation response */
             CHK(!ATOMIC_LOAD(&pTurnConnection->stopTurnConnection), retStatus);
             CHK_STATUS(deserializeStunPacket(pBuffer, bufferLen, pTurnConnection->longTermKey, KVS_MD5_DIGEST_LENGTH, &pStunPacket));
             CHK_STATUS(getStunAttribute(pStunPacket, STUN_ATTRIBUTE_TYPE_XOR_RELAYED_ADDRESS, &pStunAttr));
@@ -1058,7 +1058,7 @@ STATUS turnConnectionStepState(PTurnConnection pTurnConnection)
 
                 CHK_STATUS(turnConnectionFreePreAllocatedPackets(pTurnConnection));
                 CHK_STATUS(socketConnectionClosed(pTurnConnection->pControlChannel));
-                pTurnConnection->state = TURN_STATE_NEW;
+                pTurnConnection->state = STATUS_SUCCEEDED(pTurnConnection->errorStatus) ? TURN_STATE_NEW : TURN_STATE_FAILED;
                 ATOMIC_STORE_BOOL(&pTurnConnection->shutdownComplete, TRUE);
             }
 
@@ -1068,6 +1068,11 @@ STATUS turnConnectionStepState(PTurnConnection pTurnConnection)
             DLOGW("TurnConnection in TURN_STATE_FAILED due to 0x%08x. Aborting TurnConnection", pTurnConnection->errorStatus);
             /* Since we are aborting, not gonna do cleanup */
             ATOMIC_STORE_BOOL(&pTurnConnection->hasAllocation, FALSE);
+            /* If we haven't done cleanup, go to cleanup state which will do the cleanup then go to failed state again. */
+            if (!ATOMIC_LOAD_BOOL(&pTurnConnection->shutdownComplete)) {
+                pTurnConnection->state = TURN_STATE_CLEAN_UP;
+                pTurnConnection->stateTimeoutTime = currentTime + DEFAULT_TURN_CLEAN_UP_TIMEOUT;
+            }
 
             break;
 
@@ -1289,7 +1294,7 @@ STATUS turnConnectionTimerCallback(UINT32 timerId, UINT64 currentTime, UINT64 cu
             break;
 
         case TURN_STATE_FAILED:
-            stopScheduling = TRUE;
+            stopScheduling = ATOMIC_LOAD_BOOL(&pTurnConnection->shutdownComplete);
             break;
 
         default:

Diff for: tst/TurnConnectionFunctionalityTest.cpp

@@ -294,7 +294,9 @@ TEST_F(TurnConnectionFunctionalityTest, turnConnectionShutdownWithAllocationRemo
     BOOL doneAllocate = FALSE;
     UINT64 shutdownTimeout;
     UINT64 doneAllocateTimeout = GETTIME() + 10 * HUNDREDS_OF_NANOS_IN_A_SECOND;
-    PSocketConnection pTurnSocketConnection = NULL;
+    PSocketConnection pTurnSocketConnection = NULL, pCurrSocketConnection = NULL;
+    PDoubleListNode pCurNode = NULL;
+    BOOL connectionRemovedFromListener = TRUE;
 
     initializeTestTurnConnection();
     pTurnSocketConnection = pTurnConnection->pControlChannel;
@@ -326,6 +328,20 @@ TEST_F(TurnConnectionFunctionalityTest, turnConnectionShutdownWithAllocationRemo
     EXPECT_TRUE(ATOMIC_LOAD_BOOL(&pTurnSocketConnection->connectionClosed));
     MUTEX_UNLOCK(pTurnConnection->lock);
 
+    THREAD_SLEEP(2 * HUNDREDS_OF_NANOS_IN_A_SECOND);
+
+    MUTEX_LOCK(pConnectionListener->lock);
+    EXPECT_EQ(STATUS_SUCCESS, doubleListGetHeadNode(pConnectionListener->connectionList, &pCurNode));
+    while (pCurNode != NULL && connectionRemovedFromListener) {
+        pCurrSocketConnection = (PSocketConnection) pCurNode->data;
+        pCurNode = pCurNode->pNext;
+        connectionRemovedFromListener = pCurrSocketConnection != pTurnSocketConnection;
+    }
+    MUTEX_UNLOCK(pConnectionListener->lock);
+
+    /* make sure that pTurnSocketConnection has been removed from connection listener's list */
+    EXPECT_TRUE(connectionRemovedFromListener == TRUE);
+
     freeTestTurnConnection();
 }
 
@@ -338,7 +354,9 @@ TEST_F(TurnConnectionFunctionalityTest, turnConnectionShutdownWithoutAllocationR
     BOOL atGetCredential = FALSE;
     UINT64 shutdownTimeout;
     UINT64 atGetCredentialTimeout = GETTIME() + 10 * HUNDREDS_OF_NANOS_IN_A_SECOND;
-    PSocketConnection pTurnSocketConnection = NULL;
+    PSocketConnection pTurnSocketConnection = NULL, pCurrSocketConnection = NULL;
+    PDoubleListNode pCurNode = NULL;
+    BOOL connectionRemovedFromListener = TRUE;
 
     initializeTestTurnConnection();
     pTurnSocketConnection = pTurnConnection->pControlChannel;
@@ -369,6 +387,82 @@ TEST_F(TurnConnectionFunctionalityTest, turnConnectionShutdownWithoutAllocationR
     EXPECT_TRUE(ATOMIC_LOAD_BOOL(&pTurnSocketConnection->connectionClosed));
     MUTEX_UNLOCK(pTurnConnection->lock);
 
+    THREAD_SLEEP(2 * HUNDREDS_OF_NANOS_IN_A_SECOND);
+
+    MUTEX_LOCK(pConnectionListener->lock);
+    EXPECT_EQ(STATUS_SUCCESS, doubleListGetHeadNode(pConnectionListener->connectionList, &pCurNode));
+    while (pCurNode != NULL && connectionRemovedFromListener) {
+        pCurrSocketConnection = (PSocketConnection) pCurNode->data;
+        pCurNode = pCurNode->pNext;
+        connectionRemovedFromListener = pCurrSocketConnection != pTurnSocketConnection;
+    }
+    MUTEX_UNLOCK(pConnectionListener->lock);
+
+    /* make sure that pTurnSocketConnection has been removed from connection listener's list */
+    EXPECT_TRUE(connectionRemovedFromListener == TRUE);
+
+    freeTestTurnConnection();
+}
+
+TEST_F(TurnConnectionFunctionalityTest, turnConnectionShutdownAfterFailure)
+{
+    if (!mAccessKeyIdSet) {
+        return;
+    }
+
+    BOOL atGetCredential = FALSE;
+    UINT64 shutdownTimeout;
+    UINT64 atGetCredentialTimeout = GETTIME() + 10 * HUNDREDS_OF_NANOS_IN_A_SECOND;
+    PSocketConnection pTurnSocketConnection = NULL, pCurrSocketConnection = NULL;
+    PDoubleListNode pCurNode = NULL;
+    BOOL connectionRemovedFromListener = TRUE;
+
+    initializeTestTurnConnection();
+    pTurnSocketConnection = pTurnConnection->pControlChannel;
+
+    EXPECT_EQ(STATUS_SUCCESS, turnConnectionStart(pTurnConnection));
+
+    // wait until get credential state
+    while (!atGetCredential && GETTIME() < atGetCredentialTimeout) {
+        THREAD_SLEEP(10 * HUNDREDS_OF_NANOS_IN_A_MILLISECOND);
+        MUTEX_LOCK(pTurnConnection->lock);
+        if (pTurnConnection->state == TURN_STATE_GET_CREDENTIALS) {
+            atGetCredential = TRUE;
+        }
+        MUTEX_UNLOCK(pTurnConnection->lock);
+    }
+
+    MUTEX_LOCK(pTurnConnection->lock);
+    pTurnConnection->state = TURN_STATE_FAILED;
+    pTurnConnection->errorStatus = STATUS_INVALID_OPERATION;
+    MUTEX_UNLOCK(pTurnConnection->lock);
+
+    shutdownTimeout = GETTIME() + 5 * HUNDREDS_OF_NANOS_IN_A_SECOND;
+    while (!turnConnectionIsShutdownComplete(pTurnConnection) && GETTIME() < shutdownTimeout) {
+        THREAD_SLEEP(HUNDREDS_OF_NANOS_IN_A_SECOND);
+    }
+
+    EXPECT_TRUE(turnConnectionIsShutdownComplete(pTurnConnection));
+
+    MUTEX_LOCK(pTurnConnection->lock);
+    EXPECT_TRUE(ATOMIC_LOAD_BOOL(&pTurnSocketConnection->connectionClosed));
+    MUTEX_UNLOCK(pTurnConnection->lock);
+
+    /* select in connection timeout every 1s */
+    THREAD_SLEEP(3 * HUNDREDS_OF_NANOS_IN_A_SECOND);
+
+    MUTEX_LOCK(pConnectionListener->lock);
+    EXPECT_EQ(STATUS_SUCCESS, doubleListGetHeadNode(pConnectionListener->connectionList, &pCurNode));
+    while (pCurNode != NULL && connectionRemovedFromListener) {
+        pCurrSocketConnection = (PSocketConnection) pCurNode->data;
+        pCurNode = pCurNode->pNext;
+        connectionRemovedFromListener = pCurrSocketConnection != pTurnSocketConnection;
+    }
+    MUTEX_UNLOCK(pConnectionListener->lock);
+
+    /* make sure that pTurnSocketConnection has been removed from connection listener's list */
+    EXPECT_TRUE(connectionRemovedFromListener == TRUE);
+
     freeTestTurnConnection();
 }