Changeset 38717 in vbox for trunk/src/VBox

Timestamp:

Sep 12, 2011 2:48:50 PM (13 years ago)

Author:

vboxsync

Message:

Main/Medium+Machine+AutoLock: Integrate the queryInfo semaphore into the AutoLock infrastructure with the appropriate lock class. Adjust the locking rules for queryInfo and start propagating them up to the callers. Needs more work, but this is comparably simple: just get the mediaTree lock in the caller if the assert triggers.

Location:

trunk/src/VBox/Main

Files:

• glue/AutoLock.cpp (modified) (2 diffs)
• src-server/MachineImpl.cpp (modified) (1 diff)
• src-server/MediumImpl.cpp (modified) (16 diffs)

trunk/src/VBox/Main/glue/AutoLock.cpp

@@ -5 +5 @@
 
 /*
- * Copyright (C) 2006-2010 Oracle Corporation
+ * Copyright (C) 2006-2011 Oracle Corporation
  *
  * This file is part of VirtualBox Open Source Edition (OSE), as
@@ -73 +73 @@
         { LOCKCLASS_LISTOFMEDIA,        "7-LISTOFMEDIA" },
         { LOCKCLASS_LISTOFOTHEROBJECTS, "8-LISTOFOTHEROBJECTS" },
-        { LOCKCLASS_OTHEROBJECT,        "9-OTHEROBJECT" },
-        { LOCKCLASS_USBLIST,            "10-USBLIST" },
-        { LOCKCLASS_PROGRESSLIST,       "11-PROGRESSLIST" },
-        { LOCKCLASS_OBJECTSTATE,        "12-OBJECTSTATE" }
+        { LOCKCLASS_MEDIUMQUERY,        "9-MEDIUMQUERY" },
+        { LOCKCLASS_OTHEROBJECT,        "10-OTHEROBJECT" },
+        { LOCKCLASS_USBLIST,            "11-USBLIST" },
+        { LOCKCLASS_PROGRESSLIST,       "12-PROGRESSLIST" },
+        { LOCKCLASS_OBJECTSTATE,        "13-OBJECTSTATE" }
     };
 

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

@@ -12513 +12513 @@
     AssertComRCReturn(autoCaller.rc(), autoCaller.rc());
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+    AutoMultiWriteLock2 alock(this->lockHandle(),
+                              &mParent->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
 
     AssertReturn(   mData->mMachineState == MachineState_Starting

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

@@ -88 +88 @@
           readers(0),
           preLockState(MediumState_NotCreated),
-          queryInfoSem(NIL_RTSEMRW),
+          queryInfoSem(LOCKCLASS_MEDIUMQUERY),
           queryInfoRunning(false),
           type(MediumType_Normal),
@@ -129 +129 @@
      * which we had here for many years. Catching possible deadlocks is more
      * important than a tiny bit of efficiency. */
-    RTSEMRW queryInfoSem;
+    RWLockHandle queryInfoSem;
     bool queryInfoRunning : 1;
 
@@ -891 +891 @@
     AssertRCReturn(vrc, E_FAIL);
 
-    vrc = RTSemRWCreate(&m->queryInfoSem);
-    AssertRCReturn(vrc, E_FAIL);
-
     return BaseFinalConstruct();
 }
@@ -1372 +1369 @@
     }
 
-    RTSemRWDestroy(m->queryInfoSem);
-    m->queryInfoSem = NIL_RTSEMRW;
-
     unconst(m->pVirtualBox) = NULL;
 }
@@ -2008 +2002 @@
 
     /* queryInfo() locks this for writing. */
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+    AutoMultiWriteLock2 alock(&m->pVirtualBox->getMediaTreeLockHandle(),
+                              this->lockHandle() COMMA_LOCKVAL_SRC_POS);
 
     HRESULT rc = S_OK;
