Changeset 42272 in vbox for trunk/src/VBox/Main

Timestamp:

Jul 20, 2012 2:42:40 PM (13 years ago)

Author:

vboxsync

Message:

Guest Control 2.0: Update.

Location:

trunk/src/VBox/Main

Files:

• include/GuestCtrlImplPrivate.h (modified) (5 diffs)
• include/GuestProcessImpl.h (modified) (2 diffs)
• include/GuestSessionImpl.h (modified) (1 diff)
• src-client/GuestCtrlImpl.cpp (modified) (7 diffs)
• src-client/GuestCtrlPrivate.cpp (modified) (5 diffs)
• src-client/GuestProcessImpl.cpp (modified) (28 diffs)
• src-client/GuestSessionImpl.cpp (modified) (13 diffs)

trunk/src/VBox/Main/include/GuestCtrlImplPrivate.h

@@ -35 +35 @@
 using namespace guestControl;
 #endif
-
 
 /** Maximum number of guest sessions a VM can have. */
@@ -61 +60 @@
     ((uContextID) & 0xffff)
 
-
+/** Vector holding a process' CPU affinity. */
 typedef std::vector <LONG> ProcessAffinity;
+/** Vector holding process startup arguments. */
 typedef std::vector <Utf8Str> ProcessArguments;
 
@@ -94 +94 @@
 
     Utf8Str GetMessage(void) { return mMessage; }
+
+    int GetResultCode(void) { return mRC; }
 
     eVBoxGuestCtrlCallbackType GetType(void) { return mType; }
@@ -107 +109 @@
     /** Was the callback canceled? */
     bool                        fCanceled;
+    /** Did the callback complete? */
+    bool                        fCompleted;
     /** Pointer to user-supplied data. */
     void                       *pvData;
@@ -124 +128 @@
  * Simple structure mantaining guest credentials.
  */
-class GuestCredentials
-{
-public:
-
-
-public:
-
+struct GuestCredentials
+{
     Utf8Str                     mUser;
     Utf8Str                     mPassword;

trunk/src/VBox/Main/include/GuestProcessImpl.h

@@ -71 +71 @@
     int callbackAdd(GuestCtrlCallback *pCallback, ULONG *puContextID);
     int callbackDispatcher(uint32_t uContextID, uint32_t uFunction, void *pvData, size_t cbData);
-    bool callbackExists(ULONG uContextID);
+    inline bool callbackExists(ULONG uContextID);
     bool isReady(void);
     ULONG getPID(void) { return mData.mPID; }
@@ -81 +81 @@
     int readData(ULONG uHandle, ULONG uSize, ULONG uTimeoutMS, BYTE *pbData, size_t cbData);
     int sendCommand(uint32_t uFunction, uint32_t uParms, PVBOXHGCMSVCPARM paParms);
-    int startProcess(void);
+    int signalWaiters(int rc, const Utf8Str strMessage = "");
+    int startProcess(int *pRC = NULL, Utf8Str *pstrMessage = NULL);
     static DECLCALLBACK(int) startProcessThread(RTTHREAD Thread, void *pvUser);
     int terminateProcess(void);

trunk/src/VBox/Main/include/GuestSessionImpl.h

@@ -126 +126 @@
     const GuestEnvironment &getEnvironment(void);
     int                     processClose(ComObjPtr<GuestProcess> pProcess);
-    int                     processCreateExInteral(GuestProcessInfo &aProcInfo, IGuestProcess **aProcess);
+    int                     processCreateExInteral(const GuestProcessInfo &aProcInfo, IGuestProcess **aProcess);
     inline bool             processExists(ULONG uProcessID, ComObjPtr<GuestProcess> *pProcess);
     inline int              processGetByPID(ULONG uPID, ComObjPtr<GuestProcess> *pProcess);

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

@@ -700 +700 @@
     AssertPtr(pHeader);
 
+#ifdef DEBUG
+    LogFlowFunc(("uSession=%RU32, uProcess=%RU32, uCount=%RU32\n",
+                 VBOX_GUESTCTRL_CONTEXTID_GET_SESSION(pHeader->u32ContextID),
+                 VBOX_GUESTCTRL_CONTEXTID_GET_PROCESS(pHeader->u32ContextID),
+                 VBOX_GUESTCTRL_CONTEXTID_GET_COUNT(pHeader->u32ContextID)));
+#endif
     int rc = pGuest->dispatchToSession(pHeader->u32ContextID, u32Function, pvParms, cbParms);
 
@@ -2675 +2681 @@
 int Guest::dispatchToSession(uint32_t uContextID, uint32_t uFunction, void *pvData, size_t cbData)
 {
+    LogFlowFuncEnter();
+
     AssertPtrReturn(pvData, VERR_INVALID_POINTER);
     AssertReturn(cbData, VERR_INVALID_PARAMETER);
@@ -2680 +2688 @@
     AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
 
+    int rc;
     GuestSessions::const_iterator itSession
         = mData.mGuestSessions.find(VBOX_GUESTCTRL_CONTEXTID_GET_SESSION(uContextID));
@@ -2688 +2697 @@
 
         alock.release();
-        return pSession->dispatchToProcess(uContextID, uFunction, pvData, cbData);
-    }
-    return VERR_NOT_FOUND;
+        rc = pSession->dispatchToProcess(uContextID, uFunction, pvData, cbData);
+    }
+    else
+        rc = VERR_NOT_FOUND;
+
+    LogFlowFuncLeaveRC(rc);
+    return rc;
 }
 
