Changeset 70390 in vbox for trunk/src/VBox/Additions

Timestamp:

Dec 29, 2017 4:54:17 PM (7 years ago)

Author:

vboxsync

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

119995

Message:

VBoxServiceControlProcess.cpp: Don't try send async requests to a thread that already quit because the process didn't even start (like file not found). Fixed confused documentation on hNotificationPipeW, leaving confused (but harmless) cleanup code following return from vgsvcGstCtrlProcessProcLoop().

Location:

trunk/src/VBox/Additions/common/VBoxService

Files:

• VBoxServiceControl.h (modified) (2 diffs)
• VBoxServiceControlProcess.cpp (modified) (11 diffs)
• VBoxServiceToolBox.h (modified) (1 diff)

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceControl.h

@@ -244 +244 @@
       * needs (or is asked) to shutdown. */
     bool volatile                   fShutdown;
-    /** Whether the guest process thread was stopped
-     *  or not. */
+    /** Whether the guest process thread was stopped or not. */
     bool volatile                   fStopped;
-    /** Whether the guest process thread was started
-     *  or not. */
+    /** Whether the guest process thread was started or not. */
     bool                            fStarted;
     /** Client ID. */
@@ -276 +274 @@
      *  guest process' stdout.*/
     RTPIPE                          hPipeStdErrR;
-    /** The notification pipe associated with this guest process.
+
+    /** The write end of the notification pipe that is used to poke the thread
+     * monitoring the process.
      *  This is NIL_RTPIPE for output pipes. */
     RTPIPE                          hNotificationPipeW;
-    /** The other end of hNotificationPipeW. */
+    /** The other end of hNotificationPipeW, read by vgsvcGstCtrlProcessProcLoop(). */
     RTPIPE                          hNotificationPipeR;
 } VBOXSERVICECTRLPROCESS;

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

@@ -58 +58 @@
 static DECLCALLBACK(int)    vgsvcGstCtrlProcessOnOutput(PVBOXSERVICECTRLPROCESS pThis, const PVBGLR3GUESTCTRLCMDCTX pHostCtx,
                                                         uint32_t uHandle, uint32_t cbToRead, uint32_t uFlags);
