Changeset 25748 in vbox

Timestamp:

Jan 12, 2010 10:27:27 AM (15 years ago)

Author:

vboxsync

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

56499

Message:

iprt/cdefs,*: Use RT_LOCK_STRICT and RT_LOCK_STRICT_ORDER for controlling deadlock detection and lock order validation. Currently both are disabled by default, but it's possible to add VBOX_WITH_STRICT_LOCKS=1 to LocalConfig.kmk to enable it all.

Location:

trunk

Files:

• Config.kmk (modified) (2 diffs)
• include/iprt/cdefs.h (modified) (3 diffs)
• include/iprt/cpp/lock.h (modified) (2 diffs)
• include/iprt/critsect.h (modified) (3 diffs)
• include/iprt/lockvalidator.h (modified) (1 diff)
• src/VBox/Main/AutoLock.cpp (modified) (5 diffs)
• src/VBox/Main/webservice/Makefile.kmk (modified) (1 diff)
• src/VBox/Runtime/common/misc/lockvalidator.cpp (modified) (8 diffs)
• src/VBox/Runtime/generic/semrw-generic.cpp (modified) (1 diff)
• src/VBox/Runtime/include/internal/strict.h (modified) (5 diffs)
• src/VBox/Runtime/r3/xml.cpp (modified) (1 diff)
• src/VBox/Runtime/testcase/tstRTLockValidator.cpp (modified) (2 diffs)
• src/VBox/VMM/PDMCritSect.cpp (modified) (1 diff)
• src/VBox/VMM/PDMDevHlp.cpp (modified) (1 diff)
• src/VBox/VMM/PDMInternal.h (modified) (1 diff)

trunk/Config.kmk

@@ -547 +547 @@
  VBOX_WITH_VMMR0_DISABLE_PREEMPTION = 1
 endif
-
-## Enabled the live migration API changes.
-#VBOX_WITH_LIVE_MIGRATION = 1
+## For testing deadlock detection and lock order validation.
+# (Temporary, will be made default for debug and strict builds soon.)
+if1of ($(KBUILD_TARGET),)
+ VBOX_WITH_STRICT_LOCKS = 1
+endif
+
 
 #
@@ -1147 +1150 @@
 ifdef VBOX_PATH_APP_DOCS
  DEFS += RTPATH_APP_DOCS=\"$(VBOX_PATH_APP_DOCS)\"
+endif
+ifdef VBOX_WITH_STRICT_LOCKS
+ DEFS += RT_LOCK_STRICT RT_LOCK_STRICT_ORDER
 endif
 

trunk/include/iprt/cdefs.h

@@ -84 +84 @@
 #define IN_RT_STATIC
 #define RT_STRICT
+#define RT_LOCK_STRICT
+#define RT_LOCK_NO_STRICT
+#define RT_LOCK_STRICT_ORDER
+#define RT_LOCK_NO_STRICT_ORDER
 #define Breakpoint
 #define RT_NO_DEPRECATED_MACROS
@@ -1788 +1792 @@
 #endif
 
+/** @todo remove this: */
+#if !defined(RT_LOCK_STRICT) && !defined(DEBUG_bird)
+# define RT_LOCK_NO_STRICT
+#endif
+#if !defined(RT_LOCK_STRICT_ORDER) && !defined(DEBUG_bird)
+# define RT_LOCK_NO_STRICT_ORDER
+#endif
+
+/** @def RT_LOCK_STRICT
+ * The \#define RT_LOCK_STRICT controls whether deadlock detection and related
+ * checks are done in the lock and semaphore code.  It is by default enabled in
+ * RT_STRICT builds, but this behavior can be overriden by defining
+ * RT_LOCK_NO_STRICT. */
+#if !defined(RT_LOCK_STRICT) && !defined(RT_LOCK_NO_STRICT) && defined(RT_STRICT)
+# define RT_LOCK_STRICT
+#endif
+/** @def RT_LOCK_NO_STRICT
+ * The \#define RT_LOCK_NO_STRICT disables RT_LOCK_STRICT.  */
+#if defined(RT_LOCK_NO_STRICT) && defined(RT_LOCK_STRICT)
+# undef RT_LOCK_STRICT
+#endif
+
+/** @def RT_LOCK_STRICT_ORDER
+ * The \#define RT_LOCK_STRICT_ORDER controls whether locking order is checked
+ * by the lock and semaphore code.  It is by default enabled in RT_STRICT
+ * builds, but this behavior can be overriden by defining
+ * RT_LOCK_NO_STRICT_ORDER. */
+#if !defined(RT_LOCK_STRICT_ORDER) && !defined(RT_LOCK_NO_STRICT_ORDER) && defined(RT_STRICT)
+# define RT_LOCK_STRICT_ORDER
+#endif
+/** @def RT_LOCK_NO_STRICT_ORDER
+ * The \#define RT_LOCK_NO_STRICT_ORDER disables RT_LOCK_STRICT_ORDER.  */
+#if defined(RT_LOCK_NO_STRICT_ORDER) && defined(RT_LOCK_STRICT_ORDER)
+# undef RT_LOCK_STRICT_ORDER
+#endif
+
+
 /** Source position. */
 #define RT_SRC_POS         __FILE__, __LINE__, __PRETTY_FUNCTION__
