Changeset 47627 in vbox

Timestamp:

Aug 9, 2013 8:31:24 AM (12 years ago)

Author:

vboxsync

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

87828

Message:

Main/GuestCtrl: Reduced locking times, more error checking, adjusted logging.

Location:

trunk/src/VBox/Main

Files:

• include/GuestCtrlImplPrivate.h (modified) (1 diff)
• include/GuestProcessImpl.h (modified) (2 diffs)
• src-client/GuestCtrlImpl.cpp (modified) (9 diffs)
• src-client/GuestCtrlPrivate.cpp (modified) (9 diffs)
• src-client/GuestDirectoryImpl.cpp (modified) (2 diffs)
• src-client/GuestProcessImpl.cpp (modified) (40 diffs)
• src-client/GuestSessionImpl.cpp (modified) (26 diffs)
• src-client/GuestSessionImplTasks.cpp (modified) (2 diffs)

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

@@ -388 +388 @@
     const ComPtr<IEvent>             Event(void) { return mEvent; };
     const std::list<VBoxEventType_T> Types(void) { return mEventTypes; };
+    size_t                           TypeCount(void) { return mEventTypes.size(); }
     virtual int                      Signal(IEvent *pEvent);
     int                              Wait(RTMSINTERVAL uTimeoutMS);

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

@@ -77 +77 @@
     inline int checkPID(uint32_t uPID);
     static Utf8Str guestErrorToString(int guestRc);
-    bool isReady(void);
     int readData(uint32_t uHandle, uint32_t uSize, uint32_t uTimeoutMS, void *pvData, size_t cbData, uint32_t *pcbRead, int *pGuestRc);
     static HRESULT setErrorExternal(VirtualBoxBase *pInterface, int guestRc);
@@ -97 +96 @@
      * @{ */
     inline bool isAlive(void);
+    inline bool hasEnded(void);
     int onGuestDisconnected(PVBOXGUESTCTRLHOSTCBCTX pCbCtx, PVBOXGUESTCTRLHOSTCALLBACK pSvcCbData);
     int onProcessInputStatus(PVBOXGUESTCTRLHOSTCBCTX pCbCtx, PVBOXGUESTCTRLHOSTCALLBACK pSvcCbData);

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

@@ -110 +110 @@
     VBOXGUESTCTRLHOSTCBCTX ctxCb = { u32Function, uContextID };
     rc = pGuest->dispatchToSession(&ctxCb, pSvcCb);
-    LogFlowFuncLeaveRC(rc);
+
+    LogFlowFunc(("Returning rc=%Rrc\n", rc));
     return rc;
 }
@@ -322 +323 @@
         rc = VERR_NOT_FOUND;
 
-    LogFlowFuncLeaveRC(rc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", rc));
     return rc;
 }
@@ -341 +342 @@
         if (pSession == itSessions->second)
         {
-            GuestSession *pCurSession = itSessions->second;
-            AssertPtr(pCurSession);
+            /* Make sure to consume the pointer before the one of the
+             * iterator gets released. */
+            ComObjPtr<GuestSession> pCurSession = pSession;
 
             LogFlowFunc(("Removing session (pSession=%p, ID=%RU32) (now total %ld sessions)\n",
-                         pCurSession, pCurSession->getId(), mData.mGuestSessions.size() - 1));
+                         pSession, pSession->getId(), mData.mGuestSessions.size() - 1));
 
             itSessions->second->Release();
@@ -351 +353 @@
             mData.mGuestSessions.erase(itSessions);
 
-            fireGuestSessionRegisteredEvent(mEventSource, pSession,
+            alock.release(); /* Release lock before firing off event. */
+
+            fireGuestSessionRegisteredEvent(mEventSource, pCurSession,
                                             false /* Unregistered */);
             rc = VINF_SUCCESS;
@@ -360 +364 @@
     }
 
-    LogFlowFuncLeaveRC(rc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", rc));
     return rc;
 }
