Changeset 45697 in vbox

Timestamp:

Apr 24, 2013 1:30:50 PM (12 years ago)

Author:

vboxsync

Message:

GuestCtrl: Various bugfixes required for test driver to pass.

Location:

trunk/src/VBox

Files:

• Additions/common/VBoxService/VBoxServiceControl.h (modified) (2 diffs)
• Additions/common/VBoxService/VBoxServiceControlProcess.cpp (modified) (29 diffs)
• Additions/common/VBoxService/VBoxServiceControlSession.cpp (modified) (6 diffs)
• HostServices/GuestControl/service.cpp (modified) (5 diffs)
• Main/src-client/GuestProcessImpl.cpp (modified) (15 diffs)
• Main/src-client/GuestSessionImpl.cpp (modified) (1 diff)

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

@@ -97 +97 @@
     /** Event semaphore to serialize access. */
     RTSEMEVENTMULTI            Event;
-    /** Flag indicating if this request is asynchronous or not. */
+    /** Flag indicating if this request is asynchronous or not.
+     *  If asynchronous, this request will be deleted automatically
+     *  on completion. Otherwise the caller has to delete the
+     *  request again. */
     bool                       fAsync;
     /** The request type to handle. */
@@ -313 +316 @@
     /** Node. */
     RTLISTNODE                      Node;
+    /** Number of references using this struct. */
+    uint32_t                        cRefs;
     /** The worker thread. */
     RTTHREAD                        Thread;

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

@@ -47 +47 @@
 *******************************************************************************/
 static int                  gstcntlProcessAssignPID(PVBOXSERVICECTRLPROCESS pThread, uint32_t uPID);
+static int                  gstcntlProcessLock(PVBOXSERVICECTRLPROCESS pProcess);
 static int                  gstcntlProcessRequestCancel(PVBOXSERVICECTRLREQUEST pThread);
 static int                  gstcntlProcessSetupPipe(const char *pszHowTo, int fd, PRTHANDLE ph, PRTHANDLE *pph, PRTPIPE phPipe);
+static int                  gstcntlProcessUnlock(PVBOXSERVICECTRLPROCESS pProcess);
 
 /**
@@ -148 +150 @@
                        pProcess->uPID);
 
+    /* Do *not* set pThread->fShutdown or other stuff here!
+     * The guest thread loop will do that as soon as it processes the quit message. */
+
+    PVBOXSERVICECTRLREQUEST pRequest;
+    int rc = GstCntlProcessRequestAlloc(&pRequest, VBOXSERVICECTRLREQUEST_PROC_TERM);
+    if (RT_SUCCESS(rc))
+    {
+        rc = GstCntlProcessPerform(pProcess, pRequest,
+                                   true /* Async */);
+        if (RT_FAILURE(rc))
+            VBoxServiceVerbose(3, "[PID %RU32]: Sending termination request failed with rc=%Rrc\n",
+                               pProcess->uPID, rc);
+        /* Deletion of pRequest will be done on request completion (asynchronous). */
+    }
+
+    return rc;
+}
+
+
+/**
+ * Releases a previously acquired guest process (decreses the refcount).
+ *
+ * @param   pProcess            Process to unlock.
+ */
+void GstCntlProcessRelease(const PVBOXSERVICECTRLPROCESS pProcess)
+{
+    AssertPtrReturnVoid(pProcess);
+
     int rc = RTCritSectEnter(&pProcess->CritSect);
     if (RT_SUCCESS(rc))
     {
-        if (pProcess->pRequest)
-        {
-            rc = gstcntlProcessRequestCancel(pProcess->pRequest);
-            if (RT_FAILURE(rc))
-                VBoxServiceError("[PID %RU32]: Cancelling current request failed, rc=%Rrc\n",
-                                 pProcess->uPID, rc);
-        }
-
-        /* Do *not* set pThread->fShutdown or other stuff here!
-         * The guest thread loop will do that as soon as it processes the quit message. */
-
-        PVBOXSERVICECTRLREQUEST pRequest;
-        rc = GstCntlProcessRequestAlloc(&pRequest, VBOXSERVICECTRLREQUEST_PROC_TERM);
-        if (RT_SUCCESS(rc))
-        {
-            rc = GstCntlProcessPerform(pProcess, pRequest,
-                                       true /* Async */);
-            if (RT_FAILURE(rc))
-                VBoxServiceVerbose(3, "[PID %RU32]: Sending termination request failed with rc=%Rrc\n",
-                                   pProcess->uPID, rc);
-            /* Deletion of pRequest will be done on request completion. */
-        }
-
-        int rc2 = RTCritSectLeave(&pProcess->CritSect);
-        AssertRC(rc2);
-    }
-
-    return rc;
-}
-
-
-/**
- * Releases (unlocks) a previously locked guest process.
- *
- * @param   pProcess            Process to unlock.
- */
-void GstCntlProcessRelease(const PVBOXSERVICECTRLPROCESS pProcess)
-{
-    AssertPtrReturnVoid(pProcess);
-
-    int rc = RTCritSectLeave(&pProcess->CritSect);
-    AssertRC(rc);
+        Assert(pProcess->cRefs);
+        pProcess->cRefs--;
+        rc = RTCritSectLeave(&pProcess->CritSect);
+        AssertRC(rc);
+    }
 }
 