@@ -2718 +2731 @@
 
     int rc = VERR_MAX_PROCS_REACHED;
+    if (mData.mGuestSessions.size() >= VBOX_GUESTCTRL_MAX_SESSIONS)
+        return rc;
+
     try
     {
@@ -2726 +2742 @@
         for (;;)
         {
-            /* Is the context ID already used?  Try next ID ... */
-            if (!sessionExists(++uNewSessionID))
+            /* Is the context ID already used? */
+            if (!sessionExists(uNewSessionID))
             {
                 rc = VINF_SUCCESS;
                 break;
             }
+            uNewSessionID++;
 
             if (++uTries == UINT32_MAX)
@@ -2763 +2780 @@
 inline bool Guest::sessionExists(uint32_t uSessionID)
 {
-    AssertReturn(uSessionID, false);
-
     GuestSessions::const_iterator itSessions = mData.mGuestSessions.find(uSessionID);
     return (itSessions == mData.mGuestSessions.end()) ? false : true;

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

@@ -37 +37 @@
       uFlags(0),
       fCanceled(false),
+      fCompleted(false),
       pvData(NULL),
       cbData(0),
@@ -47 +48 @@
       uFlags(0),
       fCanceled(false),
+      fCompleted(false),
       pvData(NULL),
       cbData(0),
@@ -62 +64 @@
 int GuestCtrlCallback::Cancel(void)
 {
+    if (ASMAtomicReadBool(&fCompleted))
+        return VINF_SUCCESS;
     if (!ASMAtomicReadBool(&fCanceled))
     {
-        int rc = RTSemEventSignal(hEventSem);
+        int rc = hEventSem != NIL_RTSEMEVENT
+               ? RTSemEventSignal(hEventSem) : VINF_SUCCESS;
         if (RT_SUCCESS(rc))
             ASMAtomicXchgBool(&fCanceled, true);
@@ -128 +133 @@
 void GuestCtrlCallback::Destroy(void)
 {
+    int rc = Cancel();
+    AssertRC(rc);
+
     mType = VBOXGUESTCTRLCALLBACKTYPE_UNKNOWN;
     if (pvData)
@@ -152 +160 @@
     if (!uTimeoutMS)
         msInterval = RT_INDEFINITE_WAIT;
-    int rc = RTSemEventWait(hEventSem, msInterval);
-    if (RT_SUCCESS(rc))
-    {
-        /* Assign overall callback result. */
-        rc = mRC;
-    }
-
-    return rc;
+    return RTSemEventWait(hEventSem, msInterval);
 }
 

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

@@ -68 +68 @@
 HRESULT GuestProcess::FinalConstruct(void)
 {
-    LogFlowThisFunc(("\n"));
+    LogFlowFuncEnter();
 
     mData.mExitCode = 0;
@@ -81 +81 @@
     mData.mWaitEvent = NIL_RTSEMEVENT;
 
-    return BaseFinalConstruct();
+    HRESULT hr = BaseFinalConstruct();
+    LogFlowFuncLeaveRC(hr);
+    return hr;
 }
 
@@ -108 +110 @@
     mData.mParent = aSession;
     mData.mProcessID = aProcessID;
+    mData.mProcess = aProcInfo;
+
     mData.mStatus = ProcessStatus_Starting;
     /* Everything else will be set by the actual starting routine. */
@@ -362 +366 @@
 int GuestProcess::callbackDispatcher(uint32_t uContextID, uint32_t uFunction, void *pvData, size_t cbData)
 {
+/*    LogFlowFunc(("uPID=%RU32, uContextID=%RU32, uFunction=%RU32, pvData=%p, cbData=%z\n",
+                 mData.mPID, uContextID, uFunction, pvData, cbData));*/
+
     AssertPtrReturn(pvData, VERR_INVALID_POINTER);
     AssertReturn(cbData, VERR_INVALID_PARAMETER);
 
-    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+    int rc;
     GuestCtrlCallbacks::const_iterator it
         = mData.mCallbacks.find(VBOX_GUESTCTRL_CONTEXTID_GET_COUNT(uContextID));
@@ -373 +381 @@
         GuestCtrlCallback *pCallback = it->second;
         AssertPtr(pCallback);
-
-        int rc;
 
         switch (uFunction)
@@ -429 +435 @@
                 break;
         }
-
-        return rc;
-    }
-
-    return VERR_NOT_FOUND;
-}
-
-bool GuestProcess::callbackExists(ULONG uContextID)
-{
-    AssertReturn(uContextID, false);
-
+    }
+    else
+        rc = VERR_NOT_FOUND;
+
+    //LogFlowFuncLeaveRC(rc);
+    return rc;
+}
+
+inline bool GuestProcess::callbackExists(ULONG uContextID)
+{
     GuestCtrlCallbacks::const_iterator it = mData.mCallbacks.find(uContextID);
     return (it == mData.mCallbacks.end()) ? false : true;
@@ -459 +464 @@
 int GuestProcess::onGuestDisconnected(GuestCtrlCallback *pCallback, PCALLBACKDATACLIENTDISCONNECTED pData)
 {
-    LogFlowFunc(("uPID=%RU32, pCallback=%p, pData=%p", mData.mPID, pCallback, pData));
+    LogFlowFunc(("uPID=%RU32, pCallback=%p, pData=%p\n", mData.mPID, pCallback, pData));
 
     /* First, signal callback in every case. */
     pCallback->Signal();
 
-    Assert(mData.mWaitEvent != NIL_RTSEMEVENT);
-    int rc = RTSemEventSignal(mData.mWaitEvent);
+    /* Signal in any case. */
+    int rc = signalWaiters(VERR_CANCELLED, tr("The guest got disconnected"));
     AssertRC(rc);
 
@@ -486 +491 @@
     /* Then do the WaitFor signalling stuff. */
     if (mData.mWaitFlags & ProcessWaitForFlag_StdIn)
-    {
-        Assert(mData.mWaitEvent != NIL_RTSEMEVENT);
-        rc = RTSemEventSignal(mData.mWaitEvent);
-        AssertRC(rc);
-    }
+        rc = signalWaiters(VINF_SUCCESS);
 
     LogFlowFuncLeaveRC(rc);
@@ -504 +505 @@
 
     /* Get data from the callback payload. */
-    mData.mStatus = (ProcessStatus_T)pData->u32Status;
     if (mData.mPID)
         Assert(mData.mPID == pData->u32PID);
 
-    Utf8Str strCallbackMsg;
+    Utf8Str callbackMsg;
+    int callbackRC = VINF_SUCCESS;
+
     bool fSignal = false;
     switch (pData->u32Status)
@@ -515 +517 @@
             fSignal = (   (mData.mWaitFlags & ProcessWaitForFlag_Start)
                        || (mData.mWaitFlags & ProcessWaitForFlag_Status));
+
+            mData.mStatus = ProcessStatus_Started;
             mData.mPID = pData->u32PID;
             mData.mStarted = true;
 
-            rc = pCallback->Signal(VINF_SUCCESS,
-                                   Utf8StrFmt(tr("Guest process \"%s\" was started (PID %RU32)"),
-                                              mData.mProcess.mCommand.c_str(), mData.mPID));
+            callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" was started (PID %RU32)"),
+                                     mData.mProcess.mCommand.c_str(), mData.mPID);
+            LogRel((callbackMsg.c_str()));
             break;
 
         case PROC_STS_TEN:
         {
-            Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated normally (exit code: %RU32)"),
-                                              pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags);
-            LogRel((strErrDetail.c_str()));
-            rc = pCallback->Signal(VINF_SUCCESS, strErrDetail);
+            mData.mStatus = ProcessStatus_TerminatedNormally;
+
+            callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated normally (exit code: %d)"),
+                                     mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags);
+            LogRel((callbackMsg.c_str()));
             break;
         }
