Changeset 83595 in vbox for trunk/src/VBox/Additions/common

Timestamp:

Apr 6, 2020 6:21:34 PM (5 years ago)

Author:

vboxsync

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

136989

Message:

Guest Control/VBoxService: Cleaned up VGSvcGstCtrlSessionClose(), a bit of renaming, added vgsvcGstCtrlSessionProcessRemoveInternal(). bugref:9320

File:

• trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControlSession.cpp (modified) (8 diffs)

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControlSession.cpp

@@ -63 +63 @@
 
 static int vgsvcGstCtrlSessionCleanupProcesses(const PVBOXSERVICECTRLSESSION pSession);
+static int vgsvcGstCtrlSessionProcessRemoveInternal(PVBOXSERVICECTRLSESSION pSession, PVBOXSERVICECTRLPROCESS pProcess);
 
 
@@ -90 +91 @@
 
 
-static int vgsvcGstCtrlSessionFileDestroy(PVBOXSERVICECTRLFILE pFile)
+static int vgsvcGstCtrlSessionFileFree(PVBOXSERVICECTRLFILE pFile)
 {
     AssertPtrReturn(pFile, VERR_INVALID_POINTER);
@@ -425 +426 @@
         {
             VGSvcVerbose(2, "[File %s] Closing (handle=%RU32)\n", pFile ? pFile->szName : "<Not found>", uHandle);
-            rc = vgsvcGstCtrlSessionFileDestroy(pFile);
+            rc = vgsvcGstCtrlSessionFileFree(pFile);
         }
         else
@@ -1890 +1891 @@
 
         /* Wait for all active threads to shutdown and destroy the active thread list. */
-        pProcess = RTListGetFirst(&pSession->lstProcesses, VBOXSERVICECTRLPROCESS, Node);
-        while (pProcess)
-        {
-            PVBOXSERVICECTRLPROCESS pNext = RTListNodeGetNext(&pProcess->Node, VBOXSERVICECTRLPROCESS, Node);
-            bool fLast = RTListNodeIsLast(&pSession->lstProcesses, &pProcess->Node);
-
-            int rc2 = RTCritSectLeave(&pSession->CritSect);
-            AssertRC(rc2);
-
-            rc2 = VGSvcGstCtrlProcessWait(pProcess, 30 * 1000 /* Wait 30 seconds max. */, NULL /* rc */);
-
-            int rc3 = RTCritSectEnter(&pSession->CritSect);
+        PVBOXSERVICECTRLPROCESS pProcessNext;
+        RTListForEachSafe(&pSession->lstProcesses, pProcess, pProcessNext, VBOXSERVICECTRLPROCESS, Node)
+        {
+            int rc3 = RTCritSectLeave(&pSession->CritSect);
             AssertRC(rc3);
 
+            int rc2 = VGSvcGstCtrlProcessWait(pProcess, 30 * 1000 /* Wait 30 seconds max. */, NULL /* rc */);
+
+            rc3 = RTCritSectEnter(&pSession->CritSect);
+            AssertRC(rc3);
+
             if (RT_SUCCESS(rc2))
-                VGSvcGstCtrlProcessFree(pProcess);
-
-            if (fLast)
-                break;
-
-            pProcess = pNext;
-        }
-
-#ifdef DEBUG
-        pProcess = RTListGetFirst(&pSession->lstProcesses, VBOXSERVICECTRLPROCESS, Node);
-        while (pProcess)
-        {
-            PVBOXSERVICECTRLPROCESS pNext = RTListNodeGetNext(&pProcess->Node, VBOXSERVICECTRLPROCESS, Node);
-            bool fLast = RTListNodeIsLast(&pSession->lstProcesses, &pProcess->Node);
-
-            VGSvcVerbose(1, "Process %p (PID %RU32) still in list\n", pProcess, pProcess->uPID);
-            if (fLast)
-                break;
-
-            pProcess = pNext;
-        }
-#endif
+            {
+                rc2 = vgsvcGstCtrlSessionProcessRemoveInternal(pSession, pProcess);
+                if (RT_SUCCESS(rc2))
+                {
+                    VGSvcGstCtrlProcessFree(pProcess);
+                    pProcess = NULL;
+                }
+            }
+        }
+
+        AssertMsg(pSession->cProcesses == 0,
+                  ("Session process list still contains %RU32 when it should not\n", pSession->cProcesses));
         AssertMsg(RTListIsEmpty(&pSession->lstProcesses),
-                  ("Guest process list still contains entries when it should not\n"));
+                  ("Session process list is not empty when it should\n"));
 
         /*
@@ -1935 +1923 @@
         VGSvcVerbose(0, "Closing all guest files ...\n");
 
-        PVBOXSERVICECTRLFILE pFile;
-        pFile = RTListGetFirst(&pSession->lstFiles, VBOXSERVICECTRLFILE, Node);
-        while (pFile)
-        {
-            PVBOXSERVICECTRLFILE pNext = RTListNodeGetNext(&pFile->Node, VBOXSERVICECTRLFILE, Node);
-            bool fLast = RTListNodeIsLast(&pSession->lstFiles, &pFile->Node);
-
-            int rc2 = vgsvcGstCtrlSessionFileDestroy(pFile);
+        PVBOXSERVICECTRLFILE pFile, pFileNext;
+        RTListForEachSafe(&pSession->lstFiles, pFile, pFileNext, VBOXSERVICECTRLFILE, Node)
+        {
+            int rc2 = vgsvcGstCtrlSessionFileFree(pFile);
             if (RT_FAILURE(rc2))
             {
@@ -1951 +1935 @@
             }
 
-            if (fLast)
-                break;
-
-            pFile = pNext;
-        }
-
-        AssertMsg(RTListIsEmpty(&pSession->lstFiles), ("Guest file list still contains entries when it should not\n"));
+            pFile = NULL; /* To make it obvious. */
+        }
+
+        AssertMsg(pSession->cFiles == 0,
+                  ("Session file list still contains %RU32 when it should not\n", pSession->cFiles));
+        AssertMsg(RTListIsEmpty(&pSession->lstFiles),
+                  ("Session file list is not empty when it should\n"));
 
         int rc2 = RTCritSectLeave(&pSession->CritSect);
