Changeset 83584 in vbox for trunk/src/VBox/Devices

Timestamp:

Apr 6, 2020 9:57:12 AM (5 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

136939

Message:

Devices/Serial/DrvTCP: Fix connection handling for the server case and avoid modifying the hTcpSock member from different threads during runtime (was modified from the listen and I/O thread in the server case without using atomics or any protection). Fix timeout handling in the poll handler (wasn't adjusted properly when poll returned for an internal event)

File:

• trunk/src/VBox/Devices/Serial/DrvTCP.cpp (modified) (14 diffs)

trunk/src/VBox/Devices/Serial/DrvTCP.cpp

@@ -81 +81 @@
     /** Writing end of the wakeup pipe. */
     RTPIPE              hPipeWakeW;
-    /** Flag whether the socket is in the pollset. */
-    bool                fTcpSockInPollSet;
     /** Flag whether the send buffer is full nad it is required to wait for more
      * space until there is room again. */
     bool                fXmitBufFull;
 
+    /** Number of connections active. */
+    volatile uint32_t   cConnections;
     /** Thread for listening for new connections. */
     RTTHREAD            ListenThread;
@@ -100 +100 @@
 
 /**
+ * Kicks any possibly polling thread to get informed about changes - extended version
+ * sending additional data along with the wakeup reason.
+ *
+ * @returns VBOx status code.
+ * @param   pThis                   The TCP driver instance.
+ * @param   bReason                 The reason code to handle.
+ * @param   pvData                  The additional to send along with the wakeup reason.
+ * @param   cbData                  Number of bytes to send along.
+ */
+static int drvTcpPollerKickEx(PDRVTCP pThis, uint8_t bReason, const void *pvData, size_t cbData)
+{
+    size_t cbWritten = 0;
+    int rc = RTPipeWriteBlocking(pThis->hPipeWakeW, &bReason, 1, &cbWritten);
+    if (RT_SUCCESS(rc))
+        rc = RTPipeWriteBlocking(pThis->hPipeWakeW, pvData, cbData, &cbWritten);
+    return rc;
+}
+
+
+/**
  * Kicks any possibly polling thread to get informed about changes.
  *
@@ -109 +129 @@
 {
     size_t cbWritten = 0;
-    return RTPipeWrite(pThis->hPipeWakeW, &bReason, 1, &cbWritten);
+    return RTPipeWriteBlocking(pThis->hPipeWakeW, &bReason, 1, &cbWritten);
 }
 
@@ -121 +141 @@
     if (pThis->hTcpSock != NIL_RTSOCKET)
     {
-        if (!pThis->fTcpSockInPollSet)
-        {
-            rc = RTPollSetAddSocket(pThis->hPollSet, pThis->hTcpSock,
-                                    fEvts, DRVTCP_POLLSET_ID_SOCKET);
-            if (RT_SUCCESS(rc))
-            {
-                pThis->fTcpSockInPollSet = true;
-                pThis->fXmitBufFull = false;
-            }
-        }
-        else
-        {
-            /* Always include error event. */
-            fEvts |= RTPOLL_EVT_ERROR;
-            rc = RTPollSetEventsChange(pThis->hPollSet, DRVTCP_POLLSET_ID_SOCKET, fEvts);
-            AssertRC(rc);
-        }
+        Assert(ASMAtomicReadU32(&pThis->cConnections) > 0);
+
+        /* Always include error event. */
+        fEvts |= RTPOLL_EVT_ERROR;
+        rc = RTPollSetEventsChange(pThis->hPollSet, DRVTCP_POLLSET_ID_SOCKET, fEvts);
+        AssertRC(rc);
     }
 
@@ -146 +155 @@
             uint32_t fEvtsRecv = 0;
             uint32_t idHnd = 0;