@@ -534 +539 @@
         case PROC_STS_TES:
         {
-            Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated through signal (exit code: %RU32)"),
-                                              pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags);
-            LogRel((strErrDetail.c_str()));
-            rc = pCallback->Signal(VERR_GENERAL_FAILURE /** @todo */, strErrDetail);
+            mData.mStatus = ProcessStatus_TerminatedSignal;
+
+            callbackRC = VERR_INTERRUPTED;
+            callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated through signal (exit code: %d)"),
+                                     mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags);
+            LogRel((callbackMsg.c_str()));
             break;
         }
@@ -543 +550 @@
         case PROC_STS_TEA:
         {
-            Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated abnormally (exit code: %RU32)"),
-                                              pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags);
-            LogRel((strErrDetail.c_str()));
-            rc = pCallback->Signal(VERR_BROKEN_PIPE, strErrDetail);
+            mData.mStatus = ProcessStatus_TerminatedAbnormally;
+
+            callbackRC = VERR_BROKEN_PIPE;
+            callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated abnormally (exit code: %d)"),
+                                     mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags);
+            LogRel((callbackMsg.c_str()));
             break;
         }
@@ -552 +561 @@
         case PROC_STS_TOK:
         {
-            Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) timed out and was killed"),
-                                              pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID);
-            LogRel((strErrDetail.c_str()));
-            rc = pCallback->Signal(VERR_TIMEOUT, strErrDetail);
+            mData.mStatus = ProcessStatus_TimedOutKilled;
+
+            callbackRC = VERR_TIMEOUT;
+            callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) timed out and was killed"),
+                                     mData.mProcess.mCommand.c_str(), mData.mPID);
+            LogRel((callbackMsg.c_str()));
             break;
         }
