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

Timestamp:

Jan 23, 2012 6:38:18 PM (13 years ago)

Author:

vboxsync

Message:

GuestCtrl: Request (IPC) changes, bugfixes, fixed handle leaks.

Location:

trunk/src/VBox/Additions/common/VBoxService

Files:

• VBoxService.cpp (modified) (2 diffs)
• VBoxServiceControl.cpp (modified) (7 diffs)
• VBoxServiceControlThread.cpp (modified) (40 diffs)
• VBoxServiceInternal.h (modified) (7 diffs)

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

@@ -40 +40 @@
 #include <iprt/buildconfig.h>
 #include <iprt/initterm.h>
+#ifdef DEBUG
+# include <iprt/memtracker.h>
+#endif
 #include <iprt/message.h>
 #include <iprt/path.h>
@@ -935 +938 @@
 #ifdef DEBUG
     RTCritSectDelete(&g_csLog);
+    //RTMemTrackerDumpAllToStdOut();
 #endif
     return rcExit;

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

@@ -187 +187 @@
         }
         else if (RT_FAILURE(rc))
-            VBoxServiceVerbose(3, "Control: Getting host message failed with %Rrc\n", rc); /* VERR_GEN_IO_FAILURE seems to be normal if ran  into timeout. */
+            VBoxServiceVerbose(3, "Control: Getting host message failed with %Rrc\n", rc); /* VERR_GEN_IO_FAILURE seems to be normal if ran into timeout. */
         if (RT_SUCCESS(rc))
         {
@@ -360 +360 @@
 
     int rc = VINF_SUCCESS;
-
-    VBOXSERVICECTRLREQUEST ctrlRequest;
-    ctrlRequest.uCID   = uCID;
-    ctrlRequest.cbData = cbBuf;
-    ctrlRequest.pvData = (uint8_t*)pvBuf;
-
+    VBOXSERVICECTRLREQUESTTYPE reqType;
     switch (uHandleId)
     {
         case OUTPUT_HANDLE_ID_STDERR:
-            ctrlRequest.enmType = VBOXSERVICECTRLREQUEST_STDERR_READ;
+            reqType = VBOXSERVICECTRLREQUEST_STDERR_READ;
             break;
 
         case OUTPUT_HANDLE_ID_STDOUT:
         case OUTPUT_HANDLE_ID_STDOUT_DEPRECATED:
-            ctrlRequest.enmType = VBOXSERVICECTRLREQUEST_STDOUT_READ;
+            reqType = VBOXSERVICECTRLREQUEST_STDOUT_READ;
             break;
 
@@ -382 +377 @@
     }
 
+    PVBOXSERVICECTRLREQUEST pRequest;
     if (RT_SUCCESS(rc))
-        rc = VBoxServiceControlThreadPerform(uPID, &ctrlRequest);
-
-    if (RT_SUCCESS(rc))
-    {
-        if (pcbRead)
-            *pcbRead = ctrlRequest.cbData;
-    }
-    else /* Something went wrong, nothing read. */
-        *pcbRead = 0;
+    {
+        rc = VBoxServiceControlThreadRequestAllocEx(&pRequest, reqType,
+                                                    pvBuf, cbBuf, uCID);
+        if (RT_SUCCESS(rc))
+            rc = VBoxServiceControlThreadPerform(uPID, pRequest);
+
+        if (RT_SUCCESS(rc))
+        {
+            if (pcbRead)
+                *pcbRead = pRequest->cbData;
+        }
+
+        VBoxServiceControlThreadRequestFree(pRequest);
+    }
 
     return rc;
@@ -416 +417 @@
     /* pcbWritten is optional. */
 
-    int rc = VINF_SUCCESS;
-
-    VBOXSERVICECTRLREQUEST ctrlRequest;
-    ctrlRequest.uCID    = uCID;
-    ctrlRequest.cbData  = cbBuf;
-    ctrlRequest.pvData  = pvBuf;
-    ctrlRequest.enmType = fPendingClose
-                        ? VBOXSERVICECTRLREQUEST_STDIN_WRITE_EOF : VBOXSERVICECTRLREQUEST_STDIN_WRITE;
+    PVBOXSERVICECTRLREQUEST pRequest;
+    int rc = VBoxServiceControlThreadRequestAllocEx(&pRequest,
+                                                      fPendingClose
+                                                    ? VBOXSERVICECTRLREQUEST_STDIN_WRITE_EOF
+                                                    : VBOXSERVICECTRLREQUEST_STDIN_WRITE,
+                                                    pvBuf, cbBuf, uCID);
     if (RT_SUCCESS(rc))
-        rc = VBoxServiceControlThreadPerform(uPID, &ctrlRequest);
-
-    if (RT_SUCCESS(rc))
-    {
-        if (pcbWritten)
-            *pcbWritten = ctrlRequest.cbData;
+    {
+        rc = VBoxServiceControlThreadPerform(uPID, pRequest);
+        if (RT_SUCCESS(rc))
+        {
+            if (pcbWritten)
+                *pcbWritten = pRequest->cbData;
+        }
+
+        VBoxServiceControlThreadRequestFree(pRequest);
     }
 
@@ -559 +561 @@
     if (RT_SUCCESS(rc))
     {
-        uint32_t cbRead = 0;
         uint8_t *pBuf = (uint8_t*)RTMemAlloc(_64K);
         if (pBuf)
         {
+            uint32_t cbRead = 0;
             rc = VBoxServiceControlExecGetOutput(uPID, uContextID, uHandleID, RT_INDEFINITE_WAIT /* Timeout */,
                                                  pBuf, _64K /* cbSize */, &cbRead);
+            VBoxServiceVerbose(3, "Control: Got output returned with rc=%Rrc (PID=%u, CID=%u, cbRead=%u, uHandle=%u, uFlags=%u)\n",
+                               rc, uPID, uContextID, cbRead, uHandleID, uFlags);
 
             /** Note: Don't convert/touch/modify/whatever the output data here! This might be binary
              *        data which the host needs to work with -- so just pass through all data unfiltered! */
 
-            if (RT_SUCCESS(rc))
-                VBoxServiceVerbose(2, "Control: Got output, PID=%u, CID=%u, cbRead=%u, uHandle=%u, uFlags=%u\n",
-                                   uPID, uContextID, cbRead, uHandleID, uFlags);
-            else if (rc == VERR_NOT_FOUND)
-                VBoxServiceVerbose(2, "Control: PID=%u not found, CID=%u, uHandle=%u\n",
-                                   uPID, uContextID, uHandleID, rc);
-            else
-                VBoxServiceError("Control: Failed to retrieve output for PID=%u, CID=%u, uHandle=%u, rc=%Rrc\n",
-                                 uPID, uContextID, uHandleID, rc);
             /* Note: Since the context ID is unique the request *has* to be completed here,
              *       regardless whether we got data or not! Otherwise the progress object
              *       on the host never will get completed! */
-            /* cbRead now contains actual size. */
             int rc2 = VbglR3GuestCtrlExecSendOut(idClient, uContextID, uPID, uHandleID, uFlags,
                                                  pBuf, cbRead);
@@ -656 +650 @@
 
 #ifdef DEBUG
-        PVBOXSERVICECTRLTHREAD pThreadCur;
-        uint32_t cThreads = 0;
-        RTListForEach(&g_GuestControlThreads, pThreadCur, VBOXSERVICECTRLTHREAD, Node)
-            cThreads++;
-        VBoxServiceVerbose(4, "Control: Guest process threads left=%u\n", cThreads);
+    PVBOXSERVICECTRLTHREAD pThreadCur;
+    uint32_t cThreads = 0;
+    RTListForEach(&g_GuestControlThreads, pThreadCur, VBOXSERVICECTRLTHREAD, Node)
+        cThreads++;
+    VBoxServiceVerbose(4, "Control: Guest process threads left=%u\n", cThreads);
 #endif
     AssertMsg(RTListIsEmpty(&g_GuestControlThreads),
@@ -717 +711 @@
             RTListForEach(&g_GuestControlThreads, pThread, VBOXSERVICECTRLTHREAD, Node)
             {
-                VBOXSERVICECTRLTHREADSTATUS enmStatus = VBoxServiceControlThreadGetStatus(pThread);
-                if (enmStatus == VBOXSERVICECTRLTHREADSTATUS_STARTED)
-                    uProcsRunning++;
-                else if (enmStatus == VBOXSERVICECTRLTHREADSTATUS_STOPPED)
-                    uProcsStopped++;
-                else
-                    AssertMsgFailed(("Control: Guest process neither started nor stopped!?\n"));
+                  VBoxServiceControlThreadActive(pThread)
+                ? uProcsRunning++
+                : uProcsStopped++;
             }
 

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

@@ -44 +44 @@
 /* Internal functions. */
 static int vboxServiceControlThreadFree(PVBOXSERVICECTRLTHREAD pThread);
+static int vboxServiceControlThreadRequestCancel(PVBOXSERVICECTRLREQUEST pThread);
 
 /**
@@ -70 +71 @@
                                     const char *pszUser, const char *pszPassword, uint32_t uTimeLimitMS)
 {
-    AssertPtr(pThread);
+    AssertPtrReturn(pThread, VERR_INVALID_POINTER);
 
     /* General stuff. */
@@ -89 +90 @@
         return rc;
 
-    rc = RTSemEventMultiCreate(&pThread->RequestEvent);
-    AssertRCReturn(rc, rc);
-
     pThread->uPID = 0; /* Don't have a PID yet. */
-    pThread->pszCmd = RTStrDup(pszCmd);
     pThread->uFlags = uFlags;
     pThread->uTimeLimitMS = (   uTimeLimitMS == UINT32_MAX
@@ -136 +133 @@
                 cbLen  += cbStr + 1; /* Skip terminating '\0' */
             }
-        }
+            Assert(i == pThread->uNumEnvVars);
+        }
+
+        /* The actual command to execute. */
+        pThread->pszCmd      = RTStrDup(pszCmd);
+        AssertPtr(pThread->pszCmd);
 
         /* User management. */
@@ -161 +163 @@
     AssertPtrReturn(pThread, VERR_INVALID_POINTER);
 
-    VBoxServiceVerbose(1, "ControlThread: [PID %u]: Shutting down ...\n",
+    VBoxServiceVerbose(3, "ControlThread: [PID %u]: Freeing thread data ...\n",
                        pThread->uPID);
 
-    /* Signal the request event to unblock potential waiters. */
-    int rc = RTSemEventMultiSignal(pThread->RequestEvent);
+    int rc = vboxServiceControlThreadRequestCancel(pThread->pRequest);
     if (RT_FAILURE(rc))
         VBoxServiceError("ControlThread: [PID %u]: Signalling request event failed, rc=%Rrc\n",
-                         pThread->uPID, rc);
+                     pThread->uPID, rc);
 
     rc = RTCritSectEnter(&pThread->CritSect);
     if (RT_SUCCESS(rc))
     {
-        VBoxServiceVerbose(3, "ControlThread: [PID %u]: Freeing thread data ...\n",
-                           pThread->uPID);
-
-        RTStrFree(pThread->pszCmd);
         if (pThread->uNumEnvVars)
         {
@@ -184 +181 @@
         }
         RTGetOptArgvFree(pThread->papszArgs);
+
+        RTStrFree(pThread->pszCmd);
         RTStrFree(pThread->pszUser);
         RTStrFree(pThread->pszPassword);
 
-        rc = RTSemEventMultiDestroy(pThread->RequestEvent);
-        AssertRC(rc);
-
-        VBoxServiceVerbose(3, "ControlThread: [PID %u]: Cleaning up ...\n",
+        VBoxServiceVerbose(3, "ControlThread: [PID %u]: Setting stopped state\n",
                            pThread->uPID);
 
@@ -233 +229 @@
      * The guest thread loop will do that as soon as it processes the quit message. */
 
-    VBOXSERVICECTRLREQUEST ctrlRequest;
-    RT_ZERO(ctrlRequest);
-    ctrlRequest.enmType = VBOXSERVICECTRLREQUEST_QUIT;
-
-    int rc = VBoxServiceControlThreadPerform(pThread->uPID, &ctrlRequest);
-    if (RT_FAILURE(rc))
-        VBoxServiceVerbose(3, "ControlThread: [PID %u]: Sending quit request failed with rc=%Rrc\n",
-                           pThread->uPID, rc);
+    PVBOXSERVICECTRLREQUEST pRequest;
+    int rc = VBoxServiceControlThreadRequestAlloc(&pRequest, VBOXSERVICECTRLREQUEST_QUIT);
+    if (RT_SUCCESS(rc))
+    {
+        rc = VBoxServiceControlThreadPerform(pThread->uPID, pRequest);
+        if (RT_FAILURE(rc))
+            VBoxServiceVerbose(3, "ControlThread: [PID %u]: Sending quit request failed with rc=%Rrc\n",
+                               pThread->uPID, rc);
+
+        VBoxServiceControlThreadRequestFree(pRequest);
+    }
     return rc;
 }
@@ -266 +265 @@
         int rcThread;
         rc = RTThreadWait(pThread->Thread, msTimeout, &rcThread);
-        if (RT_FAILURE(rcThread))
-        {
-            VBoxServiceError("ControlThread: [PID %u]: Shutdown returned error rc=%Rrc\n",
-                             pThread->uPID, rcThread);
-            if (RT_SUCCESS(rc))
+        if (RT_FAILURE(rc))
+        {
+            VBoxServiceError("ControlThread: [PID %u]: Waiting for shutting down thread returned error rc=%Rrc\n",
+                             pThread->uPID, rc);
+        }
+        else
+        {
+            if (RT_FAILURE(rcThread))
+            {
+                VBoxServiceError("ControlThread: [PID %u]: Shutdown returned error rc=%Rrc\n",
+                                 pThread->uPID, rcThread);
                 rc = rcThread;
+            }
         }
     }
@@ -279 +285 @@
 
 /**
- * Returns the guest process thread's current status.
- *
- * @return  VBOXSERVICECTRLTHREADSTATUS
+ * Returns whether a guest process thread still is up and running.
+ *
+ * @return  true if thread is running, false if not.
  * @param   pThread             Thread to determine current status for.
  */
-VBOXSERVICECTRLTHREADSTATUS VBoxServiceControlThreadGetStatus(const PVBOXSERVICECTRLTHREAD pThread)
-{
-    AssertPtrReturn(pThread, VBOXSERVICECTRLTHREADSTATUS_UNKNOWN);
+bool VBoxServiceControlThreadActive(const PVBOXSERVICECTRLTHREAD pThread)
+{
+    AssertPtrReturn(pThread, false);
 
     int rc = RTCritSectEnter(&pThread->CritSect);
     if (RT_SUCCESS(rc))
     {
-        Assert(pThread->fStarted != pThread->fStopped);
-
-        VBOXSERVICECTRLTHREADSTATUS enmStatus = VBOXSERVICECTRLTHREADSTATUS_UNKNOWN;
-        /** @todo Add more logic here. */
-        /** @todo Remove fStarted/fStopped and just use this VBOXSERVICECTRLTHREADSTATUS. */
-        if (pThread->fStarted)
-            enmStatus = VBOXSERVICECTRLTHREADSTATUS_STARTED;
-        else if (pThread->fStopped)
-            enmStatus = VBOXSERVICECTRLTHREADSTATUS_STOPPED;
-        else
-            AssertMsgFailed(("ControlThread: Uknown thread status (0x%x)\n"));
+        /* We only are interested whether the thread really is alive
+         * and kicking or not, meaning:
+         * - thread was started
+         * - thread is able to serve IPC messages
+         *
+         * Even if the fShutdown flag is set this means the thread should be
+         * considered as being running (destruction phase).
+         */
+        bool fRunning = !pThread->fStopped;
 
         rc = RTCritSectLeave(&pThread->CritSect);
         AssertRC(rc);
 
-        return enmStatus;
-    }
-
-    return VBOXSERVICECTRLTHREADSTATUS_UNKNOWN;
+        return fRunning;
+    }
+
+    return false;
 }
 
@@ -368 +372 @@
                                                      PRTPIPE phPipeR, uint32_t idPollHnd)
 {
-    int rc = VINF_SUCCESS;
-
-    rc = RTPollSetRemove(hPollSet, idPollHnd);
-    AssertRC(rc);
-
-    rc = RTPipeClose(*phPipeR);
-    AssertRC(rc);
-    *phPipeR = NIL_RTPIPE;
-
-    return rc;
+    AssertPtrReturn(phPipeR, VERR_INVALID_POINTER);
+
+#ifdef DEBUG
+    VBoxServiceVerbose(4, "ControlThread: VBoxServiceControlThreadHandleOutputError: fPollEvt=0x%x, idPollHnd=%u\n",
+                       fPollEvt, idPollHnd);
+#endif
+
+    /* Remove pipe from poll set. */
+    int rc2 = RTPollSetRemove(hPollSet, idPollHnd);
+    AssertMsg(RT_SUCCESS(rc2) || rc2 == VERR_POLL_HANDLE_ID_NOT_FOUND, ("%Rrc\n", rc2));
+
+    bool fClosePipe = true; /* By default close the pipe. */
+
+    /* Check if there's remaining data to read from the pipe. */
+    size_t cbReadable;
+    rc2 = RTPipeQueryReadable(*phPipeR, &cbReadable);
+    if (   RT_SUCCESS(rc2)
+        && cbReadable)
+    {
+        VBoxServiceVerbose(3, "ControlThread: VBoxServiceControlThreadHandleOutputError: idPollHnd=%u has %ld bytes left, vetoing close\n",
+                           idPollHnd, cbReadable);
+
+        /* Veto closing the pipe yet because there's still stuff to read
+         * from the pipe. This can happen on UNIX-y systems where on
+         * error/hangup there still can be data to be read out. */
+        fClosePipe = false;
+    }
+    else
+        VBoxServiceVerbose(3, "ControlThread: VBoxServiceControlThreadHandleOutputError: idPollHnd=%u will be closed\n",
+                           idPollHnd);
+
+    if (   *phPipeR != NIL_RTPIPE
+        && fClosePipe)
+    {
+        rc2 = RTPipeClose(*phPipeR);
+        AssertRC(rc2);
+        *phPipeR = NIL_RTPIPE;
+    }
+
+    return VINF_SUCCESS;
 }
 
@@ -394 +428 @@
                                                      PRTPIPE phPipeR, uint32_t idPollHnd)
 {
+#if 0
+    VBoxServiceVerbose(4, "ControlThread: VBoxServiceControlThreadHandleOutputEvent: fPollEvt=0x%x, idPollHnd=%u\n",
+                       fPollEvt, idPollHnd);
+#endif
+
     int rc = VINF_SUCCESS;
 
-    if (fPollEvt & RTPOLL_EVT_READ)
-    {
+#ifdef DEBUG
+    size_t cbReadable;
+    rc = RTPipeQueryReadable(*phPipeR, &cbReadable);
+    if (   RT_SUCCESS(rc)
+        && cbReadable)
+    {
+        VBoxServiceVerbose(4, "ControlThread: VBoxServiceControlThreadHandleOutputEvent: cbReadable=%ld\n",
+                           cbReadable);
+    }
+#endif
+
+#if 0
+    //if (fPollEvt & RTPOLL_EVT_READ)
+    {
+        size_t cbRead = 0;
+        uint8_t byData[_64K];
+        rc = RTPipeRead(*phPipeR,
+                        byData, sizeof(byData), &cbRead);
+        VBoxServiceVerbose(4, "ControlThread: VBoxServiceControlThreadHandleOutputEvent cbRead=%u, rc=%Rrc\n",
+                           cbRead, rc);
 
         /* Make sure we go another poll round in case there was too much data
@@ -403 +460 @@
         fPollEvt &= RTPOLL_EVT_ERROR;
     }
+#endif
 
     if (fPollEvt & RTPOLL_EVT_ERROR)
@@ -415 +473 @@
                                                  PVBOXSERVICECTRLTHREAD pThread)
 {
-#ifdef DEBUG_andy
-    VBoxServiceVerbose(4, "ControlThread: HandleIPCRequest\n");
-#endif
-
     AssertPtrReturn(pThread, VERR_INVALID_POINTER);
     AssertPtrReturn(phStdInW, VERR_INVALID_POINTER);
@@ -446 +500 @@
             /** @todo Check for some conditions to check to
              *        veto quitting. */
-            pThread->fShutdown = true;
-            rcReq = VINF_SUCCESS;
+            ASMAtomicXchgBool(&pThread->fShutdown, true);
+            rcReq = VERR_CANCELLED;
             break;
         }
@@ -498 +552 @@
                 rcReq = RTPipeRead(*pPipeR,
                                    pRequest->pvData, pRequest->cbData, &cbRead);
-                if (rcReq == VERR_BROKEN_PIPE)
-                    rcReq = VINF_EOF;
+                if (RT_FAILURE(rcReq))
+                {
+                    RTPollSetRemove(hPollSet, pRequest->enmType == VBOXSERVICECTRLREQUEST_STDERR_READ
+                                              ? VBOXSERVICECTRLPIPEID_STDERR : VBOXSERVICECTRLPIPEID_STDOUT);
+                    RTPipeClose(*pPipeR);
+                    *pPipeR = NIL_RTPIPE;
+                    if (rcReq == VERR_BROKEN_PIPE)
+                        rcReq = VINF_EOF;
+                }
             }
             else
@@ -518 +579 @@
                  ? rcReq : rc;
 
-    VBoxServiceVerbose(2, "ControlThread: [PID %u]: Handled req=%u, CID=%u, rcReq=%Rrc, cbData=%u\n",
-                       pThread->uPID, pRequest->enmType, pRequest->uCID, rcReq, pRequest->cbData);
+    VBoxServiceVerbose(2, "ControlThread: [PID %u]: Handled req=%u, CID=%u, rc=%Rrc, cbData=%u\n",
+                       pThread->uPID, pRequest->enmType, pRequest->uCID, pRequest->rc, pRequest->cbData);
 
     /* In any case, regardless of the result, we notify
      * the main guest control to unblock it. */
-    int rc2 = RTSemEventMultiSignal(pThread->RequestEvent);
+    int rc2 = RTSemEventMultiSignal(pRequest->Event);
     AssertRC(rc2);
+
     /* No access to pRequest here anymore -- could be out of scope
      * or modified already! */
+    pThread->pRequest = pRequest = NULL;
 
     return rc;
@@ -640 +703 @@
         }
 
+#if 0
+        VBoxServiceVerbose(4, "ControlThread: [PID %u]: Polling done, pollRC=%Rrc, pollCnt=%u, rc=%Rrc, fShutdown=%RTbool\n",
+                           pThread->uPID, rc2, RTPollSetGetCount(hPollSet), rc, pThread->fShutdown);
+#endif
         /*
          * Check for process death.
@@ -665 +732 @@
 
         /*
-         * If the process has terminated, we're should head out.
+         * If the process has terminated and all output has been consumed,
+         * we should be heading out.
          */
-        if (!fProcessAlive)
+        if (   !fProcessAlive
+            && *phStdOutR == NIL_RTPIPE
+            && *phStdErrR == NIL_RTPIPE)
             break;
 
@@ -700 +770 @@
 
         /* Reset the polling interval since we've done all pending work. */
-        cMsPollCur = cMilliesLeft >= cMsPollBase ? cMsPollBase : cMilliesLeft;
+        cMsPollCur = fProcessAlive
+                   ? cMsPollBase
+                   : RT_MS_1MIN;
+        if (cMilliesLeft < cMsPollCur)
+            cMsPollCur = cMilliesLeft;
 
         /*
@@ -707 +781 @@
         if (pThread->fShutdown)
             break;
+    }
+
+    rc2 = RTCritSectEnter(&pThread->CritSect);
+    if (RT_SUCCESS(rc2))
+    {
+        ASMAtomicXchgBool(&pThread->fShutdown, true);
+
+        rc2 = RTCritSectLeave(&pThread->CritSect);
+        AssertRC(rc2);
     }
 
@@ -789 +872 @@
         else if (ProcessStatus.enmReason == RTPROCEXITREASON_NORMAL)
         {
-            VBoxServiceVerbose(3, "ControlThread: [PID %u]: Ended with RTPROCEXITREASON_NORMAL (%u)\n",
+            VBoxServiceVerbose(3, "ControlThread: [PID %u]: Ended with RTPROCEXITREASON_NORMAL (Exit code: %u)\n",
                                pThread->uPID, ProcessStatus.iStatus);
 
@@ -797 +880 @@
         else if (ProcessStatus.enmReason == RTPROCEXITREASON_SIGNAL)
         {
-            VBoxServiceVerbose(3, "ControlThread: [PID %u]: Ended with RTPROCEXITREASON_SIGNAL (%u)\n",
+            VBoxServiceVerbose(3, "ControlThread: [PID %u]: Ended with RTPROCEXITREASON_SIGNAL (Signal: %u)\n",
                                pThread->uPID, ProcessStatus.iStatus);
 
@@ -805 +888 @@
         else if (ProcessStatus.enmReason == RTPROCEXITREASON_ABEND)
         {
-            VBoxServiceVerbose(3, "ControlThread: [PID %u]: Ended with RTPROCEXITREASON_ABEND (%u)\n",
-                               pThread->uPID, ProcessStatus.iStatus);
+            /* ProcessStatus.iStatus will be undefined. */
+            VBoxServiceVerbose(3, "ControlThread: [PID %u]: Ended with RTPROCEXITREASON_ABEND\n",
+                               pThread->uPID);
 
             uStatus = PROC_STS_TEA;
@@ -834 +918 @@
 
 
+/**
+ * Initializes a pipe's handle and pipe object.
+ *
+ * @return  IPRT status code.
+ * @param   ph                      The pipe's handle to initialize.
+ * @param   phPipe                  The pipe's object to initialize.
+ */
 static int vboxServiceControlThreadInitPipe(PRTHANDLE ph, PRTPIPE phPipe)
 {
@@ -844 +935 @@
 
     return VINF_SUCCESS;
+}
+
+
+/**
+ * Allocates a guest thread request with the specified request data.
+ *
+ * @return  IPRT status code.
+ * @param   ppReq                   Pointer that will receive the newly allocated request.
+ *                                  Must be freed later with VBoxServiceControlThreadRequestFree().
+ * @param   enmType                 Request type.
+ * @param   pbData                  Payload data, based on request type.
+ * @param   cbData                  Size of payload data (in bytes).
+ * @param   uCID                    Context ID to which this request belongs to.
+ */
+int VBoxServiceControlThreadRequestAllocEx(PVBOXSERVICECTRLREQUEST   *ppReq,
+                                           VBOXSERVICECTRLREQUESTTYPE enmType,
+                                           void*                      pbData,
+                                           size_t                     cbData,
+                                           uint32_t                   uCID)
+{
+    AssertPtrReturn(ppReq, VERR_INVALID_POINTER);
+
+    PVBOXSERVICECTRLREQUEST pReq = (PVBOXSERVICECTRLREQUEST)
+        RTMemAlloc(sizeof(VBOXSERVICECTRLREQUEST));
+    AssertPtrReturn(pReq, VERR_NO_MEMORY);
+
+    RT_ZERO(*pReq);
+    pReq->enmType = enmType;
+    pReq->uCID    = uCID;
+    pReq->cbData  = cbData;
+    pReq->pvData  = (uint8_t*)pbData;
+
+    /* Set request result to some defined state in case
+     * it got cancelled. */
+    pReq->rc      = VERR_CANCELLED;
+
+    int rc = RTSemEventMultiCreate(&pReq->Event);
+    AssertRC(rc);
+
+    if (RT_SUCCESS(rc))
+    {
+        *ppReq = pReq;
+        return VINF_SUCCESS;
+    }
+
+    RTMemFree(pReq);
+    return rc;
+}
+
+
+/**
+ * Allocates a guest thread request with the specified request data.
+ *
+ * @return  IPRT status code.
+ * @param   ppReq                   Pointer that will receive the newly allocated request.
+ *                                  Must be freed later with VBoxServiceControlThreadRequestFree().
+ * @param   enmType                 Request type.
+ */
+int VBoxServiceControlThreadRequestAlloc(PVBOXSERVICECTRLREQUEST *ppReq,
+                                         VBOXSERVICECTRLREQUESTTYPE enmType)
+{
+    return VBoxServiceControlThreadRequestAllocEx(ppReq, enmType,
+                                                  0 /* cbData */, NULL /* pvData */,
+                                                  0 /* ContextID */);
+}
+
+
+/**
+ * Cancels a previously fired off guest thread request.
+ *
+ * Note: Does *not* do locking since VBoxServiceControlThreadRequestWait()
+ * holds the lock (critsect); so only trigger the signal; the owner
+ * needs to clean up afterwards.
+ *
+ * @return  IPRT status code.
+ * @param   pReq                    Request to cancel.
+ */
+static int vboxServiceControlThreadRequestCancel(PVBOXSERVICECTRLREQUEST pReq)
+{
+    if (!pReq) /* Silently skip non-initialized requests. */
+        return VINF_SUCCESS;
+
+    VBoxServiceVerbose(4, "ControlThread: Cancelling request 0x%p\n", pReq);
+
+    return RTSemEventMultiSignal(pReq->Event);
+}
+
+
+/**
+ * Frees a formerly allocated guest thread request.
+ *
+ * @return  IPRT status code.
+ * @param   pReq                    Request to free.
+ */
+void VBoxServiceControlThreadRequestFree(PVBOXSERVICECTRLREQUEST pReq)
+{
+    AssertPtrReturnVoid(pReq);
+
+    VBoxServiceVerbose(4, "ControlThread: Freeing request 0x%p (event 0x%p)\n",
+                       pReq, &pReq->Event);
+
+    int rc = RTSemEventMultiDestroy(pReq->Event);
+    AssertRC(rc);
+
+    RTMemFree(pReq);
+    pReq = NULL;
+}
+
+
+/**
+ * Waits for a guest thread's event to get triggered.
+ *
+ * @return  IPRT status code.
+ * @param   pReq                    Request to wait for.
+ */
+int VBoxServiceControlThreadRequestWait(PVBOXSERVICECTRLREQUEST pReq)
+{
+    AssertPtrReturn(pReq, VERR_INVALID_POINTER);
+
+    /* Wait on the request to get completed (or we are asked to abort/shutdown). */
+    int rc = RTSemEventMultiWait(pReq->Event, RT_INDEFINITE_WAIT);
+    if (RT_SUCCESS(rc))
+    {
+        VBoxServiceVerbose(4, "ControlThread: Performed request with rc=%Rrc, cbData=%u\n",
+                           pReq->rc, pReq->cbData);
+
+        /* Give back overall request result. */
+        rc = pReq->rc;
+    }
+    else
+        VBoxServiceError("ControlThread: Waiting for request failed, rc=%Rrc\n", rc);
+
+    return rc;
 }
 
@@ -865 +1089 @@
 {
     AssertPtrReturn(ph, VERR_INVALID_POINTER);
-    AssertPtrReturn(ph, VERR_INVALID_POINTER);
     AssertPtrReturn(pph, VERR_INVALID_POINTER);
+    AssertPtrReturn(phPipe, VERR_INVALID_POINTER);
 
     int rc;
@@ -914 +1138 @@
     }
     else /* Add other piping stuff here. */
-        rc = VERR_INVALID_PARAMETER;
+        rc = VINF_SUCCESS; /* Same as parent (us). */
 
     return rc;
@@ -957 +1181 @@
  * @param   cbResolved                  Size (in bytes) of resolved file name string.
  */
-static int VBoxServiceControlThreadResolveExecutable(const char *pszFileName, char *pszResolved, size_t cbResolved)
+static int VBoxServiceControlThreadResolveExecutable(const char *pszFileName,
+                                                     char *pszResolved, size_t cbResolved)
 {
     int rc = VINF_SUCCESS;
@@ -1020 +1245 @@
             if (RT_SUCCESS(rc))
                 rc = RTStrAAppend(&pszNewArgs, pszArgs);
+            RTStrFree(pszArgs);
         }
     }
@@ -1098 +1324 @@
                                     phStdIn, phStdOut, phStdErr, NULL /* pszAsUser */,
                                     NULL /* pszPassword */, phProcess);