@@ -431 +435 @@
         mData.mGuestSessions[uNewSessionID] = pGuestSession;
 
+        alock.release(); /* Release lock before firing off event. */
+
         fireGuestSessionRegisteredEvent(mEventSource, pGuestSession,
                                         true /* Registered */);
@@ -439 +445 @@
     }
 
-    LogFlowFuncLeaveRC(rc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", rc));
     return rc;
 }
@@ -452 +458 @@
 /////////////////////////////////////////////////////////////////////////////
 
-STDMETHODIMP Guest::CreateSession(IN_BSTR aUser, IN_BSTR aPassword, IN_BSTR aDomain, IN_BSTR aSessionName, IGuestSession **aGuestSession)
+STDMETHODIMP Guest::CreateSession(IN_BSTR aUser, IN_BSTR aPassword, IN_BSTR aDomain,
+                                  IN_BSTR aSessionName, IGuestSession **aGuestSession)
 {
 #ifndef VBOX_WITH_GUEST_CONTROL
@@ -515 +522 @@
     }
 
-    LogFlowFuncLeaveRC(rc);
+    LogFlowThisFunc(("Returning rc=%Rhrc\n", hr));
     return hr;
 #endif /* VBOX_WITH_GUEST_CONTROL */

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

@@ -850 +850 @@
     AssertPtrReturn(puContextID, VERR_INVALID_POINTER);
 
+    if (   uSessionID >= VBOX_GUESTCTRL_MAX_SESSIONS
+        || uObjectID  >= VBOX_GUESTCTRL_MAX_OBJECTS)
+        return VERR_INVALID_PARAMETER;
+
     uint32_t uCount = ASMAtomicIncU32(&mNextContextID);
     if (uCount == VBOX_GUESTCTRL_MAX_CONTEXTS)
@@ -859 +863 @@
     *puContextID = uNewContextID;
 
+    LogFlowThisFunc(("mNextContextID=%RU32, uSessionID=%RU32, uObjectID=%RU32, uCount=%RU32, uNewContextID=%RU32\n",
+                     mNextContextID, uSessionID, uObjectID, uCount, uNewContextID));
     return VINF_SUCCESS;
 }
@@ -913 +919 @@
                  itEvents != itTypes->second.end(); itEvents++)
             {
+                LogFlowThisFunc(("Signalling event=%p with type=%ld ...\n",
+                                 (*itEvents), aType));
+
                 ComPtr<IEvent> pThisEvent = aEvent;
                 Assert(!pThisEvent.isNull());
@@ -969 +978 @@
     AssertPtrReturn(pEvent, VERR_INVALID_POINTER);
 
-    LogFlowFunc(("pEvent=%p, uTimeoutMS=%RU32\n",
-                 pEvent, uTimeoutMS));
-
     int vrc = pEvent->Wait(uTimeoutMS);
     if (RT_SUCCESS(vrc))
@@ -991 +997 @@
     }
 
-    LogFlowFuncLeaveRC(vrc);
     return vrc;
 }