@@ -269 +262 @@
 
 
+static const char* gstcntlProcessPollHandleToString(uint32_t idPollHnd)
+{
+    switch (idPollHnd)
+    {
+        case VBOXSERVICECTRLPIPEID_UNKNOWN:
+            return "unknown";
+        case VBOXSERVICECTRLPIPEID_STDIN:
+            return "stdin";
+        case VBOXSERVICECTRLPIPEID_STDIN_WRITABLE:
+            return "stdin_writable";
+        case VBOXSERVICECTRLPIPEID_STDOUT:
+            return "stdout";
+        case VBOXSERVICECTRLPIPEID_STDERR:
+            return "stderr";
+        case VBOXSERVICECTRLPIPEID_IPC_NOTIFY:
+            return "ipc_notify";
+        default:
+            break;
+    }
+
+    return "unknown";
+}
+
+
 /**
  * Handle an error event on standard input.
@@ -301 +318 @@
 
 #ifdef DEBUG
-    VBoxServiceVerbose(4, "gstcntlProcessHandleOutputError: fPollEvt=0x%x, idPollHnd=%u\n",
-                       fPollEvt, idPollHnd);
+    VBoxServiceVerbose(4, "gstcntlProcessHandleOutputError: fPollEvt=0x%x, idPollHnd=%s\n",
+                       fPollEvt, gstcntlProcessPollHandleToString(idPollHnd));
 #endif
 
@@ -317 +334 @@
         && cbReadable)
     {
-        VBoxServiceVerbose(3, "gstcntlProcessHandleOutputError: idPollHnd=%RU32 has %zu bytes left, vetoing close\n",
-                           idPollHnd, cbReadable);
+        VBoxServiceVerbose(3, "gstcntlProcessHandleOutputError: idPollHnd=%s has %zu bytes left, vetoing close\n",
+                           gstcntlProcessPollHandleToString(idPollHnd), cbReadable);
 
         /* Veto closing the pipe yet because there's still stuff to read
@@ -326 +343 @@
     }
     else
-        VBoxServiceVerbose(3, "gstcntlProcessHandleOutputError: idPollHnd=%RU32 will be closed\n",
-                           idPollHnd);
+        VBoxServiceVerbose(3, "gstcntlProcessHandleOutputError: idPollHnd=%s will be closed\n",
+                           gstcntlProcessPollHandleToString(idPollHnd));
 
     if (   *phPipeR != NIL_RTPIPE
@@ -354 +371 @@
                                            PRTPIPE phPipeR, uint32_t idPollHnd)
 {
-#if 0
-    VBoxServiceVerbose(4, "GstCntlProcessHandleOutputEvent: fPollEvt=0x%x, idPollHnd=%u\n",
-                       fPollEvt, idPollHnd);
+#ifdef DEBUG_andy
+    VBoxServiceVerbose(4, "GstCntlProcessHandleOutputEvent: fPollEvt=0x%x, idPollHnd=%s\n",
+                       fPollEvt, gstcntlProcessPollHandleToString(idPollHnd));
 #endif
 
@@ -439 +456 @@
 
 
-static int gstcntlProcessRequestHandle(RTPOLLSET hPollSet, uint32_t fPollEvt,
-                                       PRTPIPE phStdInW, PRTPIPE phStdOutR, PRTPIPE phStdErrR,
-                                       PVBOXSERVICECTRLPROCESS pProcess, PVBOXSERVICECTRLREQUEST pRequest)
+static int gstcntlProcessRequestHandle(PVBOXSERVICECTRLPROCESS pProcess, PVBOXSERVICECTRLREQUEST pRequest,
+                                       RTPOLLSET hPollSet, uint32_t fPollEvt,
+                                       PRTPIPE phStdInW, PRTPIPE phStdOutR, PRTPIPE phStdErrR)
 {
     AssertPtrReturn(phStdInW, VERR_INVALID_POINTER);
@@ -530 +547 @@
 
         case VBOXSERVICECTRLREQUEST_PROC_TERM:
-            ASMAtomicXchgBool(&pProcess->fShutdown, true);
+            pProcess->fShutdown = true;
             fDefer = true;
             break;
@@ -605 +622 @@
                        pProcess->uPID, pProcess->StartupInfo.szCmd, pProcess->uContextID,
                        pProcess->StartupInfo.szUser, pProcess->StartupInfo.uTimeLimitMS);
-    VBGLR3GUESTCTRLCMDCTX ctx = { pProcess->uClientID, pProcess->uContextID };
-    rc = VbglR3GuestCtrlProcCbStatus(&ctx,
+    VBGLR3GUESTCTRLCMDCTX ctxStart = { pProcess->uClientID, pProcess->uContextID };
+    rc = VbglR3GuestCtrlProcCbStatus(&ctxStart,
                                      pProcess->uPID, PROC_STS_STARTED, 0 /* u32Flags */,
                                      NULL /* pvData */, 0 /* cbData */);
