Changeset 92464 in vbox for trunk/src/VBox/Main/src-server/MediumImpl.cpp

Timestamp:

Nov 16, 2021 5:59:16 PM (4 years ago)

Author:

vboxsync

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

148322

Message:

Main/Medium: Improve a few API calls where a concurrent Medium::queryInfo activity can lead to errors about incorrect medium state which is transient. bugref:10149

File:

• trunk/src/VBox/Main/src-server/MediumImpl.cpp (modified) (6 diffs)

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

@@ -1839 +1839 @@
     AutoWriteLock mlock(this COMMA_LOCKVAL_SRC_POS);
 
+    /* Wait for a concurrently running Medium::i_queryInfo to complete. */
+    while (m->queryInfoRunning)
+    {
+        mlock.release();
+        autoCaller.release();
+        treeLock.release();
+        /* Must not hold the media tree lock, as Medium::i_queryInfo needs
+         * this lock and thus we would run into a deadlock here. */
+        Assert(!m->pVirtualBox->i_getMediaTreeLockHandle().isWriteLockOnCurrentThread());
+        /* must not hold the object lock now */
+        Assert(!isWriteLockOnCurrentThread());
+        {
+            AutoReadLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
+        }
+        treeLock.acquire();
+        autoCaller.add();
+        if (FAILED(autoCaller.rc())) return autoCaller.rc();
+        mlock.acquire();
+    }
+
     switch (m->state)
     {
@@ -2111 +2131 @@
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
+    /* Wait for a concurrently running Medium::i_queryInfo to complete. */
+    if (m->queryInfoRunning)
+    {
+        /* Must not hold the media tree lock, as Medium::i_queryInfo needs this
+         * lock and thus we would run into a deadlock here. */
+        Assert(!m->pVirtualBox->i_getMediaTreeLockHandle().isWriteLockOnCurrentThread());
+        while (m->queryInfoRunning)
+        {
+            alock.release();
+            /* must not hold the object lock now */
+            Assert(!isWriteLockOnCurrentThread());
+            {
+                AutoReadLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
+            }
+            alock.acquire();
+        }
+    }
+
     switch (m->state)
     {
@@ -2498 +2536 @@
 {
     AutoWriteLock mlock(this COMMA_LOCKVAL_SRC_POS);
+
+    /* Wait for a concurrently running Medium::i_queryInfo to complete. */
+    if (m->queryInfoRunning)
+    {
+        /* Must not hold the media tree lock, as Medium::i_queryInfo needs this
+         * lock and thus we would run into a deadlock here. */
+        Assert(!m->pVirtualBox->i_getMediaTreeLockHandle().isWriteLockOnCurrentThread());
+        while (m->queryInfoRunning)
+        {
+            mlock.release();
+            /* must not hold the object lock now */
+            Assert(!isWriteLockOnCurrentThread());
+            {
+                AutoReadLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
+            }
+            mlock.acquire();
+        }
+    }
 
     switch (m->state)
@@ -5230 +5286 @@
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
+    /* Wait for a concurrently running Medium::i_queryInfo to complete. */
+    while (m->queryInfoRunning)
+    {
+        autoCaller.release();
+        multilock.release();
+        /* Must not hold the media tree lock, as Medium::i_queryInfo needs
+         * this lock and thus we would run into a deadlock here. */
+        Assert(!m->pVirtualBox->i_getMediaTreeLockHandle().isWriteLockOnCurrentThread());
+        /* must not hold the object lock now */
+        Assert(!isWriteLockOnCurrentThread());
+        {
+            AutoReadLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
+        }
+        multilock.acquire();
+        autoCaller.add();
+        if (FAILED(autoCaller.rc())) return autoCaller.rc();
+    }
+
     LogFlowFunc(("ENTER for %s\n", i_getLocationFull().c_str()));
 
@@ -5318 +5392 @@
  */
 HRESULT Medium::i_deleteStorage(ComObjPtr<Progress> *aProgress,
-                              bool aWait, bool aNotify)
+                                bool aWait, bool aNotify)
 {
     AssertReturn(aProgress != NULL || aWait == true, E_FAIL);
@@ -5347 +5421 @@
         /** @todo r=klaus would be great if this could be moved to the async
          * part of the operation as it can take quite a while */
-        if (m->queryInfoRunning)
-        {
-            while (m->queryInfoRunning)
-            {
-                alock.release();
-                autoCaller.release();
-                treelock.release();
-                /* Must not hold the media tree lock or the object lock, as
-                 * Medium::i_queryInfo needs this lock and thus we would run
-                 * into a deadlock here. */
-                Assert(!m->pVirtualBox->i_getMediaTreeLockHandle().isWriteLockOnCurrentThread());
-                Assert(!isWriteLockOnCurrentThread());
-                {
-                    AutoReadLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
-                }
-                treelock.acquire();
-                autoCaller.add();
-                AssertComRCThrowRC(autoCaller.rc());
-                alock.acquire();
-            }
+        while (m->queryInfoRunning)
+        {
+            alock.release();
+            autoCaller.release();
+            treelock.release();
+            /* Must not hold the media tree lock or the object lock, as
+             * Medium::i_queryInfo needs this lock and thus we would run
+             * into a deadlock here. */
+            Assert(!m->pVirtualBox->i_getMediaTreeLockHandle().isWriteLockOnCurrentThread());
+            Assert(!isWriteLockOnCurrentThread());
+            {
+                AutoReadLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
+            }
+            treelock.acquire();
+            autoCaller.add();
+            AssertComRCThrowRC(autoCaller.rc());
+            alock.acquire();
         }