Changeset 25280 in vbox for trunk

Timestamp:

Dec 9, 2009 6:15:54 PM (15 years ago)

Author:

vboxsync

Message:

Main: preparation for deadlock detection: prohibit recursive locking from same AutoLock object (never made sense in the first place), remove dead code

File:

• trunk/src/VBox/Main/include/AutoLock.h (modified) (10 diffs)

trunk/src/VBox/Main/include/AutoLock.h

@@ -319 +319 @@
     AutoLockBase(LockHandle *pHandle)
         : mHandle(pHandle),
-          mLockLevel(0),
-          mGlobalLockLevel(0)
+          m_fIsLocked(false),
+          m_cUnlockedInLeave(0)
     { }
 
-    LockHandle *mHandle;
-    uint32_t mLockLevel;
-    uint32_t mGlobalLockLevel;
+    LockHandle      *mHandle;
+    bool            m_fIsLocked;
+    uint32_t        m_cUnlockedInLeave;   // how many times the handle was unlocked in leave(); otherwise 0
 };
 
@@ -401 +401 @@
     ~AutoWriteLock()
     {
+        cleanup();
+    }
+
+    void cleanup()
+    {
         if (mHandle)
         {
-            if (mGlobalLockLevel)
+            if (m_cUnlockedInLeave)
             {
-                mGlobalLockLevel -= mLockLevel;
-                mLockLevel = 0;
-                for (; mGlobalLockLevel; --mGlobalLockLevel)
+                // there was a leave() before the destruction: then restore the
+                // lock level that might have been set by locks other than our own
+                if (m_fIsLocked)
+                    --m_cUnlockedInLeave;       // no lock for our own
+                m_fIsLocked = false;
+                for (; m_cUnlockedInLeave; --m_cUnlockedInLeave)
                     mHandle->lockWrite();
+
+                    // @todo r=dj is this really desirable behavior? maybe leave/enter should go altogether?
             }
 
-            AssertMsg(mLockLevel <= 1, ("Lock level > 1: %d\n", mLockLevel));
-            for (; mLockLevel; --mLockLevel)
+            if (m_fIsLocked)
                 mHandle->unlockWrite();
         }
@@ -428 +437 @@
         if (mHandle)
         {
+            AssertMsg(!m_fIsLocked, ("m_fIsLocked is true, attempting to lock twice!"));
             mHandle->lockWrite();
-            ++mLockLevel;
-            Assert(mLockLevel != 0 /* overflow? */);
+            m_fIsLocked = true;
         }
     }
@@ -444 +453 @@
         if (mHandle)
         {
-            AssertReturnVoid(mLockLevel != 0 /* unlock() w/o preceding lock()? */);
+            AssertMsg(m_fIsLocked, ("m_fIsLocked is false, cannot release!"));
             mHandle->unlockWrite();
-            --mLockLevel;
+            m_fIsLocked = false;
         }
     }
@@ -475 +484 @@
         if (mHandle)
         {
-            AssertReturnVoid(mLockLevel != 0 /* leave() w/o preceding lock()? */);
-            AssertReturnVoid(mGlobalLockLevel == 0 /* second leave() in a row? */);
-
-            mGlobalLockLevel = mHandle->writeLockLevel();
-            AssertReturnVoid(mGlobalLockLevel >= mLockLevel /* logic error! */);
-
-            for (uint32_t left = mGlobalLockLevel; left; --left)
+            AssertMsg(m_fIsLocked, ("m_fIsLocked is false, cannot leave()!"));
+            AssertMsg(m_cUnlockedInLeave == 0, ("m_cUnlockedInLeave is %d, must be 0! Called leave() twice?", m_cUnlockedInLeave));
+
+            m_cUnlockedInLeave = mHandle->writeLockLevel();
+            AssertMsg(m_cUnlockedInLeave >= 1, ("m_cUnlockedInLeave is %d, must be >=1!", m_cUnlockedInLeave));
+
+            for (uint32_t left = m_cUnlockedInLeave;
+                 left;
+                 --left)
                 mHandle->unlockWrite();
         }
