Changeset 40487 in vbox for trunk

Timestamp:

Mar 15, 2012 3:45:42 PM (13 years ago)

Author:

vboxsync

Message:

Main/VirtualBox+Machine+Snapshot+Medium: fix lock order issues introduced by new style of medium registry updates

Location:

trunk/src/VBox/Main/src-server

Files:

• MachineImpl.cpp (modified) (4 diffs)
• MachineImplCloneVM.cpp (modified) (1 diff)
• MediumImpl.cpp (modified) (8 diffs)
• SnapshotImpl.cpp (modified) (4 diffs)
• VirtualBoxImpl.cpp (modified) (1 diff)

trunk/src/VBox/Main/src-server/MachineImpl.cpp

@@ -3802 +3802 @@
             // put the parent in the machine registry then
             mediumLock.release();
+            treeLock.release();
             addMediumToRegistry(medium);
+            treeLock.acquire();
             mediumLock.acquire();
             medium->getFirstRegistryMachineId(uuidRegistryParent);
@@ -3895 +3897 @@
 
         mediumLock.release();
+        treeLock.release();
         addMediumToRegistry(medium);
+        treeLock.acquire();
         mediumLock.acquire();
     }
@@ -4303 +4307 @@
                                  aControllerPort,
                                  aDevice);
-        AutoWriteLock attLock(pAttach COMMA_LOCKVAL_SRC_POS);
         if (!oldmedium.isNull())
             oldmedium->removeBackReference(mData->mUuid);
@@ -4311 +4314 @@
 
             mediumLock.release();
+            multiLock.release();
             addMediumToRegistry(pMedium);
+            multiLock.acquire();
             mediumLock.acquire();
         }
 
+        AutoWriteLock attLock(pAttach COMMA_LOCKVAL_SRC_POS);
         pAttach->updateMedium(pMedium);
     }

trunk/src/VBox/Main/src-server/MachineImplCloneVM.cpp

@@ -1271 +1271 @@
                     AutoReadLock mlock2(pParent COMMA_LOCKVAL_SRC_POS);
                     if (pParent->getFirstRegistryMachineId(uuid))
+                    {
+                        mlock2.release();
+                        trgLock.release();
+                        srcLock.release();
                         p->mParent->markRegistryModified(uuid);
+                        srcLock.acquire();
+                        trgLock.acquire();
+                        mlock2.acquire();
+                    }
                 }
                 mlock.acquire();

trunk/src/VBox/Main/src-server/MediumImpl.cpp

@@ -1742 +1742 @@
 
     // save the settings
-    markRegistriesModified();
     mlock.release();
     treeLock.release();
+    markRegistriesModified();
     m->pVirtualBox->saveModifiedRegistries();
 
@@ -1886 +1886 @@
 
         // save the settings
+        mlock.release();
         markRegistriesModified();
-        mlock.release();
         m->pVirtualBox->saveModifiedRegistries();
     }
@@ -2349 +2349 @@
 
     // save the settings
+    mlock.release();
     markRegistriesModified();
-    mlock.release();
     m->pVirtualBox->saveModifiedRegistries();
 
@@ -2426 +2426 @@
 
     // save the settings
+    mlock.release();
     markRegistriesModified();
-    mlock.release();
     m->pVirtualBox->saveModifiedRegistries();
 
@@ -2581 +2581 @@
         /* since this medium has been just created it isn't associated yet */
         diff->m->llRegistryIDs.push_back(parentMachineRegistry);
+        alock.release();
         diff->markRegistriesModified();
+        alock.acquire();
     }
 
@@ -3324 +3326 @@
     if (FAILED(autoCaller.rc())) return;
 
-    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
-
-    for (GuidList::const_iterator it = m->llRegistryIDs.begin();
-         it != m->llRegistryIDs.end();
+    // Get local copy, as keeping the lock over VirtualBox::markRegistryModified
+    // causes trouble with the lock order
+    GuidList llRegistryIDs;
+    {
+        AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
+        llRegistryIDs = m->llRegistryIDs;
+    }
+
+    for (GuidList::const_iterator it = llRegistryIDs.begin();
+         it != llRegistryIDs.end();
          ++it)
     {
@@ -6528 +6536 @@
         m->variant = variant;
 
+        thisLock.release();
         markRegistriesModified();
         if (task.isAsync())
         {
             // in asynchronous mode, save settings now
-            thisLock.release();
             m->pVirtualBox->saveModifiedRegistries();
         }
@@ -6732 +6740 @@
     --m->numCreateDiffTasks;
 
+    mediaLock.release();
     markRegistriesModified();
     if (task.isAsync())
     {
         // in asynchronous mode, save settings now
-        mediaLock.release();
         m->pVirtualBox->saveModifiedRegistries();
     }

trunk/src/VBox/Main/src-server/SnapshotImpl.cpp

@@ -1940 +1940 @@
         }
 
+        // release the locks before updating registry and deleting image files
+        alock.release();
+
         // save machine settings, reset the modified flag and commit;
         bool fNeedsGlobalSaveSettings = false;
@@ -1950 +1953 @@
         mParent->markRegistryModified(mParent->getGlobalRegistryId());
 
-        // let go of the locks while we're deleting image files below
-        alock.release();
         // from here on we cannot roll back on failure any more
 
@@ -2493 +2494 @@
 
                 // machine will need saving now
+                machineLock.release();
                 mParent->markRegistryModified(getId());
             }
@@ -2685 +2687 @@
             aTask.pSnapshot->uninit();
 
+            machineLock.release();
             mParent->markRegistryModified(getId());
         }

trunk/src/VBox/Main/src-server/VirtualBoxImpl.cpp

@@ -3829 +3829 @@
                     pMedium->addRegistry(*puuidBetter, true /* fRecurse */);
                     // 3) and make sure the registry is saved below
+                    mlock.release();
+                    tlock.release();
                     markRegistryModified(*puuidBetter);
+                    tlock.acquire();
+                    mlock.release();
                 }
             }