Changeset 27796 in vbox for trunk/src/VBox/Main/MachineImpl.cpp

Timestamp:

Mar 29, 2010 4:04:55 PM (15 years ago)

Author:

vboxsync

Message:

Main: attempt at USB locking fixes

File:

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

trunk/src/VBox/Main/MachineImpl.cpp

@@ -4720 +4720 @@
     CheckComArgExpr(aData, !ComSafeArrayOutIsNull(aData));
 
+    NOREF(aDataSize);
+
     ReturnComNotImplemented();
 }
@@ -9139 +9141 @@
     }
 
-    /* We need to lock this object in uninit() because the lock is shared
-     * with mPeer (as well as data we modify below). mParent->addProcessToReap()
-     * and others need mParent lock, and USB needs host lock. */
-    AutoMultiWriteLock3 alock(mParent, mParent->host(), this COMMA_LOCKVAL_SRC_POS);
-
-#ifdef VBOX_WITH_RESOURCE_USAGE_API
-    unregisterMetrics(mParent->performanceCollector(), mPeer);
-#endif /* VBOX_WITH_RESOURCE_USAGE_API */
-
-    MachineState_T lastState = mData->mMachineState;
+    MachineState_T lastState;
+    {
+        AutoReadLock tempLock(this COMMA_LOCKVAL_SRC_POS);
+        lastState = mData->mMachineState;
+    }
     NOREF(lastState);
 
-    if (aReason == Uninit::Abnormal)
-    {
-        LogWarningThisFunc(("ABNORMAL client termination! (wasBusy=%d)\n",
-                             Global::IsOnlineOrTransient(lastState)));
-
-        /* reset the state to Aborted */
-        if (mData->mMachineState != MachineState_Aborted)
-            setMachineState(MachineState_Aborted);
-    }
-
-    // any machine settings modified?
-    if (m_flModifications)
-    {
-        LogWarningThisFunc(("Discarding unsaved settings changes!\n"));
-        rollback(false /* aNotify */);
-    }
-
-    Assert(mSnapshotData.mStateFilePath.isEmpty() || !mSnapshotData.mSnapshot);
-    if (!mSnapshotData.mStateFilePath.isEmpty())
-    {
-        LogWarningThisFunc(("canceling failed save state request!\n"));
-        endSavingState(FALSE /* aSuccess  */);
-    }
-    else if (!mSnapshotData.mSnapshot.isNull())
-    {
-        LogWarningThisFunc(("canceling untaken snapshot!\n"));
-
-        /* delete all differencing hard disks created (this will also attach
-         * their parents back by rolling back mMediaData) */
-        rollbackMedia();
-        /* delete the saved state file (it might have been already created) */
-        if (mSnapshotData.mSnapshot->stateFilePath().length())
-            RTFileDelete(mSnapshotData.mSnapshot->stateFilePath().c_str());
-
-        mSnapshotData.mSnapshot->uninit();
-    }
-
 #ifdef VBOX_WITH_USB
-    /* release all captured USB devices */
+    // release all captured USB devices, but do this before requesting the locks below
     if (aReason == Uninit::Abnormal && Global::IsOnline(lastState))
     {
@@ -9209 +9169 @@
     }
 #endif /* VBOX_WITH_USB */
+
+    // we need to lock this object in uninit() because the lock is shared
+    // with mPeer (as well as data we modify below). mParent->addProcessToReap()
+    // and others need mParent lock, and USB needs host lock.
+    AutoMultiWriteLock3 multilock(mParent, mParent->host(), this COMMA_LOCKVAL_SRC_POS);
+
+#ifdef VBOX_WITH_RESOURCE_USAGE_API
+    unregisterMetrics(mParent->performanceCollector(), mPeer);
+#endif /* VBOX_WITH_RESOURCE_USAGE_API */
+
+    if (aReason == Uninit::Abnormal)
+    {
+        LogWarningThisFunc(("ABNORMAL client termination! (wasBusy=%d)\n",
+                             Global::IsOnlineOrTransient(lastState)));
+
+        /* reset the state to Aborted */
+        if (mData->mMachineState != MachineState_Aborted)
+            setMachineState(MachineState_Aborted);
+    }
+
+    // any machine settings modified?
+    if (m_flModifications)
+    {
+        LogWarningThisFunc(("Discarding unsaved settings changes!\n"));
+        rollback(false /* aNotify */);
+    }
+
+    Assert(mSnapshotData.mStateFilePath.isEmpty() || !mSnapshotData.mSnapshot);
+    if (!mSnapshotData.mStateFilePath.isEmpty())
+    {
+        LogWarningThisFunc(("canceling failed save state request!\n"));
+        endSavingState(FALSE /* aSuccess  */);
+    }
+    else if (!mSnapshotData.mSnapshot.isNull())
+    {
+        LogWarningThisFunc(("canceling untaken snapshot!\n"));
+
+        /* delete all differencing hard disks created (this will also attach
+         * their parents back by rolling back mMediaData) */
+        rollbackMedia();
+        /* delete the saved state file (it might have been already created) */
+        if (mSnapshotData.mSnapshot->stateFilePath().length())
+            RTFileDelete(mSnapshotData.mSnapshot->stateFilePath().c_str());
+
+        mSnapshotData.mSnapshot->uninit();
+    }
 
     if (!mData->mSession.mType.isEmpty())
@@ -9326 +9332 @@
 
     /* leave the exclusive lock before setting the below two to NULL */
-    alock.leave();
+    multilock.leave();
 
     unconst(mParent) = NULL;
@@ -10818 +10824 @@
         /* Make sure any transient guest properties get removed from the
          * property store on shutdown. */
-        
+
         HWData::GuestPropertyList::iterator it;
         BOOL fNeedsSaving = mHWData->mGuestPropertiesModified;