@@ -561 +572 @@
         case PROC_STS_TOA:
         {
-            Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) timed out and could not be killed\n"),
-                                              pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID);
-            LogRel((strErrDetail.c_str()));
-            rc = pCallback->Signal(VERR_TIMEOUT, strErrDetail);
+            mData.mStatus = ProcessStatus_TimedOutAbnormally;
+
+            callbackRC = VERR_TIMEOUT;
+            callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) timed out and could not be killed\n"),
+                                     mData.mProcess.mCommand.c_str(), mData.mPID);
+            LogRel((callbackMsg.c_str()));
             break;
         }
@@ -570 +583 @@
         case PROC_STS_DWN:
         {
-            Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) was killed because system is shutting down\n"),
-                                              mData.mProcess.mCommand.c_str(), mData.mPID);
-            LogRel((strErrDetail.c_str()));
+            mData.mStatus = ProcessStatus_Down;
+
             /*
              * If mFlags has CreateProcessFlag_IgnoreOrphanedProcesses set, we don't report an error to
@@ -580 +592 @@
              * In this case mFlags contains the actual execution flags reached in via Guest::ExecuteProcess().
              */
-            rc = pCallback->Signal(  mData.mProcess.mFlags & ProcessCreateFlag_IgnoreOrphanedProcesses
-                                   ? VINF_SUCCESS : VERR_CANCELLED, strErrDetail);
+            callbackRC = mData.mProcess.mFlags & ProcessCreateFlag_IgnoreOrphanedProcesses
+                       ? VINF_SUCCESS : VERR_OBJECT_DESTROYED;
+            callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) was killed because system is shutting down\n"),
+                                     mData.mProcess.mCommand.c_str(), mData.mPID);
+            LogRel((callbackMsg.c_str()));
             break;
         }
