Changeset 30377 in vbox

Timestamp:

Jun 22, 2010 2:01:01 PM (15 years ago)

Author:

vboxsync

Message:

Main: do not hold any other lock while calling VirtualBox::saveSettings (mostly comments, only real change is in DHCPServer); also, VirtualBox lock is not needed in SessionMachine::endSavingState

Location:

trunk/src/VBox/Main

Files:

• DHCPServerImpl.cpp (modified) (3 diffs)
• HostImpl.cpp (modified) (3 diffs)
• MachineImpl.cpp (modified) (5 diffs)
• MediumImpl.cpp (modified) (7 diffs)
• SnapshotImpl.cpp (modified) (1 diff)
• VirtualBoxImpl.cpp (modified) (3 diffs)

trunk/src/VBox/Main/DHCPServerImpl.cpp

@@ -150 +150 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    /* VirtualBox::saveSettings() needs a write lock */
-    AutoMultiWriteLock2 alock(mVirtualBox, this COMMA_LOCKVAL_SRC_POS);
-
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
     m.enabled = aEnabled;
 
+    // save the global settings; for that we should hold only the VirtualBox lock
+    alock.release();
+    AutoWriteLock vboxLock(mVirtualBox COMMA_LOCKVAL_SRC_POS);
     HRESULT rc = mVirtualBox->saveSettings();
 
@@ -218 +219 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
-    /* VirtualBox::saveSettings() needs a write lock */
-    AutoMultiWriteLock2 alock(mVirtualBox, this COMMA_LOCKVAL_SRC_POS);
-
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
     m.IPAddress = aIPAddress;
     m.networkMask = aNetworkMask;
@@ -226 +225 @@
     m.upperIP = aUpperIP;
 
+    // save the global settings; for that we should hold only the VirtualBox lock
+    alock.release();
+    AutoWriteLock vboxLock(mVirtualBox COMMA_LOCKVAL_SRC_POS);
     return mVirtualBox->saveSettings();
 }

trunk/src/VBox/Main/HostImpl.cpp

@@ -1143 +1143 @@
     }
 
-    /* save the global settings */
+    // save the global settings; for that we should hold only the VirtualBox lock
     alock.release();
     AutoWriteLock vboxLock(m->pParent COMMA_LOCKVAL_SRC_POS);
@@ -1199 +1199 @@
     }
 
-    /* save the global settings */
+    // save the global settings; for that we should hold only the VirtualBox lock
     alock.release();
     AutoWriteLock vboxLock(m->pParent COMMA_LOCKVAL_SRC_POS);
@@ -1764 +1764 @@
 
         // save the global settings... yeah, on every single filter property change
+        // for that we should hold only the VirtualBox lock
         alock.release();
         AutoWriteLock vboxLock(m->pParent COMMA_LOCKVAL_SRC_POS);

trunk/src/VBox/Main/MachineImpl.cpp

