Changeset 28625 in vbox

Timestamp:

Apr 23, 2010 7:57:16 AM (15 years ago)

Author:

vboxsync

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

60476

Message:

Guest Control: Update (no buffered host waiting anymore, fixed a couple of deadlocks and shutdown crashes).

Location:

trunk/src/VBox

Files:

• Frontends/VBoxManage/VBoxManageGuestCtrl.cpp (modified) (1 diff)
• HostServices/GuestControl/service.cpp (modified) (1 diff)
• Main/GuestImpl.cpp (modified) (17 diffs)

trunk/src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp

@@ -65 +65 @@
              "                            <path to program> [--arguments \"<arguments>\"] [--environment \"<NAME=VALUE NAME=VALUE>\"]\n"
              "                            [--flags <flags>] [--timeout <msec>] [--username <name> [--password <password>]]\n"
-             "                            [--verbose] [--wait-for exit]\n"
+             "                            [--verbose] [--wait-for exit,stdout,stderr||]\n"
              "\n");
 }

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

@@ -531 +531 @@
     int rc = VINF_SUCCESS;
 
-    HostCmd newCmd;
-    rc = paramBufferAllocate(&newCmd.parmBuf, eFunction, cParms, paParms);
-    if (RT_SUCCESS(rc))
-    {
-        mHostCmds.push_back(newCmd);
-
-        /* Limit list size by deleting oldest element. */
-        if (mHostCmds.size() > 256) /** @todo Use a define! */
-            mHostCmds.pop_front();
-    }
-
     /* Some lazy guests to wake up which can process this command right now? */
     if (!mGuestWaiters.empty())
     {
-        GuestCall curCall = mGuestWaiters.front();
-        rc = notifyGuest(&curCall, eFunction, cParms, paParms);
-        mGuestWaiters.pop_front();
-    }
+        HostCmd newCmd;
+        rc = paramBufferAllocate(&newCmd.parmBuf, eFunction, cParms, paParms);
+        if (RT_SUCCESS(rc))
+        {
+            mHostCmds.push_back(newCmd);
+    
+            /* Limit list size by deleting oldest element. */
+            if (mHostCmds.size() > 256) /** @todo Use a define! */
+                mHostCmds.pop_front();
+
+            GuestCall curCall = mGuestWaiters.front();
+            rc = notifyGuest(&curCall, eFunction, cParms, paParms);
+            mGuestWaiters.pop_front();
+        }
+    }
+    else /* No guest waiting, don't bother ... */
+        rc = VERR_TIMEOUT;
     return rc;
 }

trunk/src/VBox/Main/GuestImpl.cpp

@@ -111 +111 @@
 
 #ifdef VBOX_WITH_GUEST_CONTROL
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
     /* Clean up callback data. */
     CallbackListIter it;
@@ -508 +510 @@
     int rc = VINF_SUCCESS;
 
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
     AssertPtr(pData);
     CallbackListIter it = getCtrlCallbackContextByID(pData->hdr.u32ContextID);
@@ -580 +584 @@
     int rc = VINF_SUCCESS;
 
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+
     AssertPtr(pData);
     CallbackListIter it = getCtrlCallbackContextByID(pData->hdr.u32ContextID);
@@ -600 +606 @@
             pCBData->cbData = pData->cbData;
         }
-
         ASMAtomicWriteBool(&it->bCalled, true);
     }