@@ -587 +602 @@
         case PROC_STS_ERROR:
         {
+            mData.mStatus = ProcessStatus_Error;
+
+            callbackRC = pData->u32Flags;
+            callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" could not be started: ", mData.mProcess.mCommand.c_str()));
+
             /* Note: It's not required that the process
              * has been started before. */
-            Utf8Str strErrDetail;
             if (pData->u32PID)
             {
-                strErrDetail = Utf8StrFmt(tr("Guest process (PID %RU32) could not be started because of rc=%Rrc"),
-                                          pData->u32PID, pData->u32Flags);
+                callbackMsg += Utf8StrFmt(tr("Error rc=%Rrc occured"), pData->u32Flags);
             }
             else
             {
-                switch (pData->u32Flags) /* u32Flags member contains the IPRT error code from guest side. */
+                switch (callbackRC) /* callbackRC contains the IPRT error code from guest side. */
                 {
                     case VERR_FILE_NOT_FOUND: /* This is the most likely error. */
-                        strErrDetail = Utf8StrFmt(tr("The specified file was not found on guest"));
+                        callbackMsg += Utf8StrFmt(tr("The specified file was not found on guest"));
                         break;
 
                     case VERR_PATH_NOT_FOUND:
-                        strErrDetail = Utf8StrFmt(tr("Could not resolve path to specified file was not found on guest"));
+                        callbackMsg += Utf8StrFmt(tr("Could not resolve path to specified file was not found on guest"));
                         break;
 
                     case VERR_BAD_EXE_FORMAT:
-                        strErrDetail = Utf8StrFmt(tr("The specified file is not an executable format on guest"));
+                        callbackMsg += Utf8StrFmt(tr("The specified file is not an executable format on guest"));
                         break;
 
                     case VERR_AUTHENTICATION_FAILURE:
-                        strErrDetail = Utf8StrFmt(tr("The specified user was not able to logon on guest"));
+                        callbackMsg += Utf8StrFmt(tr("The specified user was not able to logon on guest"));
                         break;
 
                     case VERR_TIMEOUT:
-                        strErrDetail = Utf8StrFmt(tr("The guest did not respond within time"));
+                        callbackMsg += Utf8StrFmt(tr("The guest did not respond within time"));
                         break;
 
                     case VERR_CANCELLED:
-                        strErrDetail = Utf8StrFmt(tr("The execution operation was canceled"));
+                        callbackMsg += Utf8StrFmt(tr("The execution operation was canceled"));
                         break;
 
                     case VERR_PERMISSION_DENIED:
-                        strErrDetail = Utf8StrFmt(tr("Invalid user/password credentials"));
+                        callbackMsg += Utf8StrFmt(tr("Invalid user/password credentials"));
                         break;
 
                     case VERR_MAX_PROCS_REACHED:
-                        strErrDetail = Utf8StrFmt(tr("Guest process could not be started because maximum number of parallel guest processes has been reached"));
+                        callbackMsg += Utf8StrFmt(tr("Maximum number of parallel guest processes has been reached"));
                         break;
 
                     default:
-                        strErrDetail = Utf8StrFmt(tr("Guest process reported error %Rrc"),
-                                                  pData->u32Flags);
+                        callbackMsg += Utf8StrFmt(tr("Reported error %Rrc"), callbackRC);
                         break;
                 }
-
-                Assert(!strErrDetail.isEmpty());
-                rc = pCallback->Signal(pData->u32Flags /* rc from guest. */, strErrDetail);
             }
-            fSignal = mData.mWaitFlags & ProcessWaitForFlag_Status;
+
+            LogRel((callbackMsg.c_str()));
             break;
         }
@@ -646 +661 @@
         case PROC_STS_UNDEFINED:
         default:
-            rc = pCallback->Signal(VERR_NOT_SUPPORTED,
-                                   Utf8StrFmt(tr("Got unsupported process status %RU32, skipping"), pData->u32Status));
+
+            /* Silently skip this request. */
             fSignal = true; /* Signal in any case. */
             break;
     }
+
+    /*
+     * Now do the signalling stuff.
+     */
+    rc = pCallback->Signal(callbackRC, callbackMsg);
 
     if (!fSignal)