@@ -613 +630 @@
      * Process input, output, the test pipe and client requests.
      */
-    PVBOXSERVICECTRLREQUEST pReq = NULL;
     while (   RT_SUCCESS(rc)
            && RT_UNLIKELY(!pProcess->fShutdown))
@@ -640 +656 @@
                                                          phStdOutR, idPollHnd);
                     break;
+
                 case VBOXSERVICECTRLPIPEID_STDERR:
                     rc = gstcntlProcessHandleOutputEvent(hPollSet, fPollEvt,
@@ -646 +663 @@
 
                 case VBOXSERVICECTRLPIPEID_IPC_NOTIFY:
-                    pReq = pProcess->pRequest; /** @todo Implement request queue. */
-                    rc = gstcntlProcessRequestHandle(hPollSet, fPollEvt,
-                                                     phStdInW, phStdOutR, phStdErrR,
-                                                     pProcess, pReq);
-                    if (rc == VINF_AIO_TASK_PENDING)
-                        VBoxServiceVerbose(4, "[PID %RU32]: pRequest=%p will be handled deferred\n",
-                                           pProcess->uPID, pReq);
+#ifdef DEBUG_andy
+                    VBoxServiceVerbose(4, "[PID %RU32]: IPC notify\n", pProcess->uPID);
+#endif
+                    rc = gstcntlProcessLock(pProcess);
+                    if (RT_SUCCESS(rc))
+                    {
+                        /** @todo Implement request queue. */
+                        rc = gstcntlProcessRequestHandle(pProcess, pProcess->pRequest,
+                                                         hPollSet, fPollEvt,
+                                                         phStdInW, phStdOutR, phStdErrR);
+                        if (rc != VINF_AIO_TASK_PENDING)
+                        {
+                            pProcess->pRequest = NULL;
+                        }
+                        else
+                            VBoxServiceVerbose(4, "[PID %RU32]: pRequest=%p will be handled deferred\n",
+                                               pProcess->uPID, pProcess->pRequest);
+
+                        rc2 = gstcntlProcessUnlock(pProcess);
+                        AssertRC(rc2);
+                    }
                     break;
 
@@ -664 +695 @@
         }
 #ifdef DEBUG_andy
-        VBoxServiceVerbose(4, "[PID %RU32]: Polling done, pollRc=%Rrc, pollCnt=%u, idPollHnd=%RU32, rc=%Rrc, fProcessAlive=%RTbool, fShutdown=%RTbool\n",
-                           pProcess->uPID, rc2, RTPollSetGetCount(hPollSet), idPollHnd, rc, fProcessAlive, pProcess->fShutdown);
+        VBoxServiceVerbose(4, "[PID %RU32]: Polling done, pollRc=%Rrc, pollCnt=%RU32, idPollHnd=%s, rc=%Rrc, fProcessAlive=%RTbool, fShutdown=%RTbool\n",
+                           pProcess->uPID, rc2, RTPollSetGetCount(hPollSet), gstcntlProcessPollHandleToString(idPollHnd), rc, fProcessAlive, pProcess->fShutdown);
+        VBoxServiceVerbose(4, "[PID %RU32]: stdOut=%s, stdErrR=%s\n",
+                           pProcess->uPID,
+                           *phStdOutR == NIL_RTPIPE ? "closed" : "open",
+                           *phStdErrR == NIL_RTPIPE ? "closed" : "open");
 #endif
         if (RT_UNLIKELY(pProcess->fShutdown))
@@ -677 +712 @@
             rc2 = RTProcWaitNoResume(hProcess, RTPROCWAIT_FLAGS_NOBLOCK, &ProcessStatus);
 #ifdef DEBUG_andy
-            VBoxServiceVerbose(4, "[PID %RU32]: RTProcWaitNoResume=%Rrc, stdOut=%s, stdErrR=%s\n",
-                               pProcess->uPID, rc2,
-                               *phStdOutR == NIL_RTPIPE ? "closed" : "open",
-                               *phStdErrR == NIL_RTPIPE ? "closed" : "open");
+            VBoxServiceVerbose(4, "[PID %RU32]: RTProcWaitNoResume=%Rrc\n",
+                               pProcess->uPID, rc2);
 #endif
             if (RT_SUCCESS_NP(rc2))
@@ -707 +740 @@
             && *phStdOutR == NIL_RTPIPE
             && *phStdErrR == NIL_RTPIPE)
+        {
+            VBoxServiceVerbose(3, "[PID %RU32]: All pipes closed, process not alive anymore, bailing out ...\n",
+                                   pProcess->uPID);
             break;
+        }
 
         /*
@@ -713 +750 @@
          */
         uint32_t cMilliesLeft = RT_INDEFINITE_WAIT;