@@ -611 +616 @@
 Guest::CallbackListIter Guest::getCtrlCallbackContextByID(uint32_t u32ContextID)
 {
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
     /** @todo Maybe use a map instead of list for fast context lookup. */
     CallbackListIter it;
@@ -623 +630 @@
 Guest::CallbackListIter Guest::getCtrlCallbackContextByPID(uint32_t u32PID)
 {
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
     /** @todo Maybe use a map instead of list for fast context lookup. */
     CallbackListIter it;
@@ -637 +646 @@
 {
     LogFlowFuncEnter();
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
     if (it->cbData)
     {
@@ -650 +660 @@
         }
     }
-    mCallbackList.erase(it);
     LogFlowFuncLeave();
 }
@@ -657 +666 @@
  * to the callback list. A callback is identified by a unique context ID which is used
  * to identify a callback from the guest side. */
-uint32_t Guest::addCtrlCallbackContext(void *pvData, uint32_t cbData, Progress* pProgress)
+uint32_t Guest::addCtrlCallbackContext(void *pvData, uint32_t cbData, Progress *pProgress)
 {
     LogFlowFuncEnter();
-
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
     uint32_t uNewContext = ASMAtomicIncU32(&mNextContextID);
     if (uNewContext == UINT32_MAX)
@@ -674 +683 @@
     mCallbackList.push_back(context);
     if (mCallbackList.size() > 256) /* Don't let the container size get too big! */
-        removeCtrlCallbackContext(mCallbackList.begin());
+    {
+        Guest::CallbackListIter it = mCallbackList.begin();
+        removeCtrlCallbackContext(it);
+        mCallbackList.erase(it);
+    }
 
     LogFlowFuncLeave();
@@ -704 +717 @@
         AutoCaller autoCaller(this);
         if (FAILED(autoCaller.rc())) return autoCaller.rc();
-
-        AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
 
         /*
@@ -733 +744 @@
         char **papszArgv = NULL;
         uint32_t uNumArgs = 0;
-        if(aArguments > 0)
+        if (aArguments > 0)
         {
             com::SafeArray<IN_BSTR> args(ComSafeArrayInArg(aArguments));
@@ -800 +811 @@
                     paParms[i++].setUInt32(aTimeoutMS);
 
+                    /* Make sure mParent is valid, so set a read lock in this scope. */
+                    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+
                     /* Forward the information to the VMM device. */
                     AssertPtr(mParent);
@@ -809 +823 @@
                                                    i, paParms);
                     }
+                    else
+                        vrc = VERR_INVALID_VM_HANDLE;
                     RTMemFree(pvEnv);
                 }
@@ -823 +839 @@
                  */
                 CallbackListIter it = getCtrlCallbackContextByID(uContextID);
-                uint64_t u64Started = RTTimeMilliTS();
-                do
+                if (it != mCallbackList.end())
                 {
-                    unsigned cMsWait;
-                    if (aTimeoutMS == RT_INDEFINITE_WAIT)
-                        cMsWait = 1000;
-                    else
+                    uint64_t u64Started = RTTimeMilliTS();
+                    do
                     {
-                        uint64_t cMsElapsed = RTTimeMilliTS() - u64Started;
-                        if (cMsElapsed >= aTimeoutMS)
-                            break; /* timed out */
-                        cMsWait = RT_MIN(1000, aTimeoutMS - (uint32_t)cMsElapsed);
-                    }
-                    RTThreadSleep(100);
-                } while (it != mCallbackList.end() && !it->bCalled);
+                        unsigned cMsWait;
+                        if (aTimeoutMS == RT_INDEFINITE_WAIT)
+                            cMsWait = 1000;
+                        else
+                        {
+                            uint64_t cMsElapsed = RTTimeMilliTS() - u64Started;
+                            if (cMsElapsed >= aTimeoutMS)
+                                break; /* Timed out. */
+                            cMsWait = RT_MIN(1000, aTimeoutMS - (uint32_t)cMsElapsed);
+                        }
+                        RTThreadSleep(100);
+                    } while (!it->bCalled);
+                }
+
+                AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
 
                 /* Did we get some status? */
@@ -862 +883 @@
                 else /* If callback not called within time ... well, that's a timeout! */
                     vrc = VERR_TIMEOUT;
+
+                alock.release();
 
                 /*
@@ -903 +926 @@
             else
             {
-                /* HGCM call went wrong. */
-                rc = setError(E_UNEXPECTED,
-                              tr("The service call failed with error %Rrc"), vrc);
+                if (vrc == VERR_INVALID_VM_HANDLE)
+                {
+                    rc = setError(VBOX_E_VM_ERROR,
+                                  tr("VMM device is not available (is the VM running?)"));
+                }
+                else if (vrc == VERR_TIMEOUT)
+                {
+                    rc = setError(VBOX_E_VM_ERROR,
+                                  tr("The guest execution service is not ready"));
+                }
+                else /* HGCM call went wrong. */
+                {
+                    rc = setError(E_UNEXPECTED,
+                                  tr("The service call failed with error %Rrc"), vrc);
+                }
             }