@@ -656 +676 @@
     if (fSignal)
     {
-        Assert(mData.mWaitEvent != NIL_RTSEMEVENT);
-        int rc2 = RTSemEventSignal(mData.mWaitEvent);
+        int rc2 = signalWaiters(callbackRC, callbackMsg);
         if (RT_SUCCESS(rc))
             rc = rc2;
@@ -748 +767 @@
 }
 
-int GuestProcess::startProcess(void)
-{
+int GuestProcess::signalWaiters(int rc, const Utf8Str strMessage)
+{
+    LogFlowFunc(("rc=%Rrc, strMessage=%s, mWaiting=%RTbool, mWaitEvent=%p\n",
+                 rc, strMessage.c_str(), mData.mWaiting, mData.mWaitEvent));
+
+    /* Note: No locking here -- already done in the callback dispatcher. */
+
+    /* We have to set the error information here because the waiters are public
+     * Main clients which rely on it. */
+    if (RT_FAILURE(rc))
+    {
+        HRESULT hr = setError(VBOX_E_IPRT_ERROR, strMessage.c_str());
+        ComAssertRC(hr);
+    }
+
+    int rc2 = VINF_SUCCESS;
+    if (   mData.mWaiting
+        && mData.mWaitEvent != NIL_RTSEMEVENT)
+    {
+        rc2 = RTSemEventSignal(mData.mWaitEvent);
+        AssertRC(rc2);
+    }
+
+    LogFlowFuncLeaveRC(rc2);
+    return rc2;
+}
+
+int GuestProcess::startProcess(int *pRC, Utf8Str *pstrMessage)
+{
+    /* Parameters are optional. */
+
     LogFlowFunc(("aCmd=%s, aTimeoutMS=%RU32, fFlags=%x\n",
                  mData.mProcess.mCommand.c_str(), mData.mProcess.mTimeoutMS, mData.mProcess.mFlags));
@@ -757 +805 @@
     int rc;
     ULONG uContextID;
-    GuestCtrlCallback *pCallbackStart = (GuestCtrlCallback *)RTMemAlloc(sizeof(GuestCtrlCallback));
+    GuestCtrlCallback *pCallbackStart = new GuestCtrlCallback();
     if (!pCallbackStart)
         return VERR_NO_MEMORY;
@@ -774 +822 @@
     if (RT_SUCCESS(rc))
     {
-        Assert(uContextID);
-
         ComObjPtr<GuestSession> pSession(mData.mParent);
         Assert(!pSession.isNull());
@@ -844 +890 @@
              */
             rc = pCallbackStart->Wait(mData.mProcess.mTimeoutMS);
-        }
-    }
-
-    LogFlowFuncLeave();
+            if (RT_SUCCESS(rc)) /* Wait was successful, check for supplied information. */
+            {
+                if (pRC)
+                    *pRC = pCallbackStart->GetResultCode();
+                if (pstrMessage)
+                    *pstrMessage = pCallbackStart->GetMessage();
+            }
+        }
+    }
+
+    if (pCallbackStart)
+        delete pCallbackStart;
+
+    LogFlowFuncLeaveRC(rc);
     return rc;
 }
@@ -862 +918 @@
 
     int rc = pProcess->startProcess();
-
-    LogFlowFuncLeave();
+    AssertRC(rc);
+
+    LogFlowFuncLeaveRC(rc);
     return rc;
 }
@@ -879 +936 @@
 int GuestProcess::waitFor(uint32_t fFlags, ULONG uTimeoutMS, ProcessWaitReason_T *penmReason)
 {
-    LogFlowFunc(("fFlags=%x, uTimeoutMS=%RU32, penmReason=%p, mWaitEvent=%p\n",
-                 fFlags, uTimeoutMS, penmReason, &mData.mWaitEvent));
-
     Assert(mData.mWaitEvent != NIL_RTSEMEVENT);
 
@@ -887 +941 @@
         return VERR_ALREADY_EXISTS;
 
+    /* At the moment we only support one waiter at a time. */
     int rc = RTSemMutexRequest(mData.mWaitMutex, uTimeoutMS);
     if (RT_SUCCESS(rc))
@@ -902 +957 @@
     }
 