@@ -2096 +2091 @@
     {
         alock.leave();
-        RTSemRWRequestRead(m->queryInfoSem, RT_INDEFINITE_WAIT);
-        RTSemRWReleaseRead(m->queryInfoSem);
+        {
+            AutoReadLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
+        }
         alock.enter();
     }
@@ -2203 +2199 @@
     {
         alock.leave();
-        RTSemRWRequestRead(m->queryInfoSem, RT_INDEFINITE_WAIT);
-        RTSemRWReleaseRead(m->queryInfoSem);
+        {
+            AutoReadLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
+        }
         alock.enter();
     }
@@ -3761 +3758 @@
  * Constructs a medium lock list for this medium. The lock is not taken.
  *
- * @note Locks the medium tree for reading.
+ * @note Caller must lock the medium tree for writing.
  *
  * @param fFailIfInaccessible If true, this fails with an error if a medium is inaccessible. If false,
@@ -3775 +3772 @@
                                      MediumLockList &mediumLockList)
 {
+    // Medium::queryInfo needs write lock
+    Assert(m->pVirtualBox->getMediaTreeLockHandle().isWriteLockOnCurrentThread());
+
     AutoCaller autoCaller(this);
     if (FAILED(autoCaller.rc())) return autoCaller.rc();
 
     HRESULT rc = S_OK;
-
-    /* we access parent medium objects */
-    AutoReadLock treeLock(m->pVirtualBox->getMediaTreeLockHandle() COMMA_LOCKVAL_SRC_POS);
 
     /* paranoid sanity checking if the medium has a to-be parent medium */
@@ -3797 +3794 @@
     while (!pMedium.isNull())
     {
-        // need write lock for RefreshState if medium is inaccessible
+        // need write lock for queryInfo if medium is inaccessible
         AutoWriteLock alock(pMedium COMMA_LOCKVAL_SRC_POS);
 
@@ -3806 +3803 @@
         if (mediumState == MediumState_Inaccessible)
         {
-            rc = pMedium->RefreshState(&mediumState);
+            rc = pMedium->queryInfo(false /* fSetImageId */, false /* fSetParentId */);
             if (FAILED(rc)) return rc;
 
+            mediumState = pMedium->getState();
             if (mediumState == MediumState_Inaccessible)
             {
@@ -5254 +5252 @@
  *       accessibility.
  *
- * @note Locks medium tree for reading and writing (for new diff media checked
- *       for the first time). Locks mParent for reading. Locks this object for
- *       writing.
+ * @note Caller must hold medium tree for writing.
+ *
+ * @note Locks mParent for reading. Locks this object for writing.
  *
  * @param fSetImageId Whether to reset the UUID contained in the image file to the UUID in the medium instance data (see SetIDs())
@@ -5264 +5262 @@
 HRESULT Medium::queryInfo(bool fSetImageId, bool fSetParentId)
 {
+    Assert(m->pVirtualBox->getMediaTreeLockHandle().isWriteLockOnCurrentThread());
     AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
@@ -5285 +5284 @@
         {
             alock.leave();
-            vrc = RTSemRWRequestRead(m->queryInfoSem, RT_INDEFINITE_WAIT);
-            RTSemRWReleaseRead(m->queryInfoSem);
-            AssertRC(vrc);
+            {
+                AutoReadLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
+            }
             alock.enter();
         }
@@ -5350 +5349 @@
      * violation: here it would be requesting the object lock (at the beginning
      * of the method), then SemRW, and below the other way round. */
-    vrc = RTSemRWRequestWrite(m->queryInfoSem, RT_INDEFINITE_WAIT);
-    AssertRCReturn(vrc, E_FAIL);
+    AutoWriteLock qlock(m->queryInfoSem COMMA_LOCKVAL_SRC_POS);
 
     try
@@ -5583 +5581 @@
 
     /* unblock anyone waiting for the queryInfo results */
-    RTSemRWReleaseWrite(m->queryInfoSem);
+    qlock.release();
     m->queryInfoRunning = false;