-static DECLCALLBACK(int)    vgsvcGstCtrlProcessOnTerm(PVBOXSERVICECTRLPROCESS pThis);
 
 
@@ -254 +253 @@
             if (RT_SUCCESS(rc))
             {
+                pProcess->Thread = NIL_RTTHREAD;
                 VGSvcVerbose(3, "[PID %RU32]: Thread shutdown complete, thread rc=%Rrc\n", pProcess->uPID, rcThread);
                 if (prc)
@@ -1485 +1485 @@
      */
     char **papszArgs;
-    uint32_t uNumArgs = 0; /* Initialize in case of RTGetOptArgvFromString() is failing ... */
-    rc = RTGetOptArgvFromString(&papszArgs, (int*)&uNumArgs,
-                                (pProcess->StartupInfo.uNumArgs > 0) ? pProcess->StartupInfo.szArgs : "",
+    int cArgs = 0; /* Initialize in case of RTGetOptArgvFromString() is failing ... */
+    rc = RTGetOptArgvFromString(&papszArgs, &cArgs,
+                                pProcess->StartupInfo.uNumArgs > 0 ? pProcess->StartupInfo.szArgs : "",
                                 RTGETOPTARGV_CNV_QUOTE_BOURNE_SH, NULL);
     /* Did we get the same result? */
-    Assert(pProcess->StartupInfo.uNumArgs == uNumArgs + 1 /* Take argv[0] into account */);
+    Assert((int)pProcess->StartupInfo.uNumArgs == cArgs + 1 /* Take argv[0] into account */);
 
     /*
@@ -1652 +1652 @@
                                          * So, NIL the handles to avoid closing them again.
                                          */
+                                        /** @todo r=bird: Can't see how hNotificationPipeR could be closed here!  Found (and fixed)
+                                         * confused comments documenting hNotificationPipeW, probably related. */
                                         if (RT_FAILURE(RTPollSetQueryHandle(pProcess->hPollSet,
                                                                             VBOXSERVICECTRLPIPEID_IPC_NOTIFY, NULL)))
@@ -1725 +1727 @@
         RTMemFree(papszEnv);
     }
-    if (uNumArgs)
+    if (cArgs)
         RTGetOptArgvFree(papszArgs);
 
@@ -1739 +1741 @@
 
     /* Finally, update stopped status. */
-    ASMAtomicXchgBool(&pProcess->fStopped, true);
+    ASMAtomicWriteBool(&pProcess->fStopped, true);
+    ASMAtomicWriteBool(&pProcess->fShutdown, true);
 
     return rc;
@@ -1826 +1829 @@
             AssertRC(rc);
             if (   ASMAtomicReadBool(&pProcess->fShutdown)
+                || ASMAtomicReadBool(&pProcess->fStopped)
                 || RT_FAILURE(rc))
             {
                 VGSvcError("Thread for process '%s' failed to start, rc=%Rrc\n", pStartupInfo->szCmd, rc);
+                int rc2 = RTThreadWait(pProcess->Thread, RT_MS_1SEC * 30, NULL);
+                if (RT_SUCCESS(rc2))
+                    pProcess->Thread = NIL_RTTHREAD;
                 VGSvcGstCtrlProcessFree(pProcess);
             }
@@ -2054 +2061 @@
             /* Wake up the process' notification pipe to get
              * the request being processed. */
-            Assert(pProcess->hNotificationPipeW != NIL_RTPIPE);
+            Assert(pProcess->hNotificationPipeW != NIL_RTPIPE || pProcess->fShutdown /* latter in case of race */);
             size_t cbWritten = 0;
             rc = RTPipeWrite(pProcess->hNotificationPipeW, "i", 1, &cbWritten);
@@ -2122 +2129 @@
                                    bool fPendingClose, void *pvBuf, uint32_t cbBuf)
 {
-    if (!ASMAtomicReadBool(&pProcess->fShutdown))
+    if (!ASMAtomicReadBool(&pProcess->fShutdown) && !ASMAtomicReadBool(&pProcess->fStopped))
         return vgsvcGstCtrlProcessRequestAsync(pProcess, pHostCtx, (PFNRT)vgsvcGstCtrlProcessOnInput,
                                                5 /* cArgs */, pProcess, pHostCtx, fPendingClose, pvBuf, cbBuf);
@@ -2133 +2140 @@
                                     uint32_t uHandle, uint32_t cbToRead, uint32_t fFlags)
 {
-    if (!ASMAtomicReadBool(&pProcess->fShutdown))
+    if (!ASMAtomicReadBool(&pProcess->fShutdown) && !ASMAtomicReadBool(&pProcess->fStopped))
         return vgsvcGstCtrlProcessRequestAsync(pProcess, pHostCtx, (PFNRT)vgsvcGstCtrlProcessOnOutput,
                                                5 /* cArgs */, pProcess, pHostCtx, uHandle, cbToRead, fFlags);
@@ -2143 +2150 @@
 int VGSvcGstCtrlProcessHandleTerm(PVBOXSERVICECTRLPROCESS pProcess)
 {
-    if (!ASMAtomicReadBool(&pProcess->fShutdown))
+    if (!ASMAtomicReadBool(&pProcess->fShutdown) && !ASMAtomicReadBool(&pProcess->fStopped))
         return vgsvcGstCtrlProcessRequestAsync(pProcess, NULL /* pHostCtx */, (PFNRT)vgsvcGstCtrlProcessOnTerm,
                                                1 /* cArgs */, pProcess);

trunk/src/VBox/Additions/common/VBoxService/VBoxServiceToolBox.h

@@ -26 +26 @@
 RT_C_DECLS_END
 
-#endif /* ___VBoxServiceToolBox_h */
+#endif /* !___VBoxServiceToolBox_h */