-    LogFlowFuncLeaveRC(rc);
     return rc;
 }
@@ -981 +1035 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-
+    /*
+     * Note: Do not hold any locks here while waiting!
+     */
     uint32_t fWaitFor = ProcessWaitForFlag_None;
     com::SafeArray<ProcessWaitForFlag_T> flags(ComSafeArrayInArg(aFlags));

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

@@ -61 +61 @@
 
     AssertPtrReturn(aGuest, VERR_INVALID_POINTER);
-    AssertReturn(aSessionID, VERR_INVALID_PARAMETER);
 
     /* Enclose the state transition NotReady->InInit->Ready. */
@@ -67 +66 @@
     AssertReturn(autoInitSpan.isOk(), VERR_OBJECT_DESTROYED);
 
+    mData.mTimeout = 30 * 60 * 1000; /* Session timeout is 30 mins by default. */
     mData.mParent = aGuest;
     mData.mId = aSessionID;
@@ -352 +352 @@
 int GuestSession::dispatchToProcess(uint32_t uContextID, uint32_t uFunction, void *pvData, size_t cbData)
 {
+    LogFlowFuncEnter();
+
     AssertPtrReturn(pvData, VERR_INVALID_POINTER);
     AssertReturn(cbData, VERR_INVALID_PARAMETER);
@@ -357 +359 @@
     AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
 
+    int rc;
     SessionProcesses::const_iterator itProc
         = mData.mProcesses.find(VBOX_GUESTCTRL_CONTEXTID_GET_PROCESS(uContextID));
@@ -365 +368 @@
 
         alock.release();
-        return pProcess->callbackDispatcher(uContextID, uFunction, pvData, cbData);
-    }
-    return VERR_NOT_FOUND;
+        rc = pProcess->callbackDispatcher(uContextID, uFunction, pvData, cbData);
+    }
+    else
+        rc = VERR_NOT_FOUND;
+
+    LogFlowFuncLeaveRC(rc);
+    return rc;
 }
 
@@ -414 +421 @@
 }
 