@@ -1027 +1032 @@
                              uint32_t uParms, PVBOXHGCMSVCPARM paParms)
 {
-    LogFlowThisFuncEnter();
-
 #ifndef VBOX_GUESTCTRL_TEST_CASE
     ComObjPtr<Console> pConsole = mConsole;
     Assert(!pConsole.isNull());
 
+    int vrc = VERR_HGCM_SERVICE_NOT_FOUND;
+
     /* Forward the information to the VMM device. */
     VMMDev *pVMMDev = pConsole->getVMMDev();
-    AssertPtr(pVMMDev);
-
-    LogFlowThisFunc(("uFunction=%RU32, uParms=%RU32\n", uFunction, uParms));
-    int vrc = pVMMDev->hgcmHostCall(HGCMSERVICE_NAME, uFunction, uParms, paParms);
-    if (RT_FAILURE(vrc))
-    {
-        /** @todo What to do here? */
+    if (pVMMDev)
+    {
+        LogFlowThisFunc(("uFunction=%RU32, uParms=%RU32\n", uFunction, uParms));
+        vrc = pVMMDev->hgcmHostCall(HGCMSERVICE_NAME, uFunction, uParms, paParms);
+        if (RT_FAILURE(vrc))
+        {
+            /** @todo What to do here? */
+        }
     }
 #else
+    LogFlowThisFuncEnter();
+
     /* Not needed within testcases. */
     int vrc = VINF_SUCCESS;
 #endif
-    LogFlowFuncLeaveRC(vrc);
     return vrc;
 }
@@ -1065 +1072 @@
 GuestWaitEvent::~GuestWaitEvent(void)
 {
-
 }
 
@@ -1089 +1095 @@
 int GuestWaitEvent::Wait(RTMSINTERVAL uTimeoutMS)
 {
-    LogFlowThisFunc(("uTimeoutMS=%RU32ms\n", uTimeoutMS));
-
     int rc = VINF_SUCCESS;
 
@@ -1108 +1112 @@
     }
 
-    LogFlowFuncLeaveRC(rc);
-    return rc;
-}
-
+    return rc;
+}
+

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

@@ -212 +212 @@
     uninit();
 
-    LogFlowFuncLeaveRC(rc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", rc));
     return hr;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -314 +314 @@
     }
 
-    LogFlowFuncLeaveRC(rc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", rc));
     return hr;
 #endif /* VBOX_WITH_GUEST_CONTROL */

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

@@ -363 +363 @@
     mEventSource.queryInterfaceTo(aEventSource);
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -478 +478 @@
 #endif
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
     int vrc;
     switch (pCbCtx->uFunction)
@@ -514 +512 @@
 
 #ifdef DEBUG
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
 #endif
     return vrc;
@@ -631 +629 @@
 }
 
-bool GuestProcess::isReady(void)
-{
-    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    if (mData.mStatus == ProcessStatus_Started)
-    {
-        Assert(mData.mPID); /* PID must not be 0. */
-        return true;
-    }
-
-    return false;
+inline bool GuestProcess::hasEnded(void)
+{
+    return (   mData.mStatus == ProcessStatus_TerminatedNormally
+            || mData.mStatus == ProcessStatus_TerminatedSignal
+            || mData.mStatus == ProcessStatus_TerminatedAbnormally
+            || mData.mStatus == ProcessStatus_TimedOutKilled
+            || mData.mStatus == ProcessStatus_TimedOutAbnormally
+            || mData.mStatus == ProcessStatus_Down
+            || mData.mStatus == ProcessStatus_Error);
 }
 
@@ -649 +645 @@
     AssertPtrReturn(pSvcCbData, VERR_INVALID_POINTER);
 
-    LogFlowThisFunc(("uPID=%RU32\n", mData.mPID));
-
     int vrc = setProcessStatus(ProcessStatus_Down, VINF_SUCCESS);
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -707 +701 @@
         if (inputStatus != ProcessInputStatus_Undefined)
         {
+            AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
+            /* Copy over necessary data before releasing lock again. */
+            uint32_t uPID = mData.mPID;
+            /** @todo Also handle mSession? */
+
+            alock.release(); /* Release lock before firing off event. */
+
             fireGuestProcessInputNotifyEvent(mEventSource, mSession, this,
-                                             mData.mPID, 0 /* StdIn */, dataCb.uProcessed, inputStatus);
-        }
-    }
-
-    LogFlowFuncLeaveRC(vrc);
+                                             uPID, 0 /* StdIn */, dataCb.uProcessed, inputStatus);
+        }
+    }
+
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -757 +759 @@
             {
                 procStatus = ProcessStatus_Started;
+
+                AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
                 mData.mPID = dataCb.uPID; /* Set the process PID. */
                 break;
@@ -764 +768 @@
             {
                 procStatus = ProcessStatus_TerminatedNormally;
+
+                AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
                 mData.mExitCode = dataCb.uFlags; /* Contains the exit code. */
                 break;
@@ -771 +777 @@
             {
                 procStatus = ProcessStatus_TerminatedSignal;
+
+                AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
                 mData.mExitCode = dataCb.uFlags; /* Contains the signal. */
                 break;
@@ -824 +832 @@
     }
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -860 +868 @@
     }
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -940 +948 @@
     unregisterWaitEvent(pEvent);
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -947 +955 @@
 int GuestProcess::setProcessStatus(ProcessStatus_T procStatus, int procRc)
 {
+    LogFlowThisFuncEnter();
+
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
     LogFlowThisFunc(("oldStatus=%ld, newStatus=%ld, procRc=%Rrc\n",
                      mData.mStatus, procStatus, procRc));
@@ -955 +967 @@
         /* Do not allow overwriting an already set error. If this happens
          * this means we forgot some error checking/locking somewhere. */
-        //AssertMsg(RT_SUCCESS(mData.mRC), ("Guest rc already set (to %Rrc)\n", mData.mRC));
+        AssertMsg(RT_SUCCESS(mData.mRC), ("Guest rc already set (to %Rrc)\n", mData.mRC));
     }
     else
         AssertMsg(RT_SUCCESS(procRc), ("Guest rc must not be an error (%Rrc)\n", procRc));
+
+    int rc = VINF_SUCCESS;
 
     if (mData.mStatus != procStatus) /* Was there a process status change? */
@@ -976 +990 @@
         }
 