@@ -1796 +1837 @@
 /** Source position arguments. */
 #define RT_SRC_POS_ARGS    pszFile, iLine, pszFunction
+
+/** Applies NOREF() to the source position arguments. */
+#define RT_SRC_POS_NOREF() do { NOREF(pszFile); NOREF(iLine); NOREF(pszFunction); } while (0)
 
 /** @} */

trunk/include/iprt/cpp/lock.h

@@ -28 +28 @@
  */
 
-#ifndef ___iprt_lock_h
-#define ___iprt_lock_h
+#ifndef ___iprt_cpp_lock_h
+#define ___iprt_cpp_lock_h
 
 #include <iprt/critsect.h>
+#ifdef RT_LOCK_STRICT
+# include <iprt/lockvalidator.h>
+#endif
 
 RT_C_DECLS_BEGIN
@@ -61 +64 @@
         RTLockMtx()
         {
+#ifdef RT_LOCK_STRICT_ORDER
+            RTCritSectInitEx(&mMtx, 0 /*fFlags*/,
+                             RTLockValidatorClassCreateUnique(RT_SRC_POS, NULL),
+                             RTLOCKVAL_SUB_CLASS_NONE, NULL);
+#else
             RTCritSectInit(&mMtx);
+#endif
+        }
+
+        /** Use to when creating locks that belongs in the same "class".  */
+        RTLockMtx(RT_SRC_POS_DECL, uint32_t uSubClass = RTLOCKVAL_SUB_CLASS_NONE)
+        {
+#ifdef RT_LOCK_STRICT_ORDER
+            RTCritSectInitEx(&mMtx, 0 /*fFlags*/,
+                             RTLockValidatorClassForSrcPos(RT_SRC_POS_ARGS, NULL),
+                             uSubClass, NULL);
+#else
+            RTCritSectInit(&mMtx);
+            RT_SRC_POS_NOREF();
+#endif
         }
 

trunk/include/iprt/critsect.h

@@ -37 +37 @@
 # include <iprt/thread.h>
 #endif
-
+#ifdef RT_LOCK_STRICT_ORDER
+# include <iprt/lockvalidator.h>
+#endif
 
 RT_C_DECLS_BEGIN
@@ -336 +338 @@
 }
 
-/* Strict build: Remap the three enter calls to the debug versions. */
-#ifdef RT_STRICT
+/* Lock strict build: Remap the three enter calls to the debug versions. */
+#ifdef RT_LOCK_STRICT
 # ifdef ___iprt_asm_h
 #  define RTCritSectEnter(pCritSect)                        RTCritSectEnterDebug(pCritSect, (uintptr_t)ASMReturnAddress(), RT_SRC_POS)
@@ -349 +351 @@
 #endif
 
+/* Strict lock order: Automatically classify locks by init location. */
+#ifdef RT_LOCK_STRICT_ORDER
+# define RTCritSectInit(pCritSect) \
+    RTCritSectInitEx((pCritSect), 0 /*fFlags*/, \
+                     RTLockValidatorClassForSrcPos(RT_SRC_POS, NULL), \
+                     RTLOCKVAL_SUB_CLASS_NONE, NULL)
+#endif
+
 /** @} */
 

