Changeset 24074 in vbox for trunk/src

Timestamp:

Oct 26, 2009 1:02:06 PM (15 years ago)

Author:

vboxsync

Message:

Main/Machine: fix race when spawning session, caused VBoxSVC crash due to temporary setting the session state to "Spawning", without having a progress object

File:

• trunk/src/VBox/Main/MachineImpl.cpp (modified) (14 diffs)

trunk/src/VBox/Main/MachineImpl.cpp

@@ -2566 +2566 @@
                         tr("Setting passthrough rejected as the device attached to device slot %d on port %d of controller '%ls' is not a DVD"),
                         aDevice, aControllerPort, aControllerName);
-    pAttach->updatePassthrough(aPassthrough);
+    pAttach->updatePassthrough(!!aPassthrough);
 
     return S_OK;
@@ -3771 +3771 @@
 
     /* may not be busy */
-    AssertReturn(!Global::IsOnlineOrTransient (mData->mMachineState), E_FAIL);
+    AssertReturn(!Global::IsOnlineOrTransient(mData->mMachineState), E_FAIL);
 
     /* get the session PID */
     RTPROCESS pid = NIL_RTPROCESS;
-    AssertCompile (sizeof (ULONG) == sizeof (RTPROCESS));
-    aControl->GetPID ((ULONG *) &pid);
-    Assert (pid != NIL_RTPROCESS);
+    AssertCompile(sizeof(ULONG) == sizeof(RTPROCESS));
+    aControl->GetPID((ULONG *) &pid);
+    Assert(pid != NIL_RTPROCESS);
 
     if (mData->mSession.mState == SessionState_Spawning)
@@ -3800 +3800 @@
     ComObjPtr<SessionMachine> sessionMachine;
     sessionMachine.createObject();
-    HRESULT rc = sessionMachine->init (this);
-    AssertComRC (rc);
+    HRESULT rc = sessionMachine->init(this);
+    AssertComRC(rc);
 
     /* NOTE: doing return from this function after this point but
@@ -3812 +3812 @@
     {
 #ifdef VBOX_WITH_RESOURCE_USAGE_API
-        registerMetrics (mParent->performanceCollector(), this, pid);
+        registerMetrics(mParent->performanceCollector(), this, pid);
 #endif /* VBOX_WITH_RESOURCE_USAGE_API */
 
@@ -3836 +3836 @@
 
         LogFlowThisFunc(("Calling AssignMachine()...\n"));
-        rc = aControl->AssignMachine (sessionMachine);
+        rc = aControl->AssignMachine(sessionMachine);
         LogFlowThisFunc(("AssignMachine() returned %08X\n", rc));
 
         /* The failure may occur w/o any error info (from RPC), so provide one */
-        if (FAILED (rc))
-            setError (VBOX_E_VM_ERROR,
-                tr ("Failed to assign the machine to the session (%Rrc)"), rc);
+        if (FAILED(rc))
+            setError(VBOX_E_VM_ERROR,
+                tr("Failed to assign the machine to the session (%Rrc)"), rc);
 
         if (SUCCEEDED(rc) && origState == SessionState_Spawning)
@@ -3850 +3850 @@
             /* get the console from the direct session */
             ComPtr<IConsole> console;
-            rc = aControl->GetRemoteConsole (console.asOutParam());
-            ComAssertComRC (rc);
+            rc = aControl->GetRemoteConsole(console.asOutParam());
+            ComAssertComRC(rc);
 
             if (SUCCEEDED(rc) && !console)
             {
-                ComAssert (!!console);
+                ComAssert(!!console);
                 rc = E_FAIL;
             }
@@ -3868 +3868 @@
                 LogFlowThisFunc(("Calling AssignRemoteMachine()...\n"));
                 rc = mData->mSession.mRemoteControls.front()->
-                    AssignRemoteMachine (sessionMachine, console);
+                    AssignRemoteMachine(sessionMachine, console);
                 LogFlowThisFunc(("AssignRemoteMachine() returned %08X\n", rc));
 
@@ -3932 +3932 @@
          * at least on XPCOM) */
         ComPtr<IUnknown> unk = mData->mSession.mDirectControl;
-        NOREF (unk);
+        NOREF(unk);
     }
 
@@ -3938 +3938 @@
     {
         /* finalize the progress after setting the state, for consistency */
-        mData->mSession.mProgress->notifyComplete (rc);
+        mData->mSession.mProgress->notifyComplete(rc);
         mData->mSession.mProgress.setNull();
     }
@@ -4407 +4407 @@
 
     /* VirtualBox::addProcessToReap() needs a write lock */
-    AutoMultiWriteLock2 alock (mParent, this);
+    AutoMultiWriteLock2 alock(mParent, this);
 
     if (mData->mSession.mState != SessionState_Spawning)
@@ -4418 +4418 @@
     HRESULT rc = S_OK;
 
-#if defined (RT_OS_WINDOWS) || defined (RT_OS_OS2)
+#if defined(RT_OS_WINDOWS) || defined(RT_OS_OS2)
 
     /* the process was already unexpectedly terminated, we just need to set an
@@ -4427 +4427 @@
 #else
 
+    /* PID not yet initialized, skip check. */
+    if (mData->mSession.mPid == NIL_RTPROCESS)
+        return false;
+
     RTPROCSTATUS status;
-    int vrc = ::RTProcWait (mData->mSession.mPid, RTPROCWAIT_FLAGS_NOBLOCK,
-                            &status);
+    int vrc = ::RTProcWait(mData->mSession.mPid, RTPROCWAIT_FLAGS_NOBLOCK,
+                           &status);
 
     if (vrc != VERR_PROCESS_RUNNING)
@@ -4443 +4447 @@
          * the relevant part in checkForSpawnFailure()). */
 
-        Assert (mData->mSession.mRemoteControls.size() == 1);
+        Assert(mData->mSession.mRemoteControls.size() == 1);
         if (mData->mSession.mRemoteControls.size() == 1)
         {
@@ -4454 +4458 @@
 
         /* finalize the progress after setting the state, for consistency */
-        mData->mSession.mProgress->notifyComplete (rc);
-        mData->mSession.mProgress.setNull();
-
-        mParent->addProcessToReap (mData->mSession.mPid);
+        if (!mData->mSession.mProgress.isNull())
+        {
+            mData->mSession.mProgress->notifyComplete(rc);
+            mData->mSession.mProgress.setNull();
+        }
+
+        mParent->addProcessToReap(mData->mSession.mPid);
         mData->mSession.mPid = NIL_RTPROCESS;
 
-        mParent->onSessionStateChange (mData->mUuid, SessionState_Closed);
+        mParent->onSessionStateChange(mData->mUuid, SessionState_Closed);
         return true;
     }