Changeset 91999 in vbox for trunk/src

Timestamp:

Oct 22, 2021 11:43:28 AM (3 years ago)

Author:

vboxsync

Message:

Audio/Validation Kit: Fixed memory leaks found by ASAN, added + renamed some functions to streamline client destruction. bugref:10008

Location:

trunk/src/VBox/Devices/Audio

Files:

• AudioTestService.cpp (modified) (6 diffs)
• AudioTestServiceClient.cpp (modified) (2 diffs)
• AudioTestServiceInternal.h (modified) (1 diff)
• AudioTestServiceTcp.cpp (modified) (8 diffs)
• testcase/tstAudioTestService.cpp (modified) (1 diff)

trunk/src/VBox/Devices/Audio/AudioTestService.cpp

@@ -819 +819 @@
 }
 
+/**
+ * Disconnects a client.
+ *
+ * @returns VBox status code.
+ * @param   pThis               The ATS instance.
+ * @param   pInst               The ATS client to disconnect.
+ */
 static int atsClientDisconnect(PATSSERVER pThis, PATSCLIENTINST pInst)
 {
@@ -824 +831 @@
 
     pInst->enmState = ATSCLIENTSTATE_DESTROYING;
-    pThis->pTransport->pfnNotifyBye(pThis->pTransportInst, pInst->pTransportClient);
+
+    if (   pThis->pTransportInst
+        && pInst->pTransportClient)
+    {
+        if (pThis->pTransport->pfnNotifyBye)
+            pThis->pTransport->pfnNotifyBye(pThis->pTransportInst, pInst->pTransportClient);
+
+        pThis->pTransport->pfnDisconnect(pThis->pTransportInst, pInst->pTransportClient);
+        /* Pointer is now invalid due to the call above. */
+        pInst->pTransportClient = NULL;
+    }
 
     return VINF_SUCCESS;
@@ -837 +854 @@
 static void atsClientFree(PATSCLIENTINST pInst)
 {
+    if (!pInst)
+        return;
+
+    /* Make sure that there is no transport client associated with it anymore. */
+    AssertReturnVoid(pInst->enmState = ATSCLIENTSTATE_DESTROYING);
+    AssertReturnVoid(pInst->pTransportClient == NULL);
+
     if (pInst->pszHostname)
         RTStrFree(pInst->pszHostname);
     RTMemFree(pInst);
+    pInst = NULL;
 }
 
@@ -959 +984 @@
     }
 
+    if (papInsts)
+    {
+        for (size_t i = 0; i < cClientsMax; i++)
+            RTMemFree(papInsts[i]);
+        RTMemFree(papInsts);
+    }
+
     return rc;
 }
@@ -992 +1024 @@
 
         /*
-         * New connection, create new client structure and spin of
+         * New connection, create new client structure and spin off
          * the request handling thread.
          */
@@ -1236 +1268 @@
     {
         RTListNodeRemove(&pIt->NdLst);
-
-        RTMemFree(pIt);
-        pIt = NULL;
+        atsClientDisconnect(pThis, pIt);
+        atsClientFree(pIt);
     }
 

trunk/src/VBox/Devices/Audio/AudioTestServiceClient.cpp

@@ -321 +321 @@
     if (pClient->pTransport)
     {
-        pClient->pTransport->pfnStop(pClient->pTransportInst);
         pClient->pTransport->pfnDestroy(pClient->pTransportInst);
-        pClient->pTransport = NULL;
+        pClient->pTransportInst = NULL; /* Invalidate pointer. */
     }
 }
@@ -579 +578 @@
         if (pClient->pTransport->pfnNotifyBye)
             pClient->pTransport->pfnNotifyBye(pClient->pTransportInst, pClient->pTransportClient);
+
+        pClient->pTransport->pfnDisconnect(pClient->pTransportInst, pClient->pTransportClient);
+        pClient->pTransportClient = NULL;
+
+        pClient->pTransport->pfnStop(pClient->pTransportInst);
     }
 

trunk/src/VBox/Devices/Audio/AudioTestServiceInternal.h

@@ -131 +131 @@
      *                              or \c false if not (regular client). Optional and can be NULL.
      * @param   ppClientNew         Where to return the allocated client on success.