trunk/include/iprt/lockvalidator.h

@@ -901 +901 @@
 
 /**
+ * Creates a new lock validator class with a reference that is consumed by the
+ * first call to RTLockValidatorClassRetain.
+ *
+ * This is tailored for use in the parameter list of a semaphore constructor.
+ *
+ * @returns Class handle with a reference that is automatically consumed by the
+ *          first retainer.  NIL_RTLOCKVALCLASS if we run into trouble.
+ *
+ * @param   pszFile             The source position of the call, file.
+ * @param   iLine               The source position of the call, line.
+ * @param   pszFunction         The source position of the call, function.
+ * @param   pszNameFmt          Class name format string, optional (NULL).  Max
+ *                              length is 32 bytes.
+ * @param   ...                 Format string arguments.
+ */
+RTDECL(RTLOCKVALCLASS) RTLockValidatorClassCreateUnique(RT_SRC_POS_DECL, const char *pszNameFmt, ...);
+
+/**
  * Finds a class for the specified source position.
  *

trunk/src/VBox/Main/AutoLock.cpp

@@ -259 +259 @@
  */
 
-    int vrc = RTSemRWCreate(&m->sem);
+    int vrc = RTSemRWCreateEx(&m->sem, 0 /*fFlags*/, NIL_RTLOCKVALCLASS, RTLOCKVAL_SUB_CLASS_NONE, NULL);
     AssertRC(vrc);
 
@@ -283 +283 @@
     validateLock(LOCKVAL_SRC_POS_ARGS);
 #endif
-#if defined(RT_STRICT) && defined(VBOX_WITH_DEBUG_LOCK_VALIDATOR)
+#if defined(RT_LOCK_STRICT) && defined(VBOX_WITH_DEBUG_LOCK_VALIDATOR)
     int vrc = RTSemRWRequestWriteDebug(m->sem, RT_INDEFINITE_WAIT, (uintptr_t)ASMReturnAddress(), RT_SRC_POS_ARGS);
-#elif defined(RT_STRICT)
+#elif defined(RT_LOCK_STRICT)
     int vrc = RTSemRWRequestWriteDebug(m->sem, RT_INDEFINITE_WAIT, (uintptr_t)ASMReturnAddress(), RT_SRC_POS);
 #else
@@ -308 +308 @@
     validateLock(LOCKVAL_SRC_POS_ARGS);
 #endif
-#if defined(RT_STRICT) && defined(VBOX_WITH_DEBUG_LOCK_VALIDATOR)
+#if defined(RT_LOCK_STRICT) && defined(VBOX_WITH_DEBUG_LOCK_VALIDATOR)
     int vrc = RTSemRWRequestReadDebug(m->sem, RT_INDEFINITE_WAIT, (uintptr_t)ASMReturnAddress(), RT_SRC_POS_ARGS);
-#elif defined(RT_STRICT)
+#elif defined(RT_LOCK_STRICT)
     int vrc = RTSemRWRequestReadDebug(m->sem, RT_INDEFINITE_WAIT, (uintptr_t)ASMReturnAddress(), RT_SRC_POS);
 #else