-        if (pProcess->StartupInfo.uTimeLimitMS != RT_INDEFINITE_WAIT)
+        if (   pProcess->StartupInfo.uTimeLimitMS != RT_INDEFINITE_WAIT
+            && pProcess->StartupInfo.uTimeLimitMS != 0)
         {
             uint64_t u64Now = RTTimeMilliTS();
@@ -746 +784 @@
     }
 
-    rc2 = RTCritSectEnter(&pProcess->CritSect);
-    if (RT_SUCCESS(rc2))
-    {
-        ASMAtomicXchgBool(&pProcess->fShutdown, true);
-
-        rc2 = RTCritSectLeave(&pProcess->CritSect);
-        AssertRC(rc2);
-    }
+    VBoxServiceVerbose(2, "[PID %RU32]: Process loop ended with rc=%Rrc\n",
+                       pProcess->uPID, rc);
+
+    /* Signal that this thread is in progress of shutting down. */
+    ASMAtomicXchgBool(&pProcess->fShutdown, true);
 
     /*
@@ -762 +797 @@
         if (MsProcessKilled == UINT64_MAX)
         {
-            VBoxServiceVerbose(3, "[PID %RU32]: Is still alive and not killed yet\n",
+            VBoxServiceVerbose(2, "[PID %RU32]: Is still alive and not killed yet\n",
                                pProcess->uPID);
 
             MsProcessKilled = RTTimeMilliTS();
-            RTProcTerminate(hProcess);
+            rc2 = RTProcTerminate(hProcess);
+            if (RT_FAILURE(rc))
+                VBoxServiceError("PID %RU32]: Killing process failed with rc=%Rrc\n",
+                                 pProcess->uPID, rc);
             RTThreadSleep(500);
         }
@@ -786 +824 @@
                 VBoxServiceVerbose(4, "[PID %RU32]: Kill attempt %d/10: Trying to terminate ...\n",
                                    pProcess->uPID, i + 1);
-                RTProcTerminate(hProcess);
+                rc2 = RTProcTerminate(hProcess);
+                if (RT_FAILURE(rc))
+                    VBoxServiceError("PID %RU32]: Killing process failed with rc=%Rrc\n",
+                                     pProcess->uPID, rc);
             }
             RTThreadSleep(i >= 5 ? 2000 : 500);
@@ -792 +833 @@
 
         if (fProcessAlive)
-            VBoxServiceVerbose(3, "[PID %RU32]: Could not be killed\n", pProcess->uPID);
-
-        if (   pReq /* Handle deferred termination request. */
-            && pReq->enmType == VBOXSERVICECTRLREQUEST_PROC_TERM)
-        {
-            rc2 = gstcntlProcessRequestComplete(pReq,
-                                                fProcessAlive ? VINF_SUCCESS : VERR_PROCESS_RUNNING);
-            AssertRC(rc2);
-        }
-        else if (pReq)
-            AssertMsgFailed(("Unable to handle unknown deferred request (type: %RU32)\n", pReq->enmType));
+            VBoxServiceError("[PID %RU32]: Could not be killed\n", pProcess->uPID);
+    }
+
+    rc2 = gstcntlProcessLock(pProcess);
+    if (RT_SUCCESS(rc2))
+    {
+        if (pProcess->pRequest)
+        {
+            switch (pProcess->pRequest->enmType)
+            {
+                /* Handle deferred termination request. */
+                case VBOXSERVICECTRLREQUEST_PROC_TERM:
+                    rc2 = gstcntlProcessRequestComplete(pProcess->pRequest,
+                                                        fProcessAlive ? VINF_SUCCESS : VERR_PROCESS_RUNNING);
+                    AssertRC(rc2);
+                    break;
+
+                /* Cancel all others.
+                 ** @todo Clear request queue as soon as it's implemented. */
+                default:
+                    rc2 = gstcntlProcessRequestCancel(pProcess->pRequest);
+                    AssertRC(rc2);
+                    break;
+            }
+
+            pProcess->pRequest = NULL;
+        }
+#ifdef DEBUG
+        else
+            VBoxServiceVerbose(3, "[PID %RU32]: No pending request found\n", pProcess->uPID);
+#endif
+        rc2 = gstcntlProcessUnlock(pProcess);
+        AssertRC(rc2);
     }
 