@@ -2033 +2017 @@
 }
 
+/**
+ * Removes a guest process from a session's process list.
+ * Internal version, does not do locking.
+ *
+ * @return  VBox status code.
+ * @param   pSession                Guest session to remove process from.
+ * @param   pProcess                Guest process to remove.
+ */
+static int vgsvcGstCtrlSessionProcessRemoveInternal(PVBOXSERVICECTRLSESSION pSession, PVBOXSERVICECTRLPROCESS pProcess)
+{
+    VGSvcVerbose(3, "Removing process (PID %RU32) from session ID=%RU32\n", pProcess->uPID, pSession->StartupInfo.uSessionID);
+    AssertReturn(pProcess->cRefs == 0, VERR_WRONG_ORDER);
+
+    RTListNodeRemove(&pProcess->Node);
+
+    AssertReturn(pSession->cProcesses, VERR_WRONG_ORDER);
+    pSession->cProcesses--;
+    VGSvcVerbose(3, "Now session ID=%RU32 has %RU32 processes total\n",
+                 pSession->StartupInfo.uSessionID, pSession->cProcesses);
+
+    return VINF_SUCCESS;
+}
 
 /**
@@ -2049 +2055 @@
     if (RT_SUCCESS(rc))
     {
-        VGSvcVerbose(3, "Removing process (PID %RU32) from session ID=%RU32\n", pProcess->uPID, pSession->StartupInfo.uSessionID);
-        AssertReturn(pProcess->cRefs == 0, VERR_WRONG_ORDER);
-
-        RTListNodeRemove(&pProcess->Node);
-
-        AssertReturn(pSession->cProcesses, VERR_WRONG_ORDER);
-        pSession->cProcesses--;
-        VGSvcVerbose(3, "Now session ID=%RU32 has %RU32 processes total\n",
-                     pSession->StartupInfo.uSessionID, pSession->cProcesses);
+        rc = vgsvcGstCtrlSessionProcessRemoveInternal(pSession, pProcess);
 
         int rc2 = RTCritSectLeave(&pSession->CritSect);