Changeset 38709 in vbox for trunk/src/VBox

Timestamp:

Sep 9, 2011 4:45:50 PM (13 years ago)

Author:

vboxsync

Message:

Main/Medium: replace the event semaphore used to wait for queryInfo by a RW semaphore

File:

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

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

@@ -88 +88 @@
           readers(0),
           preLockState(MediumState_NotCreated),
-          queryInfoSem(NIL_RTSEMEVENTMULTI),
+          queryInfoSem(NIL_RTSEMRW),
           queryInfoRunning(false),
           type(MediumType_Normal),
@@ -124 +124 @@
     MediumState_T preLockState;
 
-    RTSEMEVENTMULTI queryInfoSem;
+    /** Special synchronization for operations which must wait for queryInfo()
+     * in another thread to complete. Using a SemRW is not quite ideal, but at
+     * least it is subject to the lock validator, unlike the SemEventMulti
+     * which we had here for many years. Catching possible deadlocks is more
+     * important than a tiny bit of efficiency. */
+    RTSEMRW queryInfoSem;
     bool queryInfoRunning : 1;
 
@@ -886 +891 @@
     AssertRCReturn(vrc, E_FAIL);
 
-    vrc = RTSemEventMultiCreate(&m->queryInfoSem);
-    AssertRCReturn(vrc, E_FAIL);
-    vrc = RTSemEventMultiSignal(m->queryInfoSem);
+    vrc = RTSemRWCreate(&m->queryInfoSem);
     AssertRCReturn(vrc, E_FAIL);
 
@@ -1369 +1372 @@
     }
 
-    RTSemEventMultiSignal(m->queryInfoSem);
-    RTSemEventMultiDestroy(m->queryInfoSem);
-    m->queryInfoSem = NIL_RTSEMEVENTMULTI;
+    RTSemRWDestroy(m->queryInfoSem);
+    m->queryInfoSem = NIL_RTSEMRW;
 
     unconst(m->pVirtualBox) = NULL;
@@ -2094 +2096 @@
     {
         alock.leave();
-        RTSemEventMultiWait(m->queryInfoSem, RT_INDEFINITE_WAIT);
+        RTSemRWRequestRead(m->queryInfoSem, RT_INDEFINITE_WAIT);
+        RTSemRWReleaseRead(m->queryInfoSem);
         alock.enter();
     }
@@ -2200 +2203 @@
     {
         alock.leave();
-        RTSemEventMultiWait(m->queryInfoSem, RT_INDEFINITE_WAIT);
+        RTSemRWRequestRead(m->queryInfoSem, RT_INDEFINITE_WAIT);
+        RTSemRWReleaseRead(m->queryInfoSem);
         alock.enter();
     }
@@ -5278 +5282 @@
                || m->state == MediumState_LockedWrite);
 
-        alock.leave();
-        vrc = RTSemEventMultiWait(m->queryInfoSem, RT_INDEFINITE_WAIT);
-        alock.enter();
-
-        AssertRC(vrc);
+        while (m->queryInfoRunning)
+        {
+            alock.leave();
+            vrc = RTSemRWRequestRead(m->queryInfoSem, RT_INDEFINITE_WAIT);
+            RTSemRWReleaseRead(m->queryInfoSem);
+            AssertRC(vrc);
+            alock.enter();
+        }
 
         return S_OK;
@@ -5335 +5342 @@
     bool fRepairImageZeroParentUuid = false;
 
-    /* leave the lock before a lengthy operation */
-    vrc = RTSemEventMultiReset(m->queryInfoSem);
-    AssertRCReturn(vrc, E_FAIL);
+    /* leave the object lock before a lengthy operation */
     m->queryInfoRunning = true;
     alock.leave();
+    /* Note that taking the queryInfoSem after leaving the object lock above
+     * can lead to short spinning of the loops waiting for queryInfo() to
+     * complete. This is unavoidable since the other order causes a lock order
+     * 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);
 
     try
@@ -5570 +5582 @@
     }
 
-    /* inform other callers if there are any */
-    RTSemEventMultiSignal(m->queryInfoSem);
+    /* unblock anyone waiting for the queryInfo results */
+    RTSemRWReleaseWrite(m->queryInfoSem);
     m->queryInfoRunning = false;