@@ -361 +361 @@
 {
     m = new Data;
-    RTCritSectInit(&m->sem);
+    /** @todo see todo in RWLockHandle::RWLockHandle(). */
+    int vrc = RTCritSectInitEx(&m->sem, 0/*fFlags*/, NIL_RTLOCKVALCLASS, RTLOCKVAL_SUB_CLASS_NONE, NULL);
+    AssertRC(vrc);
 
 #ifdef VBOX_WITH_DEBUG_LOCK_VALIDATOR
@@ -385 +387 @@
 #endif
 
-#if defined(RT_STRICT) && defined(VBOX_WITH_DEBUG_LOCK_VALIDATOR)
+#if defined(RT_LOCK_STRICT) && defined(VBOX_WITH_DEBUG_LOCK_VALIDATOR)
     RTCritSectEnterDebug(&m->sem, (uintptr_t)ASMReturnAddress(), RT_SRC_POS_ARGS);
-#elif defined(RT_STRICT)
+#elif defined(RT_LOCK_STRICT)
     RTCritSectEnterDebug(&m->sem, (uintptr_t)ASMReturnAddress(),
                          "return address >>>", 0, __PRETTY_FUNCTION__);

trunk/src/VBox/Main/webservice/Makefile.kmk

@@ -257 +257 @@
  ifdef VBOX_USE_VCC80
   vboxwebsrv_CXXFLAGS.win += -bigobj
+ endif
+ ifn1of ($(KBUILD_TARGET), win)
+  vboxwebsrv_CXXFLAGS += -Wno-shadow
  endif
  vboxwebsrv_LIBS += \

trunk/src/VBox/Runtime/common/misc/lockvalidator.cpp

@@ -190 +190 @@
     /** Whether this class is in the tree. */
     bool                    fInTree;
+    /** Donate a reference to the next retainer. This is a hack to make
+     *  RTLockValidatorClassCreateUnique work. */
+    bool volatile           fDonateRefToNextRetainer;
+    /** Reserved future use / explicit alignment. */
+    bool                    afReserved[3];
     /** The minimum wait interval for which we do deadlock detection
      *  (milliseconds). */
@@ -196 +201 @@
     RTMSINTERVAL            cMsMinOrder;
     /** More padding. */
-    uint32_t                au32Reserved[ARCH_BITS == 32 ? 6 : 3];
+    uint32_t                au32Reserved[ARCH_BITS == 32 ? 5 : 2];
     /** Classes that may be taken prior to this one.
      * This is a linked list where each node contains a chunk of locks so that we
@@ -274 +279 @@
     {
         if (!RTCritSectIsInitialized(&g_LockValClassTeachCS))
-            RTCritSectInit(&g_LockValClassTeachCS);
+            RTCritSectInitEx(&g_LockValClassTeachCS, RTCRITSECT_FLAGS_NO_LOCK_VAL, NIL_RTLOCKVALCLASS,
+                             RTLOCKVAL_SUB_CLASS_ANY, "RTLockVal-Teach");
 
         if (g_hLockValClassTreeRWLock == NIL_RTSEMRW)
         {
             RTSEMRW hSemRW;
-            int rc = RTSemRWCreate(&hSemRW);
+            int rc = RTSemRWCreateEx(&hSemRW, RTSEMRW_FLAGS_NO_LOCK_VAL, NIL_RTLOCKVALCLASS, RTLOCKVAL_SUB_CLASS_ANY, "RTLockVal-Tree");
             if (RT_SUCCESS(rc))
                 ASMAtomicWriteHandle(&g_hLockValClassTreeRWLock, hSemRW);
@@ -990 +996 @@
     pThis->fStrictReleaseOrder  = fStrictReleaseOrder;
     pThis->fInTree              = false;
+    pThis->fDonateRefToNextRetainer = false;
+    pThis->afReserved[0]        = false;
+    pThis->afReserved[1]        = false;
+    pThis->afReserved[2]        = false;
     pThis->cMsMinDeadlock       = cMsMinDeadlock;
     pThis->cMsMinOrder          = cMsMinOrder;
@@ -1039 +1049 @@
 
 /**
+ * Creates a new lock validator class with a reference that is consumed by the
+ * first call to RTLockValidatorClassRetain.
+ *
+ * This is tailored for use in the parameter list of a semaphore constructor.
+ *
+ * @returns Class handle with a reference that is automatically consumed by the
+ *          first retainer.  NIL_RTLOCKVALCLASS if we run into trouble.
+ *
+ * @param   pszFile             The source position of the call, file.
+ * @param   iLine               The source position of the call, line.
+ * @param   pszFunction         The source position of the call, function.
+ * @param   pszNameFmt          Class name format string, optional (NULL).  Max
+ *                              length is 32 bytes.
+ * @param   ...                 Format string arguments.
+ */
+RTDECL(RTLOCKVALCLASS) RTLockValidatorClassCreateUnique(RT_SRC_POS_DECL, const char *pszNameFmt, ...)
+{
+    RTLOCKVALSRCPOS SrcPos = RTLOCKVALSRCPOS_INIT_POS_NO_ID();
+    RTLOCKVALCLASSINT *pClass;
+    va_list va;
+    va_start(va, pszNameFmt);
+    int rc = RTLockValidatorClassCreateExV(&pClass, &SrcPos,
+                                           true /*fAutodidact*/, true /*fRecursionOk*/, false /*fStrictReleaseOrder*/,
+                                           1 /*cMsMinDeadlock*/, 1 /*cMsMinOrder*/,
+                                           pszNameFmt, va);
+    va_end(va);
+    if (RT_FAILURE(rc))
+        return NIL_RTLOCKVALCLASS;
+    ASMAtomicWriteBool(&pClass->fDonateRefToNextRetainer, true); /* see rtLockValidatorClassRetain */
+    return pClass;
+}
+
+
+/**
  * Internal class retainer.
  * @returns The new reference count.
@@ -1048 +1092 @@
     if (cRefs > RTLOCKVALCLASS_MAX_REFS)
         ASMAtomicWriteU32(&pClass->cRefs, RTLOCKVALCLASS_MAX_REFS);
+    else if (   cRefs == 2
+             && ASMAtomicXchgBool(&pClass->fDonateRefToNextRetainer, false))
+        cRefs = ASMAtomicDecU32(&pClass->cRefs);
     return cRefs;
 }
@@ -2776 +2823 @@
 static int rtLockValidatorDeadlockDetection(PRTLOCKVALRECUNION pRec, PRTTHREADINT pThreadSelf, PCRTLOCKVALSRCPOS pSrcPos)
 {
-#ifdef DEBUG_bird
     RTLOCKVALDDSTACK Stack;
     int rc = rtLockValidatorDdDoDetection(&Stack, pRec, pThreadSelf);
@@ -2799 +2845 @@
     rcLockValidatorDoDeadlockComplaining(&Stack, pRec, pThreadSelf, pSrcPos, rc);
     return rc;
-#else
-    return VINF_SUCCESS;
-#endif
 }
 

trunk/src/VBox/Runtime/generic/semrw-generic.cpp

@@ -115 +115 @@
          * Create the semaphores.
          */
-        rc = RTSemEventCreate(&pThis->WriteEvent);
+        rc = RTSemEventCreateEx(&pThis->WriteEvent, RTSEMEVENT_FLAGS_NO_LOCK_VAL, NIL_RTLOCKVALCLASS, NULL);
         if (RT_SUCCESS(rc))
         {
-            rc = RTSemEventMultiCreate(&pThis->ReadEvent);
+            rc = RTSemEventMultiCreateEx(&pThis->ReadEvent, RTSEMEVENT_FLAGS_NO_LOCK_VAL, NIL_RTLOCKVALCLASS, NULL);
             if (RT_SUCCESS(rc))
             {

trunk/src/VBox/Runtime/include/internal/strict.h

@@ -35 +35 @@
  * @{ */
 
-#ifdef DEBUG_bird /** @todo reenable this for everyone. Just being a little be cautious right now... */
 /** @def RTCRITSECT_STRICT
  * Enables strictness checks and lock accounting of the RTCritSect API.
  */
-#if defined(DOXYGEN_RUNNING) || (!defined(RTCRITSECT_STRICT) && defined(IN_RING3) && (defined(RT_STRICT) || defined(RT_LOCK_STRICT)))
+#if (!defined(RTCRITSECT_STRICT)      && defined(IN_RING3) && defined(RT_LOCK_STRICT)) || defined(DOXYGEN_RUNNING)
 # define RTCRITSECT_STRICT
 #endif
@@ -46 +45 @@
  * Enables strictness checks and lock accounting of the RTSemEvent API.
  */
-#if defined(DOXYGEN_RUNNING) || (!defined(RTSEMEVENT_STRICT) && defined(IN_RING3) && (defined(RT_STRICT) || defined(RT_LOCK_STRICT) || defined(RTSEM_STRICT)))
+#if (!defined(RTSEMEVENT_STRICT)      && defined(IN_RING3) && defined(RT_LOCK_STRICT)) || defined(DOXYGEN_RUNNING)
 # define RTSEMEVENT_STRICT
 #endif
@@ -53 +52 @@
  * Enables strictness checks and lock accounting of the RTSemEventMulti API.
  */
-#if defined(DOXYGEN_RUNNING) || (!defined(RTSEMEVENTMULTI_STRICT) && defined(IN_RING3) && (defined(RT_STRICT) || defined(RT_LOCK_STRICT) || defined(RTSEM_STRICT)))
+#if (!defined(RTSEMEVENTMULTI_STRICT) && defined(IN_RING3) && defined(RT_LOCK_STRICT)) || defined(DOXYGEN_RUNNING)
 # define RTSEMEVENTMULTI_STRICT
 #endif
@@ -60 +59 @@
  * Enables strictness checks and lock accounting of the RTSemMutex API.
  */
-#if defined(DOXYGEN_RUNNING) || (!defined(RTSEMMUTEX_STRICT) && defined(IN_RING3) && (defined(RT_STRICT) || defined(RT_LOCK_STRICT) || defined(RTSEM_STRICT)))
+#if (!defined(RTSEMMUTEX_STRICT)      && defined(IN_RING3) && defined(RT_LOCK_STRICT)) || defined(DOXYGEN_RUNNING)
 # define RTSEMMUTEX_STRICT
 #endif
@@ -68 +67 @@
  * Enables strictness checks and lock accounting of the RTSemRW API.
  */
-#if defined(DOXYGEN_RUNNING) || (!defined(RTSEMRW_STRICT) && defined(IN_RING3) && (defined(RT_STRICT) || defined(RT_LOCK_STRICT) || defined(RTSEM_STRICT)))
+#if (!defined(RTSEMRW_STRICT)         && defined(IN_RING3) && defined(RT_LOCK_STRICT)) || defined(DOXYGEN_RUNNING)
 # define RTSEMRW_STRICT
 #endif
-#endif /* DEBUG_bird */
 
 

trunk/src/VBox/Runtime/r3/xml.cpp

@@ -57 +57 @@
  *
  * The constructor and destructor of this structure are used to perform global
- * module initiaizaton and cleanup. Thee must be only one global variable of
+ * module initiaizaton and cleanup. There must be only one global variable of
  * this structure.
  */

trunk/src/VBox/Runtime/testcase/tstRTLockValidator.cpp

@@ -1200 +1200 @@
     RTTEST_CHECK_RC_OK_RET(g_hTest, RTSemRWDestroy(hSemRW), false);
 
-#if 0 /** @todo detect it on RTSemMutex... */
+#if 0 /** @todo detect it on RTSemMutex... wrong locking order? */
     RTSEMMUTEX hSemMtx;
     RTTEST_CHECK_RC_OK_RET(g_hTest, RTSemMutexCreate(&hSemRW), false);
@@ -1253 +1253 @@
     RTLockValidatorSetQuiet(false);
 
-    bool fTestDd = false;//true;
+    bool fTestDd = true;
     bool fTestLo = true;
 

trunk/src/VBox/VMM/PDMCritSect.cpp

@@ -139 +139 @@
 #else
             rc = RTLockValidatorRecExclCreate(&pCritSect->Core.pValidatorRec,
+# ifdef RT_LOCK_STRICT_ORDER
                                               RTLockValidatorClassForSrcPos(RT_SRC_POS_ARGS, "%s", pszName),
+# else
+                                              NIL_RTLOCKVALCLASS,
+# endif
                                               RTLOCKVAL_SUB_CLASS_NONE,
                                               pCritSect, true, "%s", pszName);

trunk/src/VBox/VMM/PDMDevHlp.cpp

@@ -51 +51 @@
  */
 #if /*defined(DEBUG_bird) ||*/ defined(DOXYGEN_RUNNING)
-# define PDM_DEVHLP_DEADLOCK_DETECTION
+# define PDM_DEVHLP_DEADLOCK_DETECTION /**< @todo enable DevHlp deadlock detection! */
 #endif
 

trunk/src/VBox/VMM/PDMInternal.h

@@ -53 +53 @@
 /** @def PDMCRITSECT_STRICT
  * Enables/disables PDM critsect strictness like deadlock detection. */
-#ifdef DEBUG_bird /** @todo temporarily bird only */
-#if (defined(VBOX_STRICT) && defined(IN_RING3)) || defined(DOXYGEN_RUNNING)
+#if (defined(RT_LOCK_STRICT) && defined(IN_RING3)) || defined(DOXYGEN_RUNNING)
 # define PDMCRITSECT_STRICT
-#endif
 #endif