-int GuestSession::processCreateExInteral(GuestProcessInfo &aProcInfo, IGuestProcess **aProcess)
-{
-    AssertPtrReturn(aProcess, VERR_INVALID_POINTER);
-
+int GuestSession::processCreateExInteral(const GuestProcessInfo &aProcInfo, IGuestProcess **aProcess)
+{
     /* Validate flags. */
     if (aProcInfo.mFlags)
@@ -430 +435 @@
     }
 
+    GuestProcessInfo procInfo = aProcInfo;
+
     /* Adjust timeout. If set to 0, we define
      * an infinite timeout. */
-    if (aProcInfo.mTimeoutMS == 0)
-        aProcInfo.mTimeoutMS = UINT32_MAX;
+    if (procInfo.mTimeoutMS == 0)
+        procInfo.mTimeoutMS = UINT32_MAX;
 
     /** @tood Implement process priority + affinity. */
 
-    int rc = VERR_MAX_THRDS_REACHED;
+    int rc = VERR_MAX_PROCS_REACHED;
+    if (mData.mProcesses.size() >= VBOX_GUESTCTRL_MAX_PROCESSES)
+        return rc;
 
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
@@ -447 +456 @@
     for (;;)
     {
-        /* Is the context ID already used?  Try next ID ... */
-        if (!processExists(uNewProcessID++, NULL /* pProgress */))
+        /* Is the context ID already used? */
+        if (!processExists(uNewProcessID, NULL /* pProgress */))
         {
             /* Callback with context ID was not found. This means
@@ -456 +465 @@
             break;
         }
+        uNewProcessID++;
 
         if (++uTries == UINT32_MAX)
@@ -470 +480 @@
 
         rc = pGuestProcess->init(mData.mParent->getConsole() /* Console */, this /* Session */,
-                                 uNewProcessID, aProcInfo);
+                                 uNewProcessID, procInfo);
         if (RT_FAILURE(rc)) throw rc;
 
@@ -489 +499 @@
 inline bool GuestSession::processExists(ULONG uProcessID, ComObjPtr<GuestProcess> *pProcess)
 {
-    AssertReturn(uProcessID, false);
-
     SessionProcesses::const_iterator it = mData.mProcesses.find(uProcessID);
     if (it != mData.mProcesses.end())
@@ -932 +940 @@
     com::SafeArray<LONG> affinity;
 
-    return ProcessCreateEx(aCommand, ComSafeArrayInArg(aArguments), ComSafeArrayInArg(aEnvironment),
-                           ComSafeArrayInArg(aFlags), aTimeoutMS, ProcessPriority_Default, ComSafeArrayAsInParam(affinity), aProcess);
+    HRESULT hr = ProcessCreateEx(aCommand, ComSafeArrayInArg(aArguments), ComSafeArrayInArg(aEnvironment),
+                                 ComSafeArrayInArg(aFlags), aTimeoutMS, ProcessPriority_Default, ComSafeArrayAsInParam(affinity), aProcess);
+    LogFlowFuncLeaveRC(hr);
+    return hr;
 #endif /* VBOX_WITH_GUEST_CONTROL */
 }
@@ -957 +967 @@
 
     procInfo.mCommand = Utf8Str(aCommand);
-
-    com::SafeArray<IN_BSTR> arguments(ComSafeArrayInArg(aArguments));
-    procInfo.mArguments.reserve(arguments.size());
-    for (size_t i = 0; i < arguments.size(); i++)
-        procInfo.mArguments[i] = Utf8Str(Bstr(arguments[i]));
+    procInfo.mFlags = ProcessCreateFlag_None;
+
+    if (aArguments)
+    {
+        com::SafeArray<IN_BSTR> arguments(ComSafeArrayInArg(aArguments));
+        procInfo.mArguments.reserve(arguments.size());
+        for (size_t i = 0; i < arguments.size(); i++)
+            procInfo.mArguments[i] = Utf8Str(Bstr(arguments[i]));
+    }
 
     int rc = VINF_SUCCESS;
 
-    /* Create the process environment:
+    /*
+     * Create the process environment:
      * - Apply the session environment in a first step, and
      * - Apply environment variables specified by this call to
      *   have the chance of overwriting/deleting session entries.
      */
-    procInfo.mEnvironment = mData.mEnvironment;
-    com::SafeArray<IN_BSTR> environment(ComSafeArrayInArg(aEnvironment));
-    for (size_t i = 0; i < environment.size() && RT_SUCCESS(rc); i++)
-        rc = mData.mEnvironment.Set(Utf8Str(environment[i]));
+    procInfo.mEnvironment = mData.mEnvironment; /* Apply original session environment. */
+    if (aEnvironment) /* Apply/overwrite environment, if set. */
+    {
+        com::SafeArray<IN_BSTR> environment(ComSafeArrayInArg(aEnvironment));
+        for (size_t i = 0; i < environment.size() && RT_SUCCESS(rc); i++)
+            rc = mData.mEnvironment.Set(Utf8Str(environment[i]));
+    }
 
     if (RT_SUCCESS(rc))
     {
-        com::SafeArray<ProcessCreateFlag_T> flags(ComSafeArrayInArg(aFlags));
-        for (size_t i = 0; i < flags.size(); i++)
-            procInfo.mFlags |= flags[i];
+        if (aFlags)
+        {
+            com::SafeArray<ProcessCreateFlag_T> flags(ComSafeArrayInArg(aFlags));
+            for (size_t i = 0; i < flags.size(); i++)
+                procInfo.mFlags |= flags[i];
+        }
 
         procInfo.mTimeoutMS = aTimeoutMS;
 
-        com::SafeArray<LONG> affinity(ComSafeArrayInArg(aAffinity));
-        procInfo.mAffinity.reserve(affinity.size());
-        for (size_t i = 0; i < affinity.size(); i++)
-            procInfo.mAffinity[i] = affinity[i]; /** @todo Really necessary? Later. */
+        if (aAffinity)
+        {
+            com::SafeArray<LONG> affinity(ComSafeArrayInArg(aAffinity));
+            procInfo.mAffinity.reserve(affinity.size());
+            for (size_t i = 0; i < affinity.size(); i++)
+                procInfo.mAffinity[i] = affinity[i]; /** @todo Really necessary? Later. */
+        }
 
         procInfo.mPriority = aPriority;