+        /* Copy over necessary data before releasing lock again. */
+        uint32_t uPID =  mData.mPID;
+        ProcessStatus_T uStatus = mData.mStatus;
+        /** @todo Also handle mSession? */
+
+        alock.release(); /* Release lock before firing off event. */
+
         fireGuestProcessStateChangedEvent(mEventSource, mSession, this,
-                                          mData.mPID, mData.mStatus, errorInfo);
-    }
-
-    return VINF_SUCCESS;
+                                          uPID, uStatus, errorInfo);
+#if 0
+        /*
+         * On Guest Additions < 4.3 there is no guarantee that outstanding
+         * requests will be delivered to the host after the process has ended,
+         * so just cancel all waiting events here to not let clients run
+         * into timeouts.
+         */
+        if (   mSession->getProtocolVersion() < 2
+            && hasEnded())
+        {
+            LogFlowThisFunc(("Process ended, canceling outstanding wait events ...\n"));
+            rc = cancelWaitEvents();
+        }
+#endif
+    }
+
+    return rc;
 }
 
@@ -994 +1029 @@
 int GuestProcess::startProcess(uint32_t uTimeoutMS, int *pGuestRc)
 {
-    LogFlowThisFunc(("uTimeoutMS=%RU32, procCmd=%s, procTimeoutMS=%RU32, procFlags=%x\n",
-                     uTimeoutMS, mData.mProcess.mCommand.c_str(), mData.mProcess.mTimeoutMS, mData.mProcess.mFlags));
+    LogFlowThisFunc(("uTimeoutMS=%RU32, procCmd=%s, procTimeoutMS=%RU32, procFlags=%x, sessionID=%RU32\n",
+                     uTimeoutMS, mData.mProcess.mCommand.c_str(), mData.mProcess.mTimeoutMS, mData.mProcess.mFlags,
+                     mSession->getId()));
 
     /* Wait until the caller function (if kicked off by a thread)
@@ -1133 +1169 @@
     unregisterWaitEvent(pEvent);
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -1165 +1201 @@
     }
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -1187 +1223 @@
     /* Nothing to do here anymore. */
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowFunc(("pProcess=%p returning rc=%Rrc\n", pProcess, vrc));
     return vrc;
 }