@@ -518 +529 @@
         if (mHandle)
         {
-            AssertReturnVoid(mLockLevel != 0 /* enter() w/o preceding lock()+leave()? */);
-            AssertReturnVoid(mGlobalLockLevel != 0 /* enter() w/o preceding leave()? */);
-
-            for (; mGlobalLockLevel; --mGlobalLockLevel)
+            AssertMsg(m_fIsLocked, ("m_fIsLocked is false, cannot enter()!"));
+            AssertMsg(m_cUnlockedInLeave != 0, ("m_cUnlockedInLeave is 0! enter() without leave()?"));
+
+            for (; m_cUnlockedInLeave; --m_cUnlockedInLeave)
                 mHandle->lockWrite();
         }
@@ -541 +552 @@
         if (mHandle != aHandle)
         {
-            uint32_t lockLevel = mLockLevel;
-
-            /* perform the destructor part */
+            bool fWasLocked = m_fIsLocked;
+
+            cleanup();
+
+            mHandle = aHandle;
+            m_fIsLocked = fWasLocked;
+
             if (mHandle)
-            {
-                if (mGlobalLockLevel)
-                {
-                    mGlobalLockLevel -= mLockLevel;
-                    mLockLevel = 0;
-                    for (; mGlobalLockLevel; --mGlobalLockLevel)
-                        mHandle->lockWrite();
-                }
-
-                AssertMsg(mLockLevel <= 1, ("Lock level > 1: %d\n", mLockLevel));
-                for (; mLockLevel; --mLockLevel)
-                    mHandle->unlockWrite();
-            }
-
-            mHandle = aHandle;
-            mLockLevel = lockLevel;
-
-            if (mHandle)
-                for (; lockLevel; --lockLevel)
+                if (fWasLocked)
                     mHandle->lockWrite();
         }
@@ -697 +694 @@
         if (mHandle)
         {
-            AssertMsg(mLockLevel <= 1, ("Lock level > 1: %d\n", mLockLevel));
-            for (; mLockLevel; --mLockLevel)
+            if (m_fIsLocked)
                 mHandle->unlockRead();
         }
@@ -719 +715 @@
         if (mHandle)
         {
+            AssertMsg(!m_fIsLocked, ("m_fIsLocked is true, attempting to lock twice!"));
             mHandle->lockRead();
-            ++mLockLevel;
-            Assert(mLockLevel != 0 /* overflow? */);
+            m_fIsLocked = true;
         }
     }
@@ -735 +731 @@
         if (mHandle)
         {
-            AssertReturnVoid(mLockLevel != 0 /* unlock() w/o preceding lock()? */);
+            AssertMsg(m_fIsLocked, ("m_fIsLocked is false, cannot release!"));
             mHandle->unlockRead();
-            --mLockLevel;
+            m_fIsLocked = false;
         }
-    }
-
-    /**
-     * Attaches another handle to this auto lock instance.
-     *
-     * The previous object's lock is completely released before the new one is
-     * acquired. The lock level of the new handle will be the same. This also
-     * means that if the lock was not acquired at all before #attach(), it will
-     * not be acquired on the new handle too.
-     *
-     * @param aHandle   New handle to attach.
-     */
-    void attach(LockHandle *aHandle)
-    {
-        /* detect simple self-reattachment */
-        if (mHandle != aHandle)
-        {
-            uint32_t lockLevel = mLockLevel;
-            if (mHandle)
-                for (; mLockLevel; --mLockLevel)
-                    mHandle->unlockRead();
-            mHandle = aHandle;
-            mLockLevel = lockLevel;
-            if (mHandle)
-                for (; lockLevel; --lockLevel)
-                    mHandle->lockRead();
-        }
-    }
-
-    /** @see attach (LockHandle *) */
-    void attach(LockHandle &aHandle)
-    {
-        attach(&aHandle);
-    }
-
-    /** @see attach (LockHandle *) */
-    void attach(const Lockable &aLockable)
-    {
-        attach(aLockable.lockHandle());
-    }
-
-    /** @see attach (LockHandle *) */
-    void attach(const Lockable *aLockable)
-    {
-        attach(aLockable ? aLockable->lockHandle() : NULL);
     }