+     *                              Must be destroyed with pfnDisconnect() when done.
      */
     DECLR3CALLBACKMEMBER(int, pfnWaitForConnect, (PATSTRANSPORTINST pThis, RTMSINTERVAL msTimeout, bool *pfFromServer, PPATSTRANSPORTCLIENT ppClientNew));
+
+    /**
+     * Disconnects a client and frees up its resources.
+     *
+     * @param   pThis               The transport instance.
+     * @param   pClient             Client to disconnect.
+     *                              The pointer will be invalid after calling.
+     */
+    DECLR3CALLBACKMEMBER(void, pfnDisconnect, (PATSTRANSPORTINST pThis, PATSTRANSPORTCLIENT pClient));
 
     /**

trunk/src/VBox/Devices/Audio/AudioTestServiceTcp.cpp

@@ -124 +124 @@
 /**
  * Disconnects the current client and frees all stashed data.
+ *
+ * @param   pThis           Transport instance.
+ * @param   pClient         Client to disconnect.
  */
 static void atsTcpDisconnectClient(PATSTRANSPORTINST pThis, PATSTRANSPORTCLIENT pClient)
 {
     RT_NOREF(pThis);
+
+    LogRelFlowFunc(("pClient=%RTsock\n", pClient->hTcpClient));
 
     if (pClient->hTcpClient != NIL_RTSOCKET)
@@ -147 +152 @@
         pClient->pbTcpStashed = NULL;
     }
+}
+
+/**
+ * Free's a client.
+ *
+ * @param   pThis           Transport instance.
+ * @param   pClient         Client to free.
+ *                          The pointer will be invalid after calling.
+ */
+static void atsTcpFreeClient(PATSTRANSPORTINST pThis, PATSTRANSPORTCLIENT pClient)
+{
+    if (!pClient)
+        return;
+
+    /* Make sure to disconnect first. */
+    atsTcpDisconnectClient(pThis, pClient);
+
+    RTMemFree(pClient);
+    pClient = NULL;
 }
 
@@ -432 +456 @@
         if (pClient)
         {
-            RTTcpServerDisconnectClient2(pClient->hTcpClient);
-
-            RTMemFree(pClient);
+            atsTcpFreeClient(pThis, pClient);
             pClient = NULL;
         }
@@ -495 +517 @@
     while (rc == VERR_INTERRUPTED);
 
+    LogRelFlowFunc(("pClient=%RTsock, rc=%Rrc\n", pClient->hTcpClient, rc));
+
     /*
      * Disconnect the client.
      */
-    LogRelFlowFunc(("atsTcpDisconnectClient(%RTsock) (RTTcpWrite rc=%Rrc)\n", pClient->hTcpClient, rc));
     atsTcpDisconnectClient(pThis, pClient);
 }
@@ -701 +724 @@
 
 /**
+ * @interface_method_impl{ATSTRANSPORT,pfnDisconnect}
+ */
+static DECLCALLBACK(void) atsTcpDisconnect(PATSTRANSPORTINST pThis, PATSTRANSPORTCLIENT pClient)
+{
+    atsTcpFreeClient(pThis, pClient);
+}
+
+/**
  * @interface_method_impl{ATSTRANSPORT,pfnPollIn}
  */
@@ -769 +800 @@
 static DECLCALLBACK(int) atsTcpDestroy(PATSTRANSPORTINST pThis)
 {
+    /* Stop things first. */
+    atsTcpStop(pThis);
+
     /* Finally, clean up the critical section. */
     if (RTCritSectIsInitialized(&pThis->CritSect))
@@ -774 +808 @@
 
     RTMemFree(pThis);
+    pThis = NULL;
 
     return VINF_SUCCESS;
@@ -909 +944 @@
     /* .pfnStop           = */ atsTcpStop,
     /* .pfnWaitForConnect = */ atsTcpWaitForConnect,
+    /* .pfnDisconnect     = */ atsTcpDisconnect,
     /* .pfnPollIn         = */ atsTcpPollIn,
     /* .pfnPollSetAdd     = */ atsTcpPollSetAdd,

trunk/src/VBox/Devices/Audio/testcase/tstAudioTestService.cpp

@@ -154 +154 @@
     RTTEST_CHECK_RC_OK(hTest, rc);
 
+    AudioTestSvcClientDestroy(&Client);
+
     rc = AudioTestSvcStop(&Srv);
     RTTEST_CHECK_RC_OK(hTest, rc);