@@ -1244 +1280 @@
     }
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -1367 +1403 @@
 int GuestProcess::waitFor(uint32_t fWaitFlags, ULONG uTimeoutMS, ProcessWaitResult_T &waitResult, int *pGuestRc)
 {
-    LogFlowThisFuncEnter();
-
     AssertReturn(fWaitFlags, VERR_INVALID_PARAMETER);
 
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
-    LogFlowThisFunc(("fWaitFlags=0x%x, uTimeoutMS=%RU32, mStatus=%RU32, pGuestRc=%p\n",
-                     fWaitFlags, uTimeoutMS, mData.mStatus, pGuestRc));
+    LogFlowThisFunc(("fWaitFlags=0x%x, uTimeoutMS=%RU32, procStatus=%RU32, procRc=%Rrc, pGuestRc=%p\n",
+                     fWaitFlags, uTimeoutMS, mData.mStatus, mData.mRC, pGuestRc));
 
     /* Did some error occur before? Then skip waiting and return. */
@@ -1387 +1421 @@
 
     waitResult = waitFlagsToResult(fWaitFlags);
-    LogFlowThisFunc(("procStatus=%ld, procRc=%Rrc, waitResult=%ld\n",
-                     mData.mStatus, mData.mRC, waitResult));
+    LogFlowThisFunc(("waitFlagToResult=%ld\n", waitResult));
 
     /* No waiting needed? Return immediately using the last set error. */
@@ -1450 +1483 @@
         else /* Waiting failed, bail out. */
             break;
+
+        alock.release(); /* Don't hold lock in next waiting round. */
     }
 
     unregisterWaitEvent(pEvent);
 
-    LogFlowThisFunc(("waitResult=%ld, processStatus=%ld, rc=%Rrc\n",
+    LogFlowThisFunc(("Returned waitResult=%ld, processStatus=%ld, rc=%Rrc\n",
                      waitResult, processStatus, vrc));
     return vrc;
@@ -1499 +1534 @@
 {
     AssertPtrReturn(pEvent, VERR_INVALID_POINTER);
+    /* pvData is optional. */
+    /* cbData is optional. */
+    /* pcbRead is optional. */
+
+    LogFlowThisFunc(("cEventTypes=%zu, pEvent=%p, uHandle=%RU32, uTimeoutMS=%RU32, pvData=%p, cbData=%zu, pcbRead=%p\n",
+                     pEvent->TypeCount(), pEvent, uHandle, uTimeoutMS, pvData, cbData, pcbRead));
 
     int vrc;
@@ -1510 +1551 @@
         if (RT_SUCCESS(vrc))
         {
+#ifdef DEBUG_andy
+            LogFlowThisFunc(("pEvent=%p, evtType=%ld\n", pEvent, evtType));
+#endif
             if (evtType == VBoxEventType_OnGuestProcessOutput)
             {
@@ -1517 +1561 @@
                 ULONG uHandleEvent;
                 HRESULT hr = pProcessEvent->COMGETTER(Handle)(&uHandleEvent);
-                if (uHandleEvent == uHandle)
+                LogFlowThisFunc(("Received output, uHandle=%RU32\n", uHandleEvent));
+                if (   SUCCEEDED(hr)
+                    && uHandleEvent == uHandle)
                 {
                     if (pvData)
@@ -1536 +1582 @@
                         }
                     }
-                    if (pcbRead)
+
+                    if (   RT_SUCCESS(vrc)
+                        && pcbRead)
                     {
                         ULONG cbRead;
@@ -1546 +1594 @@
                     break;
                 }
+                else if (FAILED(hr))
+                    vrc = VERR_COM_UNEXPECTED;
             }
             else
@@ -1551 +1601 @@
         }
 
-    } while (RT_SUCCESS(vrc));
-
-    LogFlowFuncLeaveRC(vrc);
+    } while (vrc == VINF_SUCCESS);
+
+    if (   vrc != VINF_SUCCESS
+        && pcbRead)
+    {
+        *pcbRead = 0;
+    }
+
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -1759 +1815 @@
     LogFlowThisFunc(("rc=%Rrc, cbRead=%RU32\n", vrc, cbRead));
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return hr;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -1854 +1910 @@
     }
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return hr;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -1922 +1978 @@
     *aWritten = (ULONG)cbWritten;
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return hr;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -1993 +2049 @@
     }
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }
@@ -2084 +2140 @@
         if (RT_SUCCESS(vrc))
             return vrc;