+                RTGetOptArgvFree(papszArgsExp);
             }
-            RTGetOptArgvFree(papszArgsExp);
         }
         return rc;
@@ -1214 +1440 @@
                 RTHANDLE    hStdOut;
                 PRTHANDLE   phStdOut;
-                RTPIPE      hStdOutR;
+                RTPIPE      pipeStdOutR;
                 rc = VBoxServiceControlThreadSetupPipe(  (pThread->uFlags & EXECUTEPROCESSFLAG_WAIT_STDOUT)
                                                        ? "|" : "/dev/null",
                                                        1 /*STDOUT_FILENO*/,
-                                                       &hStdOut, &phStdOut, &hStdOutR);
+                                                       &hStdOut, &phStdOut, &pipeStdOutR);
                 if (RT_SUCCESS(rc))
                 {
                     RTHANDLE    hStdErr;
                     PRTHANDLE   phStdErr;
-                    RTPIPE      hStdErrR;
+                    RTPIPE      pipeStdErrR;
                     rc = VBoxServiceControlThreadSetupPipe(  (pThread->uFlags & EXECUTEPROCESSFLAG_WAIT_STDERR)
                                                            ? "|" : "/dev/null",
                                                            2 /*STDERR_FILENO*/,
-                                                           &hStdErr, &phStdErr, &hStdErrR);
+                                                           &hStdErr, &phStdErr, &pipeStdErrR);
                     if (RT_SUCCESS(rc))
                     {
@@ -1238 +1464 @@
                         if (RT_SUCCESS(rc))
                         {
+                            uint32_t uFlags = RTPOLL_EVT_ERROR;
+#if 0
+                            /* Add reading event to pollset to get some more information. */
+                            uFlags |= RTPOLL_EVT_READ;
+#endif
                             /* Stdin. */
                             if (RT_SUCCESS(rc))
@@ -1243 +1474 @@
                             /* Stdout. */
                             if (RT_SUCCESS(rc))
-                                rc = RTPollSetAddPipe(hPollSet, hStdOutR, RTPOLL_EVT_ERROR, VBOXSERVICECTRLPIPEID_STDOUT);
+                                rc = RTPollSetAddPipe(hPollSet, pipeStdOutR, uFlags, VBOXSERVICECTRLPIPEID_STDOUT);
                             /* Stderr. */
                             if (RT_SUCCESS(rc))
-                                rc = RTPollSetAddPipe(hPollSet, hStdErrR, RTPOLL_EVT_ERROR, VBOXSERVICECTRLPIPEID_STDERR);
+                                rc = RTPollSetAddPipe(hPollSet, pipeStdErrR, uFlags, VBOXSERVICECTRLPIPEID_STDERR);
                             /* IPC notification pipe. */
                             if (RT_SUCCESS(rc))
@@ -1269 +1500 @@
                                  */
                                 int rc2 = RTThreadUserSignal(RTThreadSelf());
-                                if (RT_FAILURE(rc2))
+                                if (RT_SUCCESS(rc))
                                     rc = rc2;
                                 fSignalled = true;
@@ -1288 +1519 @@
                                     rc = VBoxServiceControlThreadProcLoop(pThread,
                                                                           hProcess, pThread->uTimeLimitMS, hPollSet,
-                                                                          &pThread->pipeStdInW, &hStdOutR, &hStdErrR);
+                                                                          &pThread->pipeStdInW, &pipeStdOutR, &pipeStdErrR);
                                     /*
                                      * The handles that are no longer in the set have
@@ -1315 +1546 @@
                             pThread->hNotificationPipeW = NIL_RTPIPE;
                         }
-                        RTPipeClose(hStdErrR);
-                        hStdErrR = NIL_RTPIPE;
+                        RTPipeClose(pipeStdErrR);
+                        pipeStdErrR = NIL_RTPIPE;
                         RTHandleClose(phStdErr);
+                        if (phStdErr)
+                            RTHandleClose(phStdErr);
                     }
-                    RTPipeClose(hStdOutR);
-                    hStdOutR = NIL_RTPIPE;
-                    RTHandleClose(phStdOut);
+                    RTPipeClose(pipeStdOutR);
+                    pipeStdOutR = NIL_RTPIPE;
+                    RTHandleClose(&hStdOut);
+                    if (phStdOut)
+                        RTHandleClose(phStdOut);
                 }
                 RTPipeClose(pThread->pipeStdInW);
+                pThread->pipeStdInW = NIL_RTPIPE;
                 RTHandleClose(phStdIn);
             }
@@ -1343 +1579 @@
         }
 
-        VBoxServiceVerbose(3, "ControlThread: [PID %u]: Cancelling pending waits (client ID=%u)\n",
+        VBoxServiceVerbose(3, "ControlThread: [PID %u]: Cancelling pending host requests (client ID=%u)\n",
                            pThread->uPID, pThread->uClientID);
         rc2 = VbglR3GuestCtrlCancelPendingWaits(pThread->uClientID);
         if (RT_FAILURE(rc2))
         {
-            VBoxServiceError("ControlThread: [PID %u]: Cancelling pending waits failed; rc=%Rrc\n",
+            VBoxServiceError("ControlThread: [PID %u]: Cancelling pending host requests failed; rc=%Rrc\n",
                              pThread->uPID, rc2);
             if (RT_SUCCESS(rc))
@@ -1421 +1657 @@
  * @param   pszPassword                 Password of specified user name (account).
  * @param   uTimeLimitMS                Time limit (in ms) of the process' life time.
- * @param   ppNode                       The thread's list node to insert into the global thread list
+ * @param   ppNode                      The thread's list node to insert into the global thread list
  *                                      on success.
+ *
+ ** @todo Put (all?) these parameters into a struct!
  */
 int VBoxServiceControlThreadStart(uint32_t uClientID, uint32_t uContextID,
@@ -1450 +1688 @@
     {
         static uint32_t s_uCtrlExecThread = 0;
-
+        if (s_uCtrlExecThread++ == UINT32_MAX)
+            s_uCtrlExecThread = 0; /* Wrap around to not let IPRT freak out. */
         rc = RTThreadCreateF(&pThread->Thread, VBoxServiceControlThread,
                              pThread /*pvUser*/, 0 /*cbStack*/,
-                             RTTHREADTYPE_DEFAULT, RTTHREADFLAGS_WAITABLE, "controlexec%u", s_uCtrlExecThread++);
+                             RTTHREADTYPE_DEFAULT, 0 /* Flags */, "gctl%u", s_uCtrlExecThread);
         if (RT_FAILURE(rc))
         {
@@ -1505 +1744 @@
     if (pThread)
     {
-        /* Set request result to some defined state in case
-         * it got cancelled. */
-        pRequest->rc = VERR_CANCELLED;
-
-        /* Set request structure pointer. */
-        pThread->pRequest = pRequest;
-
-        /** @todo To speed up simultaneous guest process handling we could add a worker threads
-         *        or queue in order to wait for the request to happen. Later. */
-
-        /* Wake up guest thrad by sending a wakeup byte to the notification pipe so
-         * that RTPoll unblocks (returns) and we then can do our requested operation. */
-        if (pThread->hNotificationPipeW == NIL_RTPIPE)
-            rc = VERR_BROKEN_PIPE;
-        size_t cbWritten;
-        if (RT_SUCCESS(rc))
-            rc = RTPipeWrite(pThread->hNotificationPipeW, "i", 1, &cbWritten);
-
-        if (   RT_SUCCESS(rc)
-            && cbWritten)
-        {
-            VBoxServiceVerbose(3, "ControlThread: [PID %u]: Waiting for response on enmType=%u, pvData=0x%p, cbData=%u\n",
-                               uPID, pRequest->enmType, pRequest->pvData, pRequest->cbData);
-            /* Wait on the request to get completed (or we are asked to abort/shutdown). */
-            rc = RTSemEventMultiWait(pThread->RequestEvent, RT_INDEFINITE_WAIT);
+        if (ASMAtomicReadBool(&pThread->fShutdown))
+        {
+            rc = VERR_CANCELLED;
+        }
+        else
+        {
+            /* Set request structure pointer. */
+            pThread->pRequest = pRequest;
+
+            /** @todo To speed up simultaneous guest process handling we could add a worker threads
+             *        or queue in order to wait for the request to happen. Later. */
+            /* Wake up guest thrad by sending a wakeup byte to the notification pipe so
+             * that RTPoll unblocks (returns) and we then can do our requested operation. */
+            Assert(pThread->hNotificationPipeW != NIL_RTPIPE);
+            size_t cbWritten;
             if (RT_SUCCESS(rc))
+                rc = RTPipeWrite(pThread->hNotificationPipeW, "i", 1, &cbWritten);
+
+            if (   RT_SUCCESS(rc)
+                && cbWritten)
             {
-                VBoxServiceVerbose(4, "ControlThread: [PID %u]: Performed with rc=%Rrc, cbData=%u\n",
-                                   uPID, pRequest->rc, pRequest->cbData);
-
-                /* Give back overall request result. */
-                rc = pRequest->rc;
-
-                /* Reset the semaphore. */
-                int rc2 = RTSemEventMultiReset(pThread->RequestEvent);
-                if (RT_FAILURE(rc2))
-                    VBoxServiceError("ControlThread: [PID %u]: Unable to reset request event, rc=%Rrc\n",
-                                     uPID, rc2);
+                VBoxServiceVerbose(3, "ControlThread: [PID %u]: Waiting for response on enmType=%u, pvData=0x%p, cbData=%u\n",
+                                   uPID, pRequest->enmType, pRequest->pvData, pRequest->cbData);
+
+                rc = VBoxServiceControlThreadRequestWait(pRequest);
             }
-            else
-                VBoxServiceError("ControlThread: [PID %u]: Wait failed, rc=%Rrc\n",
-                                 uPID, rc);
         }
 

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

@@ -163 +163 @@
 typedef struct VBOXSERVICECTRLREQUEST
 {
+    /** Event semaphore to serialize access. */
+    RTSEMEVENTMULTI            Event;
     /** The request type to handle. */
     VBOXSERVICECTRLREQUESTTYPE enmType;
@@ -172 +174 @@
     void                      *pvData;
     /** The context ID which is required to complete the
-     *  request. */
+     *  request. Not used at the moment. */
     uint32_t                   uCID;
     /** The overall result of the operation. */
@@ -189 +191 @@
     /** The worker thread. */
     RTTHREAD                        Thread;
-    /** Shutdown indicator. */
+    /** Shutdown indicator; will be set when the thread
+      * needs (or is asked) to shutdown. */
     bool volatile                   fShutdown;
     /** Indicator set by the service thread exiting. */
@@ -218 +221 @@
      *  is allowed to run. 0 for indefinite time. */
     uint32_t                        uTimeLimitMS;
-    RTSEMEVENTMULTI                 RequestEvent;
     /** Pointer to the current IPC request being
      *  processed. */
@@ -233 +235 @@
 /** Pointer to thread data. */
 typedef VBOXSERVICECTRLTHREAD *PVBOXSERVICECTRLTHREAD;
-
-/**
- * Request types to perform on a started guest process.
- */
-typedef enum VBOXSERVICECTRLTHREADSTATUS
-{
-    /** Unknown status. Do not use / should not happen. */
-    VBOXSERVICECTRLTHREADSTATUS_UNKNOWN          = 0,
-    VBOXSERVICECTRLTHREADSTATUS_STARTED          = 100,
-    VBOXSERVICECTRLTHREADSTATUS_STOPPED          = 200
-} VBOXSERVICECTRLTHREADSTATUS;
 #endif /* VBOX_WITH_GUEST_CONTROL */
 #ifdef VBOX_WITH_GUEST_PROPS
@@ -345 +336 @@
 extern void         VBoxServiceControlRemoveThread(const PVBOXSERVICECTRLTHREAD pThread);
 /* Guest process functions. */
-extern VBOXSERVICECTRLTHREADSTATUS VBoxServiceControlThreadGetStatus(const PVBOXSERVICECTRLTHREAD pThread);
+extern bool         VBoxServiceControlThreadActive(const PVBOXSERVICECTRLTHREAD pThread);
 extern int          VBoxServiceControlThreadStart(uint32_t uClientID, uint32_t uContext,
                                                   const char *pszCmd, uint32_t uFlags,
@@ -352 +343 @@
                                                   const char *pszUser, const char *pszPassword, uint32_t uTimeLimitMS,
                                                   PRTLISTNODE *ppNode);
+extern int          VBoxServiceControlThreadRequestAlloc(PVBOXSERVICECTRLREQUEST   *ppReq,
+                                                         VBOXSERVICECTRLREQUESTTYPE enmType);
+extern int          VBoxServiceControlThreadRequestAllocEx(PVBOXSERVICECTRLREQUEST    *ppReq,
+                                                           VBOXSERVICECTRLREQUESTTYPE  enmType,
+                                                           void*                       pbData,
+                                                           size_t                      cbData,
+                                                           uint32_t                    uCID);
+extern void         VBoxServiceControlThreadRequestFree(PVBOXSERVICECTRLREQUEST pReq);
 extern int          VBoxServiceControlThreadPerform(uint32_t uPID, PVBOXSERVICECTRLREQUEST pRequest);
 extern int          VBoxServiceControlThreadSignalShutdown(const PVBOXSERVICECTRLTHREAD pThread);