+            uint64_t tsStartMs = RTTimeMilliTS();
+            RTMSINTERVAL cThisWaitMs = cMillies;
 
             /*
@@ -157 +168 @@
             if (   (fEvts & RTPOLL_EVT_WRITE)
                 && !pThis->fXmitBufFull
-                && pThis->fTcpSockInPollSet)
-                cMillies = 0;
-
-            rc = RTPoll(pThis->hPollSet, cMillies, &fEvtsRecv, &idHnd);
+                && pThis->hTcpSock != NIL_RTSOCKET)
+                cThisWaitMs = 0;
+
+            rc = RTPoll(pThis->hPollSet, cThisWaitMs, &fEvtsRecv, &idHnd);
+
+            /* Adjust remaining time to wait. */
+            uint64_t tsPollSpanMs = RTTimeMilliTS() - tsStartMs;
+            cMillies -= RT_MIN(cMillies, tsPollSpanMs);
             if (RT_SUCCESS(rc))
             {
@@ -175 +190 @@
                     else if (bReason == DRVTCP_WAKEUP_REASON_NEW_CONNECTION)
                     {
-                        Assert(!pThis->fTcpSockInPollSet);
-                        rc = RTPollSetAddSocket(pThis->hPollSet, pThis->hTcpSock,
+                        Assert(pThis->hTcpSock == NIL_RTSOCKET);
+
+                        /* Read the socket handle. */
+                        RTSOCKET hTcpSockNew = NIL_RTSOCKET;
+                        rc = RTPipeReadBlocking(pThis->hPipeWakeR, &hTcpSockNew, sizeof(hTcpSockNew), NULL);
+                        AssertRC(rc);
+
+                        /* Always include error event. */
+                        fEvts |= RTPOLL_EVT_ERROR;
+                        rc = RTPollSetAddSocket(pThis->hPollSet, hTcpSockNew,
                                                 fEvts, DRVTCP_POLLSET_ID_SOCKET);
                         if (RT_SUCCESS(rc))
-                            pThis->fTcpSockInPollSet = true;
+                            pThis->hTcpSock = hTcpSockNew;
                     }
                     else
@@ -199 +222 @@
                             RTSocketClose(pThis->hTcpSock);
                         pThis->hTcpSock = NIL_RTSOCKET;
-                        pThis->fTcpSockInPollSet = false;
+                        ASMAtomicDecU32(&pThis->cConnections);
                         /* Continue with polling. */
                     }
@@ -261 +284 @@
                     RTSocketClose(pThis->hTcpSock);
                 pThis->hTcpSock = NIL_RTSOCKET;
-                pThis->fTcpSockInPollSet = false;
                 rc = VINF_SUCCESS;
             }
@@ -338 +360 @@
         if (RT_SUCCESS(rc))
         {
-            if (pThis->hTcpSock != NIL_RTSOCKET)
+            if (ASMAtomicReadU32(&pThis->cConnections) > 0)
             {
                 LogRel(("DrvTCP%d: only single connection supported\n", pThis->pDrvIns->iInstance));
@@ -345 +367 @@
             else
             {
-                pThis->hTcpSock = hTcpSockNew;
+                ASMAtomicIncU32(&pThis->cConnections);
+
                 /* Inform the poller about the new socket. */
-                drvTcpPollerKick(pThis, DRVTCP_WAKEUP_REASON_NEW_CONNECTION);
+                drvTcpPollerKickEx(pThis, DRVTCP_WAKEUP_REASON_NEW_CONNECTION, &hTcpSockNew, sizeof(hTcpSockNew));
             }
         }
@@ -485 +508 @@
     pThis->pszLocation                  = NULL;
     pThis->fIsServer                    = false;
+    pThis->fXmitBufFull                 = false;
+    pThis->cConnections                 = 0;
 
     pThis->hTcpServ                     = NULL;
@@ -492 +517 @@
     pThis->hPipeWakeR                   = NIL_RTPIPE;
     pThis->hPipeWakeW                   = NIL_RTPIPE;
-    pThis->fTcpSockInPollSet            = false;
 
     pThis->ListenThread                 = NIL_RTTHREAD;
@@ -591 +615 @@
                                        pDrvIns->iInstance, pThis->pszLocation);
 
-        pThis->fTcpSockInPollSet = true;
+        ASMAtomicIncU32(&pThis->cConnections);
     }