+        /* else do the the waiting below. */
     }
 
@@ -2095 +2152 @@
     LogFlowThisFunc(("waitFlags=0x%x\n", fWaitFlags));
 
-    /** @todo Decrease timeout. */
+    /** @todo Decrease timeout while running. */
     uint32_t uTimeoutMS = mStartupInfo.mTimeoutMS;
 
@@ -2163 +2220 @@
         if (fHandleStdOut)
         {
+            cbRead = 0;
             vrc = pProcess->readData(OUTPUT_HANDLE_ID_STDOUT, sizeof(byBuf),
                                      uTimeoutMS, byBuf, sizeof(byBuf),
@@ -2171 +2229 @@
             if (cbRead)
             {
-                LogFlowThisFunc(("Received %RU64 bytes from stdout\n", cbRead));
+                LogFlowThisFunc(("Received %RU32 bytes from stdout\n", cbRead));
                 vrc = mStdOut.AddData(byBuf, cbRead);
 
@@ -2189 +2247 @@
         if (fHandleStdErr)
         {
+            cbRead = 0;
             vrc = pProcess->readData(OUTPUT_HANDLE_ID_STDERR, sizeof(byBuf),
                                      uTimeoutMS, byBuf, sizeof(byBuf),
@@ -2197 +2256 @@
             if (cbRead)
             {
-                LogFlowThisFunc(("Received %RU64 bytes from stderr\n", cbRead));
+                LogFlowThisFunc(("Received %RU32 bytes from stderr\n", cbRead));
                 vrc = mStdErr.AddData(byBuf, cbRead);
             }
@@ -2211 +2270 @@
         *pGuestRc = guestRc;
 
-    LogFlowFuncLeaveRC(vrc);
+    LogFlowThisFunc(("Returning rc=%Rrc\n", vrc));
     return vrc;
 }

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

@@ -341 +341 @@
     mData.mCredentials.mUser.cloneTo(aUser);
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -362 +362 @@
     mData.mCredentials.mDomain.cloneTo(aDomain);
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -383 +383 @@
     mData.mSession.mName.cloneTo(aName);
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -404 +404 @@
     *aId = mData.mSession.mID;
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -425 +425 @@
     *aStatus = mData.mStatus;
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -446 +446 @@
     *aTimeout = mData.mTimeout;
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -465 +465 @@
     mData.mTimeout = aTimeout;
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -486 +486 @@
     *aVersion = mData.mProtocolVersion;
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -517 +517 @@
     environment.detachTo(ComSafeArrayOutArg(aEnvironment));
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -631 +631 @@
     mEventSource.queryInterfaceTo(aEventSource);
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -685 +685 @@
     paParms[i++].setUInt32(uFlags);
 
+    alock.release(); /* Drop the write lock before waiting. */
+
     vrc = sendCommand(HOST_SESSION_CLOSE, i, paParms);
     if (RT_SUCCESS(vrc))
-    {
-        alock.release(); /* Drop the write lock before waiting. */
-
         vrc = waitForStatusChange(pEvent, GuestSessionWaitForFlag_Terminate, uTimeoutMS,
                                   NULL /* Session status */, pGuestRc);
-    }
 
     unregisterWaitEvent(pEvent);
@@ -991 +989 @@
         if (pFile == itFiles->second)
         {
-            GuestFile *pThis = itFiles->second;
-            AssertPtr(pThis);
+            /* Make sure to consume the pointer before the one of thfe
+             * iterator gets released. */
+            ComObjPtr<GuestFile> pCurFile = pFile;
 
             Bstr strName;
-            HRESULT hr = pThis->COMGETTER(FileName)(strName.asOutParam());
+            HRESULT hr = pCurFile->COMGETTER(FileName)(strName.asOutParam());
             ComAssertComRC(hr);
 
@@ -1002 +1001 @@
                              Utf8Str(strName).c_str(), mData.mSession.mID, mData.mFiles.size() - 1, mData.mNumObjects - 1));
 
+            itFiles->second->Release();
+
             mData.mFiles.erase(itFiles);
             mData.mNumObjects--;
 
-            fireGuestFileRegisteredEvent(mEventSource, this, pFile,
+            alock.release(); /* Release lock before firing off event. */
+
+            fireGuestFileRegisteredEvent(mEventSource, this, pCurFile,
                                          false /* Unregistered */);
             return VINF_SUCCESS;
@@ -1107 +1110 @@
         LogFlowFunc(("Added new guest file \"%s\" (Session: %RU32) (now total %ld files, %ld objects)\n",
                      openInfo.mFileName.c_str(), mData.mSession.mID, mData.mFiles.size(), mData.mNumObjects));
+
+        alock.release(); /* Release lock before firing off event. */
 
         fireGuestFileRegisteredEvent(mEventSource, this, pFile,
@@ -1334 +1339 @@
 
     /* Set the session status. */
-    if (sessionStatus != GuestSessionStatus_Undefined)
-    {
-        int rc2 = setSessionStatus(sessionStatus, guestRc);
-        if (RT_SUCCESS(vrc))
-            vrc = rc2;
-    }
+    if (RT_SUCCESS(vrc))
+        vrc = setSessionStatus(sessionStatus, guestRc);
 
     LogFlowThisFunc(("ID=%RU32, guestRc=%Rrc\n", mData.mSession.mID, guestRc));
@@ -1405 +1406 @@
     paParms[i++].setUInt32(mData.mSession.mOpenFlags);
 
+    alock.release(); /* Drop write lock before sending. */
+
     vrc = sendCommand(HOST_SESSION_CREATE, i, paParms);
     if (RT_SUCCESS(vrc))
-    {
-        alock.release(); /* Drop write lock before waiting. */
-
         vrc = waitForStatusChange(pEvent, GuestSessionWaitForFlag_Start,
                                   30 * 1000 /* 30s timeout */,
                                   NULL /* Session status */, pGuestRc);
-    }
 
     unregisterWaitEvent(pEvent);
@@ -1495 +1494 @@
         if (pProcess == itProcs->second)
         {
-            GuestProcess *pCurProc = itProcs->second;
-            AssertPtr(pCurProc);
-
-            hr = pCurProc->COMGETTER(PID)(&uPID);
+            /* Make sure to consume the pointer before the one of thfe
+             * iterator gets released. */
+            ComObjPtr<GuestProcess> pCurProcess = pProcess;
+
+            hr = pCurProcess->COMGETTER(PID)(&uPID);
             ComAssertComRC(hr);
 
             Assert(mData.mNumObjects);
             LogFlowFunc(("Removing process ID=%RU32 (Session: %RU32), guest PID=%RU32 (now total %ld processes, %ld objects)\n",
-                         pCurProc->getObjectID(), mData.mSession.mID, uPID, mData.mProcesses.size() - 1, mData.mNumObjects - 1));
-
-            pCurProc->cancelWaitEvents();
-
-            itProcs->second->Release();
+                         pProcess->getObjectID(), mData.mSession.mID, uPID, mData.mProcesses.size() - 1, mData.mNumObjects - 1));
+
+            pProcess->cancelWaitEvents();
+            pProcess->Release();
 
             mData.mProcesses.erase(itProcs);
             mData.mNumObjects--;
 
-            fireGuestProcessRegisteredEvent(mEventSource, this /* Session */, NULL /* Process */,
+            alock.release(); /* Release lock before firing off event. */
+
+            fireGuestProcessRegisteredEvent(mEventSource, this /* Session */, pCurProcess,
                                             uPID, false /* Process unregistered */);
             rc = VINF_SUCCESS;
@@ -1815 +1816 @@
     uint32_t uVBoxMinor    = VBOX_FULL_VERSION_GET_MINOR(uVerAdditions);
 
+#if 0
+    /* Hardcode the to-used protocol version; nice for testing side effects. */
+    mData.mProtocolVersion = 2;
+#else
     mData.mProtocolVersion = (
                               /* VBox 5.0 and up. */
@@ -1823 +1828 @@
                            : 1; /* Legacy guest control (VBox < 4.3). */
     /* Build revision is ignored. */
+#endif
 
     LogFlowThisFunc(("uVerAdditions=%RU32 (%RU32.%RU32), mProtocolVersion=%RU32\n",
@@ -1829 +1835 @@
     /* Tell the user but don't bitch too often. */
     static short s_gctrlLegacyWarning = 0;
-    if (s_gctrlLegacyWarning++ < 3) /** @todo Find a bit nicer text. */
+    if (   mData.mProtocolVersion < 2
+        && s_gctrlLegacyWarning++ < 3) /** @todo Find a bit nicer text. */
         LogRel((tr("Warning: Guest Additions are older (%ld.%ld) than host capabilities for guest control, please upgrade them. Using protocol version %ld now\n"),
                 uVBoxMajor, uVBoxMinor, mData.mProtocolVersion));
@@ -1941 +1948 @@
     }
 
-    alock.release(); /* Release lock before waiting. */
-
     int vrc;
 
@@ -1961 +1966 @@
     if (RT_FAILURE(vrc))
         return vrc;
+
+    alock.release(); /* Release lock before waiting. */
 
     GuestSessionStatus_T sessionStatus;
@@ -2035 +2042 @@
             *pGuestRc = (int)lGuestRc;
 
-        LogFlowThisFunc(("Status changed event for session ID=%RU32: %ld (%Rrc)\n",
+        LogFlowThisFunc(("Status changed event for session ID=%RU32, new status is: %ld (%Rrc)\n",
                          mData.mSession.mID, sessionStatus,
                          RT_SUCCESS((int)lGuestRc) ? VINF_SUCCESS : (int)lGuestRc));
@@ -2530 +2537 @@
     mData.mEnvironment.Clear();
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -2555 +2562 @@
     strValue.cloneTo(aValue);
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */
@@ -2597 +2604 @@
     mData.mEnvironment.Unset(Utf8Str(aName));
 
-    LogFlowFuncLeaveRC(S_OK);
+    LogFlowThisFuncLeave();
     return S_OK;
 #endif /* VBOX_WITH_GUEST_CONTROL */

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

@@ -361 +361 @@
              * order to reduce the CPU load due to busy waiting. */
             if (waitRes == ProcessWaitResult_WaitFlagNotSupported)
-                RTThreadSleep(1); /* Optional, don't check rc. */
+                RTThreadYield(); /* Optional, don't check rc. */
 
             size_t cbRead = 0;
@@ -695 +695 @@
                          * order to reduce the CPU load due to busy waiting. */
                         if (waitRes == ProcessWaitResult_WaitFlagNotSupported)
-                            RTThreadSleep(1); /* Optional, don't check rc. */
-
-                        uint32_t cbRead;
+                            RTThreadYield(); /* Optional, don't check rc. */
+
+                        uint32_t cbRead = 0; /* readData can return with VWRN_GSTCTL_OBJECTSTATE_CHANGED. */
                         rc = pProcess->readData(OUTPUT_HANDLE_ID_STDOUT, sizeof(byBuf),
                                                 30 * 1000 /* Timeout */, byBuf, sizeof(byBuf),