@@ -3163 +3163 @@
     if (fNeedsSaveSettings)
     {
+        // save the global settings; for that we should hold only the VirtualBox lock
         mediumLock.release();
         treeLock.leave();
@@ -3257 +3258 @@
         if (fNeedsGlobalSaveSettings)
         {
+            // save the global settings; for that we should hold only the VirtualBox lock
             alock.release();
             AutoWriteLock vboxlock(this COMMA_LOCKVAL_SRC_POS);
@@ -3656 +3658 @@
         if (fNeedsGlobalSaveSettings)
         {
+            // save the global settings; for that we should hold only the VirtualBox lock
             alock.release();
             AutoWriteLock vboxlock(mParent COMMA_LOCKVAL_SRC_POS);
@@ -3691 +3694 @@
     if (SUCCEEDED(rc) && fNeedsGlobalSaveSettings)
     {
+        // save the global settings; for that we should hold only the VirtualBox lock
         AutoWriteLock vlock(mParent COMMA_LOCKVAL_SRC_POS);
         rc = mParent->saveSettings();
@@ -10799 +10803 @@
     AssertComRCReturn(autoCaller.rc(), autoCaller.rc());
 
-    /* saveSettings() needs mParent lock */
-    AutoMultiWriteLock2 alock(mParent, this COMMA_LOCKVAL_SRC_POS);
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     HRESULT rc = S_OK;

trunk/src/VBox/Main/MediumImpl.cpp

@@ -1443 +1443 @@
     m->type = aType;
 
+    // save the global settings; for that we should hold only the VirtualBox lock
     mlock.release();
-
-    // saveSettings needs vbox lock
     AutoWriteLock alock(m->pVirtualBox COMMA_LOCKVAL_SRC_POS);
-
     HRESULT rc = m->pVirtualBox->saveSettings();
 
@@ -1572 +1570 @@
         m->autoReset = !!aAutoReset;
 
+        // save the global settings; for that we should hold only the VirtualBox lock
         mlock.release();
-
-        // saveSettings needs vbox lock
         AutoWriteLock alock(m->pVirtualBox COMMA_LOCKVAL_SRC_POS);
-
         return m->pVirtualBox->saveSettings();
     }
@@ -2014 +2010 @@
         it->second = aValue;
 
+    // save the global settings; for that we should hold only the VirtualBox lock
     mlock.release();
-
-    // saveSettings needs vbox lock
     AutoWriteLock alock(m->pVirtualBox COMMA_LOCKVAL_SRC_POS);
     HRESULT rc = m->pVirtualBox->saveSettings();
@@ -2179 +2174 @@
 
     bool fNeedsSaveSettings = false;
-    ComObjPtr <Progress> pProgress;
+    ComObjPtr<Progress> pProgress;
 
     HRESULT rc = deleteStorage(&pProgress,
@@ -5210 +5205 @@
                                task.mVDOperationIfaces);
             if (RT_FAILURE(vrc))
-            {
                 throw setError(E_FAIL,
                             tr("Could not create the hard disk storage unit '%s'%s"),
                             location.raw(), vdError(vrc).raw());
-            }
 
             size = VDGetFileSize(hdd, 0);
@@ -5441 +5434 @@
         if (fNeedsSaveSettings)
         {
+            // save the global settings; for that we should hold only the VirtualBox lock
             mediaLock.release();
             AutoWriteLock vboxlock(m->pVirtualBox COMMA_LOCKVAL_SRC_POS);
@@ -5727 +5721 @@
     {
         // in asynchronous mode, save settings now
+        // for that we should hold only the VirtualBox lock
         AutoWriteLock vboxlock(m->pVirtualBox COMMA_LOCKVAL_SRC_POS);
         m->pVirtualBox->saveSettings();

trunk/src/VBox/Main/SnapshotImpl.cpp

@@ -1530 +1530 @@
     mSnapshotData.mSnapshot.setNull();
 
-    // save VirtualBox.xml (media registry most probably changed with diff image)
+    // save VirtualBox.xml (media registry most probably changed with diff image);
+    // for that we should hold only the VirtualBox lock
     machineLock.release();
     AutoWriteLock vboxLock(mParent COMMA_LOCKVAL_SRC_POS);

trunk/src/VBox/Main/VirtualBoxImpl.cpp

@@ -3531 +3531 @@
  *  places internally when settings need saving.
  *
- *  @note Caller must have locked the VirtualBox object for writing!
+ *  @note Caller must have locked the VirtualBox object for writing and must not hold any
+ *    other locks since this locks all kinds of member objects and trees temporarily,
+ *    which could cause conflicts.
  */
 HRESULT VirtualBox::saveSettings()
@@ -4792 +4794 @@
     rc = FindDHCPServerByNetworkName(name, existing.asOutParam());
     if (SUCCEEDED(rc))
-    {
         return E_INVALIDARG;
-    }
 
     rc = S_OK;
@@ -4836 +4836 @@
     AssertComRCReturn(autoCaller.rc(), autoCaller.rc());
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
     AutoCaller dhcpServerCaller(aDHCPServer);
     AssertComRCReturn(dhcpServerCaller.rc(), dhcpServerCaller.rc());