@@ -877 +940 @@
         if (!(pProcess->StartupInfo.uFlags & EXECUTEPROCESSFLAG_WAIT_START))
         {
-            VBGLR3GUESTCTRLCMDCTX ctx = { pProcess->uClientID, pProcess->uContextID };
-            rc2 = VbglR3GuestCtrlProcCbStatus(&ctx,
+            VBGLR3GUESTCTRLCMDCTX ctxEnd = { pProcess->uClientID, pProcess->uContextID };
+            rc2 = VbglR3GuestCtrlProcCbStatus(&ctxEnd,
                                               pProcess->uPID, uStatus, uFlags,
                                               NULL /* pvData */, 0 /* cbData */);
@@ -890 +953 @@
     }
 
-    VBoxServiceVerbose(3, "[PID %RU32]: Process loop ended with rc=%Rrc\n",
+    VBoxServiceVerbose(3, "[PID %RU32]: Process loop returned with rc=%Rrc\n",
                        pProcess->uPID, rc);
     return rc;
@@ -993 +1056 @@
         return VINF_SUCCESS;
 
-    VBoxServiceVerbose(4, "Cancelling request=0x%p\n", pReq);
+    VBoxServiceVerbose(4, "Cancelling outstanding request=0x%p\n", pReq);
 
     return RTSemEventMultiSignal(pReq->Event);
@@ -1034 +1097 @@
     if (RT_SUCCESS(rc))
     {
-        VBoxServiceVerbose(4, "Performed request with rc=%Rrc, cbData=%u\n",
+        VBoxServiceVerbose(4, "Performed request with rc=%Rrc, cbData=%RU32\n",
                            pReq->rc, pReq->cbData);
 
@@ -1783 +1846 @@
 
 
+static int gstcntlProcessLock(PVBOXSERVICECTRLPROCESS pProcess)
+{
+    AssertPtrReturn(pProcess, VERR_INVALID_POINTER);
+    int rc = RTCritSectEnter(&pProcess->CritSect);
+    AssertRC(rc);
+    return rc;
+}
+
+
 /**
  * Thread main routine for a started process.
@@ -1796 +1868 @@
     AssertPtrReturn(pProcess, VERR_INVALID_POINTER);
     return gstcntlProcessProcessWorker(pProcess);
+}
+
+
+static int gstcntlProcessUnlock(PVBOXSERVICECTRLPROCESS pProcess)
+{
+    AssertPtrReturn(pProcess, VERR_INVALID_POINTER);
+    int rc = RTCritSectLeave(&pProcess->CritSect);
+    AssertRC(rc);
+    return rc;
 }
 
@@ -1884 +1965 @@
     int rc = VINF_SUCCESS;
 
-    AssertMsgReturn(pProcess->pRequest == NULL,
-                    ("Another request still is in progress (%p)\n", pProcess->pRequest),
-                    VERR_ACCESS_DENIED);
-
     if (   ASMAtomicReadBool(&pProcess->fShutdown)
         || ASMAtomicReadBool(&pProcess->fStopped))
@@ -1895 +1972 @@
     else
     {
-        VBoxServiceVerbose(3, "[PID %RU32]: Sending pRequest=%p\n",
-                           pProcess->uPID, pRequest);
-
-        /* Set request structure pointer. */
-        pProcess->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 thread by sending a wakeup byte to the notification pipe so
-         * that RTPoll unblocks (returns) and we then can do our requested operation. */
-        Assert(pProcess->hNotificationPipeW != NIL_RTPIPE);
-        size_t cbWritten = 0;
+        rc = gstcntlProcessLock(pProcess);
         if (RT_SUCCESS(rc))
-            rc = RTPipeWrite(pProcess->hNotificationPipeW, "i", 1, &cbWritten);
-
-        if (RT_SUCCESS(rc))
-        {
-            Assert(cbWritten);
-            if (!fAsync)
+        {
+            AssertMsgReturn(pProcess->pRequest == NULL,
+                            ("Another request still is in progress (%p)\n", pProcess->pRequest),
+                            VERR_ALREADY_EXISTS);
+
+            VBoxServiceVerbose(3, "[PID %RU32]: Sending pRequest=%p\n",
+                               pProcess->uPID, pRequest);
+
+            /* Set request structure pointer. */
+            pProcess->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 thread by sending a wakeup byte to the notification pipe so
+             * that RTPoll unblocks (returns) and we then can do our requested operation. */
+            Assert(pProcess->hNotificationPipeW != NIL_RTPIPE);
+            size_t cbWritten = 0;
+            if (RT_SUCCESS(rc))
+                rc = RTPipeWrite(pProcess->hNotificationPipeW, "i", 1, &cbWritten);
+
+            int rcWait = VINF_SUCCESS;
+            if (RT_SUCCESS(rc))
             {
-                VBoxServiceVerbose(3, "[PID %RU32]: Waiting for response on pRequest=%p, enmType=%u, pvData=0x%p, cbData=%u\n",
-                                   pProcess->uPID, pRequest, pRequest->enmType, pRequest->pvData, pRequest->cbData);
-
-                rc = GstCntlProcessRequestWait(pRequest);
-                if (RT_SUCCESS(rc))
+                Assert(cbWritten);
+                if (!fAsync)
+                {
+                    VBoxServiceVerbose(3, "[PID %RU32]: Waiting for response on pRequest=%p, enmType=%u, pvData=0x%p, cbData=%u\n",
+                                       pProcess->uPID, pRequest, pRequest->enmType, pRequest->pvData, pRequest->cbData);
+
+                    rc = gstcntlProcessUnlock(pProcess);
+                    if (RT_SUCCESS(rc))
+                        rcWait = GstCntlProcessRequestWait(pRequest);
+
+                    /* NULL current request in any case. */
                     pProcess->pRequest = NULL;
+                }
             }
+
+            if (   RT_FAILURE(rc)
+                || fAsync)
+            {
+                int rc2 = gstcntlProcessUnlock(pProcess);
+                AssertRC(rc2);
+            }
+
+            if (RT_SUCCESS(rc))
+                rc = rcWait;
         }
     }

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

@@ -553 +553 @@
                                          &startupInfo.uFlags,
                                          /* Arguments */
-                                         startupInfo.szArgs,     sizeof(startupInfo.szArgs), &startupInfo.uNumArgs,
+                                         startupInfo.szArgs,     sizeof(startupInfo.szArgs),    &startupInfo.uNumArgs,
                                          /* Environment */
-                                         startupInfo.szEnv, &startupInfo.cbEnv, &startupInfo.uNumEnvVars,
+                                         startupInfo.szEnv,      &startupInfo.cbEnv,            &startupInfo.uNumEnvVars,
                                          /* Credentials; for hosts with VBox < 4.3 (protocol version 1).
                                           * For protocl v2 and up the credentials are part of the session
@@ -601 +601 @@
                          rc, pHostCtx->uProtocol, pHostCtx->uNumParms);
 
-        /*
-         * Note: The context ID can be 0 because we mabye weren't able to fetch the command
-         *       from the host. The host in case has to deal with that!
-         */
-        int rc2 = VbglR3GuestCtrlProcCbStatus(pHostCtx, 0 /* PID, invalid */,
-                                              PROC_STS_ERROR, rc,
-                                              NULL /* pvData */, 0 /* cbData */);
-        if (RT_FAILURE(rc2))
-        {
-            VBoxServiceError("Error sending start process status to host, rc=%Rrc\n", rc2);
-            if (RT_SUCCESS(rc))
-                rc = rc2;
+        /* Don't report back if we didn't supply sufficient buffer for getting
+         * the actual command -- we don't have the matching context ID. */
+        if (rc != VERR_TOO_MUCH_DATA)
+        {
+            /*
+             * Note: The context ID can be 0 because we mabye weren't able to fetch the command
+             *       from the host. The host in case has to deal with that!
+             */
+            int rc2 = VbglR3GuestCtrlProcCbStatus(pHostCtx, 0 /* PID, invalid */,
+                                                  PROC_STS_ERROR, rc,
+                                                  NULL /* pvData */, 0 /* cbData */);
+            if (RT_FAILURE(rc2))
+                VBoxServiceError("Error sending start process status to host, rc=%Rrc\n", rc2);
         }
     }
@@ -881 +882 @@
     int rc = VINF_SUCCESS;
 
-    /* Only anonymous sessions (that is, sessions which run with local
+    /** @todo Implement asynchronous handling of all commands to speed up overall
+     *        performance for handling multiple guest processes at once. At the moment
+     *        only one guest process at a time can and will be served. */
+
+    /**
+     * Only anonymous sessions (that is, sessions which run with local
      * service privileges) or forked session processes can do certain
-     * operations. */
+     * operations.
+     */
     bool fImpersonated = (   pSession->uFlags & VBOXSERVICECTRLSESSION_FLAG_FORK
                           || pSession->uFlags & VBOXSERVICECTRLSESSION_FLAG_ANONYMOUS);
@@ -1255 +1262 @@
 
 /**
- * Finds a (formerly) started guest process given by its PID and locks it.
- * Must be unlocked by the caller with GstCntlProcessRelease().
+ * Finds a (formerly) started guest process given by its PID and increses
+ * its reference count. Must be decreased by the caller with GstCntlProcessRelease().
  *
  * @return  PVBOXSERVICECTRLTHREAD      Locked guest process if found, otherwise NULL.
@@ -1277 +1284 @@
                 rc = RTCritSectEnter(&pCurProcess->CritSect);
                 if (RT_SUCCESS(rc))
+                {
+                    pCurProcess->cRefs++;
+                    rc = RTCritSectLeave(&pCurProcess->CritSect);
+                    AssertRC(rc);
+                }
+
+                if (RT_SUCCESS(rc))
                     pProcess = pCurProcess;
                 break;
@@ -1282 +1296 @@
         }
 
-        int rc2 = RTCritSectLeave(&pSession->CritSect);
-        if (RT_SUCCESS(rc))
-            rc = rc2;
+        rc = RTCritSectLeave(&pSession->CritSect);
+        AssertRC(rc);
     }
 

trunk/src/VBox/HostServices/GuestControl/service.cpp

@@ -332 +332 @@
                                 rc = VERR_INVALID_PARAMETER;
 
-                            if (paDstParms[i].u.pointer.size < mpParms[i].u.pointer.size)
+                            if (   RT_SUCCESS(rc)
+                                && paDstParms[i].u.pointer.size < mpParms[i].u.pointer.size)
                                 rc = VERR_BUFFER_OVERFLOW;
 
@@ -525 +526 @@
         mHostCmdRc    = VINF_SUCCESS;
         mHostCmdTries = 0;
+        mPeekCount    = 0;
 
         return nextItem;
@@ -630 +632 @@
         int rc = VINF_SUCCESS;
 
-        LogFlowFunc(("[Client %RU32] pConnection=%p, mHostCmdRc=%Rrc, mHostCmdTries=%RU32\n",
-                      mID, pConnection, mHostCmdRc, mHostCmdTries));
+        LogFlowFunc(("[Client %RU32] pConnection=%p, mHostCmdRc=%Rrc, mHostCmdTries=%RU32, mPeekCount=%RU32\n",
+                      mID, pConnection, mHostCmdRc, mHostCmdTries, mPeekCount));
 
         mHostCmdRc = SendReply(pConnection, pHostCmd);
@@ -644 +646 @@
             /*
              * If the client understood the message but supplied too little buffer space
-             * don't send this message again and drop it after 3 unsuccessful attempts.
-             * The host then should take care of next actions (maybe retry it with a smaller buffer).
+             * don't send this message again and drop it after 6 unsuccessful attempts.
+             *
+             * Note: Due to legacy reasons this the retry counter has to be even because on
+             *       every peek there will be the actual command retrieval from the client side.
+             *       To not get the actual command if the client actually only wants to peek for
+             *       the next command, there needs to be two rounds per try, e.g. 3 rounds = 6 tries.
+             *
+             ** @todo Fix the mess stated above. GUEST_MSG_WAIT should be become GUEST_MSG_PEEK, *only*
+             *        (and every time) returning the next upcoming host command (if any, blocking). Then
+             *        it's up to the client what to do next, either peeking again or getting the actual
+             *        host command via an own GUEST_ type message.
              */
             if (mHostCmdRc == VERR_TOO_MUCH_DATA)
             {
-                if (mHostCmdTries >= 3)
+                if (mHostCmdTries == 6)
                     fRemove = true;
             }
@@ -777 +788 @@
         else
         {
+            /* If this is the very first peek, make sure to *always* give back the peeking answer
+             * instead of the actual command, even if this command would fit into the current
+             * connection buffer. */
             if (!mPeekCount)
             {

trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp

@@ -736 +736 @@
 
     com::SafeArray<BYTE> data((size_t)dataCb.cbData);
-    data.initFrom((BYTE*)dataCb.pvData, dataCb.cbData);
+    if (dataCb.cbData)
+        data.initFrom((BYTE*)dataCb.pvData, dataCb.cbData);
 
     fireGuestProcessOutputEvent(mEventSource, mSession, this,
@@ -755 +756 @@
     /* pcbRead is optional. */
 
-    /** @todo Validate uHandle. */
-
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
-    if (mData.mStatus != ProcessStatus_Started)
+    if (   mData.mStatus != ProcessStatus_Started
+        /* Skip reading if the process wasn't started with the appropriate
+         * flags. */
+        || (   (   uHandle == OUTPUT_HANDLE_ID_STDOUT
+                || uHandle == OUTPUT_HANDLE_ID_STDOUT_DEPRECATED)
+            && !(mData.mProcess.mFlags & ProcessCreateFlag_WaitForStdOut))
+        || (   uHandle == OUTPUT_HANDLE_ID_STDERR
+            && !(mData.mProcess.mFlags & ProcessCreateFlag_WaitForStdErr))
+       )
     {
         if (pcbRead)
@@ -780 +787 @@
         paParms[i++].setUInt32(0 /* Flags, none set yet. */);
 
+        alock.release(); /* Drop the write lock before sending. */
+
         vrc = sendCommand(HOST_EXEC_GET_OUTPUT, i, paParms);
     }
 
     if (RT_SUCCESS(vrc))
-    {
-        alock.release(); /* Drop the write lock before waiting. */
-
         vrc = waitForOutput(uHandle, uTimeoutMS,
                             pvData, cbData, pcbRead);
-    }
 
     LogFlowFuncLeaveRC(vrc);
@@ -904 +909 @@
         if (RT_SUCCESS(vrc))
             vrc = mData.mProcess.mEnvironment.BuildEnvironmentBlock(&pvEnv, &cbEnv, NULL /* cEnv */);
+
+        uint32_t uTimeoutMS = mData.mProcess.mTimeoutMS;
 
         if (RT_SUCCESS(vrc))
@@ -939 +946 @@
                 paParms[i++].setUInt32(UINT32_MAX /* Infinite timeout */);
             else
-                paParms[i++].setUInt32(mData.mProcess.mTimeoutMS);
+                paParms[i++].setUInt32(uTimeoutMS);
             if (uProtocol >= 2)
             {
@@ -950 +957 @@
             }
 
+            alock.release(); /* Drop the write lock before sending. */
+
             /* Note: Don't hold the write lock in here. */
             vrc = sendCommand(HOST_EXEC_CMD, i, paParms);
@@ -963 +972 @@
             RTStrFree(pszArgs);
 
-        uint32_t uTimeoutMS = mData.mProcess.mTimeoutMS;
-
-        /* Drop the write lock again before waiting. */
-        alock.release();
-
         if (RT_SUCCESS(vrc))
         {
-            vrc = waitForStatusChange(ProcessWaitForFlag_Start, 30 * 1000 /* Timeout */,
+            vrc = waitForStatusChange(ProcessWaitForFlag_Start, uTimeoutMS,
                                       NULL /* Process status */, pGuestRc);
         }
@@ -1055 +1059 @@
         paParms[i++].setUInt32(mData.mPID);
 
+        alock.release(); /* Drop the write lock before sending. */
+
         vrc = sendCommand(HOST_EXEC_TERMINATE, i, paParms);
     }
 
     if (RT_SUCCESS(vrc))
-    {
-        alock.release(); /* Drop the write lock before waiting. */
-
         vrc = waitForStatusChange(ProcessWaitForFlag_Terminate,
                                   30 * 1000 /* 30s timeout */,
                                   NULL /* ProcessStatus */, pGuestRc);
-    }
-
     LogFlowFuncLeaveRC(vrc);
     return vrc;
@@ -1267 +1268 @@
     if (SUCCEEDED(hr))
     {
-        LogFlowThisFunc(("Waiting for guest file event(s) (timeout=%RU32ms, %zu events) ...\n",
+        LogFlowThisFunc(("Waiting for guest process event(s) (timeout=%RU32ms, %zu events) ...\n",
                          uTimeoutMS, arrEventTypes.size()));
 
@@ -1427 +1428 @@
                     ComAssertComRC(hr);
                     size_t cbRead = data.size();
-                    if (   cbRead
-                        && cbRead <= cbData)
+                    if (cbRead)
                     {
-                        memcpy(pvData, data.raw(), data.size());
+                        if (cbRead <= cbData)
+                        {
+                            /* Copy data from event into our buffer. */
+                            memcpy(pvData, data.raw(), data.size());
+                        }
+                        else
+                            vrc = VERR_BUFFER_OVERFLOW;
                     }
-                    else
-                        vrc = VERR_BUFFER_OVERFLOW;
                 }
                 if (pcbRead)
                 {
-                    hr = pProcessEvent->COMGETTER(Processed)((ULONG*)pcbRead);
+                    ULONG cbRead;
+                    hr = pProcessEvent->COMGETTER(Processed)(&cbRead);
                     ComAssertComRC(hr);
+                    *pcbRead = (uint32_t)cbRead;
                 }
 
@@ -1525 +1531 @@
         paParms[i++].setUInt32(cbData);
 
+        alock.release(); /* Drop the write lock before sending. */
+
         vrc = sendCommand(HOST_EXEC_SET_INPUT, i, paParms);
     }
@@ -1530 +1538 @@
     if (RT_SUCCESS(vrc))
     {
-        alock.release(); /* Drop the write lock before waiting. */
-
         ProcessInputStatus_T inputStatus;
         uint32_t cbProcessed;
@@ -1593 +1599 @@
     }
 
-    LogFlowThisFunc(("rc=%Rrc, cbRead=%RU64\n", vrc, cbRead));
+    LogFlowThisFunc(("rc=%Rrc, cbRead=%RU32\n", vrc, cbRead));
 
     LogFlowFuncLeaveRC(vrc);
@@ -1736 +1742 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
+    com::SafeArray<BYTE> data(ComSafeArrayInArg(aData));
+
     HRESULT hr = S_OK;
 
-    com::SafeArray<BYTE> data(ComSafeArrayInArg(aData)); int guestRc;
-    int vrc = writeData(aHandle, aFlags, data.raw(), data.size(), aTimeoutMS, (uint32_t*)aWritten, &guestRc);
+    uint32_t cbWritten; int guestRc;
+    int vrc = writeData(aHandle, aFlags, data.raw(), data.size(), aTimeoutMS, &cbWritten, &guestRc);
     if (RT_FAILURE(vrc))
     {
@@ -1756 +1764 @@
     }
 
-    LogFlowThisFunc(("rc=%Rrc, aWritten=%RU32\n", vrc, aWritten));
+    LogFlowThisFunc(("rc=%Rrc, aWritten=%RU32\n", vrc, cbWritten));
+
+    *aWritten = (ULONG)cbWritten;
 
     LogFlowFuncLeaveRC(vrc);

trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp

@@ -1358 +1358 @@
                          pCurProc->getObjectID(), mData.mSession.mID, uPID, mData.mProcesses.size() - 1, mData.mNumObjects - 1));
 
-            LogFlowFunc(("1\n"));
             mData.mProcesses.erase(itProcs);
             mData.mNumObjects--;
 
-            LogFlowFunc(("2\n"));
             fireGuestProcessRegisteredEvent(mEventSource, this /* Session */, NULL /* Process */,
                                             uPID, false /* Process unregistered */);
-            LogFlowFunc(("3\n"));
             rc = VINF_SUCCESS;
             break;
         }
 
-        LogFlowFunc(("4\n"));
         itProcs++;
     }