Make LOCK, LOCK2, TRY_LOCK work with CWaitableCriticalSection

They should also work with any other mutex type which std::unique_lock
supports.

There is no change in behavior for current code that calls these macros with
CCriticalSection mutexes.
This commit is contained in:
Russell Yanofsky 2017-11-08 16:21:13 -05:00
parent ba1f095aad
commit 1382913e61
2 changed files with 53 additions and 37 deletions

View file

@ -71,13 +71,17 @@ void static inline DeleteLock(void* cs) {}
#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs) #define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs)
/** /**
* Template mixin that adds -Wthread-safety locking * Template mixin that adds -Wthread-safety locking annotations and lock order
* annotations to a subset of the mutex API. * checking to a subset of the mutex API.
*/ */
template <typename PARENT> template <typename PARENT>
class LOCKABLE AnnotatedMixin : public PARENT class LOCKABLE AnnotatedMixin : public PARENT
{ {
public: public:
~AnnotatedMixin() {
DeleteLock((void*)this);
}
void lock() EXCLUSIVE_LOCK_FUNCTION() void lock() EXCLUSIVE_LOCK_FUNCTION()
{ {
PARENT::lock(); PARENT::lock();
@ -92,19 +96,15 @@ public:
{ {
return PARENT::try_lock(); return PARENT::try_lock();
} }
using UniqueLock = std::unique_lock<PARENT>;
}; };
/** /**
* Wrapped mutex: supports recursive locking, but no waiting * Wrapped mutex: supports recursive locking, but no waiting
* TODO: We should move away from using the recursive lock by default. * TODO: We should move away from using the recursive lock by default.
*/ */
class CCriticalSection : public AnnotatedMixin<std::recursive_mutex> typedef AnnotatedMixin<std::recursive_mutex> CCriticalSection;
{
public:
~CCriticalSection() {
DeleteLock((void*)this);
}
};
/** Wrapped mutex: supports waiting but not recursive locking */ /** Wrapped mutex: supports waiting but not recursive locking */
typedef AnnotatedMixin<std::mutex> CWaitableCriticalSection; typedef AnnotatedMixin<std::mutex> CWaitableCriticalSection;
@ -119,20 +119,19 @@ typedef std::unique_lock<std::mutex> WaitableLock;
void PrintLockContention(const char* pszName, const char* pszFile, int nLine); void PrintLockContention(const char* pszName, const char* pszFile, int nLine);
#endif #endif
/** Wrapper around std::unique_lock<CCriticalSection> */ /** Wrapper around std::unique_lock style lock for Mutex. */
class SCOPED_LOCKABLE CCriticalBlock template <typename Mutex, typename Base = typename Mutex::UniqueLock>
class SCOPED_LOCKABLE CCriticalBlock : public Base
{ {
private: private:
std::unique_lock<CCriticalSection> lock;
void Enter(const char* pszName, const char* pszFile, int nLine) void Enter(const char* pszName, const char* pszFile, int nLine)
{ {
EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex())); EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()));
#ifdef DEBUG_LOCKCONTENTION #ifdef DEBUG_LOCKCONTENTION
if (!lock.try_lock()) { if (!Base::try_lock()) {
PrintLockContention(pszName, pszFile, nLine); PrintLockContention(pszName, pszFile, nLine);
#endif #endif
lock.lock(); Base::lock();
#ifdef DEBUG_LOCKCONTENTION #ifdef DEBUG_LOCKCONTENTION
} }
#endif #endif
@ -140,15 +139,15 @@ private:
bool TryEnter(const char* pszName, const char* pszFile, int nLine) bool TryEnter(const char* pszName, const char* pszFile, int nLine)
{ {
EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()), true); EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true);
lock.try_lock(); Base::try_lock();
if (!lock.owns_lock()) if (!Base::owns_lock())
LeaveCritical(); LeaveCritical();
return lock.owns_lock(); return Base::owns_lock();
} }
public: public:
CCriticalBlock(CCriticalSection& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : lock(mutexIn, std::defer_lock) CCriticalBlock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : Base(mutexIn, std::defer_lock)
{ {
if (fTry) if (fTry)
TryEnter(pszName, pszFile, nLine); TryEnter(pszName, pszFile, nLine);
@ -156,11 +155,11 @@ public:
Enter(pszName, pszFile, nLine); Enter(pszName, pszFile, nLine);
} }
CCriticalBlock(CCriticalSection* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn) CCriticalBlock(Mutex* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn)
{ {
if (!pmutexIn) return; if (!pmutexIn) return;
lock = std::unique_lock<CCriticalSection>(*pmutexIn, std::defer_lock); *static_cast<Base*>(this) = Base(*pmutexIn, std::defer_lock);
if (fTry) if (fTry)
TryEnter(pszName, pszFile, nLine); TryEnter(pszName, pszFile, nLine);
else else
@ -169,22 +168,28 @@ public:
~CCriticalBlock() UNLOCK_FUNCTION() ~CCriticalBlock() UNLOCK_FUNCTION()
{ {
if (lock.owns_lock()) if (Base::owns_lock())
LeaveCritical(); LeaveCritical();
} }
operator bool() operator bool()
{ {
return lock.owns_lock(); return Base::owns_lock();
} }
}; };
template<typename MutexArg>
using DebugLock = CCriticalBlock<typename std::remove_reference<typename std::remove_pointer<MutexArg>::type>::type>;
#define PASTE(x, y) x ## y #define PASTE(x, y) x ## y
#define PASTE2(x, y) PASTE(x, y) #define PASTE2(x, y) PASTE(x, y)
#define LOCK(cs) CCriticalBlock PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__) #define LOCK(cs) DebugLock<decltype(cs)> PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__)
#define LOCK2(cs1, cs2) CCriticalBlock criticalblock1(cs1, #cs1, __FILE__, __LINE__), criticalblock2(cs2, #cs2, __FILE__, __LINE__) #define LOCK2(cs1, cs2) \
#define TRY_LOCK(cs, name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, true) DebugLock<decltype(cs1)> criticalblock1(cs1, #cs1, __FILE__, __LINE__); \
DebugLock<decltype(cs2)> criticalblock2(cs2, #cs2, __FILE__, __LINE__);
#define TRY_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__, true)
#define WAIT_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__)
#define ENTER_CRITICAL_SECTION(cs) \ #define ENTER_CRITICAL_SECTION(cs) \
{ \ { \

View file

@ -7,16 +7,10 @@
#include <boost/test/unit_test.hpp> #include <boost/test/unit_test.hpp>
BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup) namespace {
template <typename MutexType>
BOOST_AUTO_TEST_CASE(potential_deadlock_detected) void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2)
{ {
#ifdef DEBUG_LOCKORDER
bool prev = g_debug_lockorder_abort;
g_debug_lockorder_abort = false;
#endif
CCriticalSection mutex1, mutex2;
{ {
LOCK2(mutex1, mutex2); LOCK2(mutex1, mutex2);
} }
@ -32,6 +26,23 @@ BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
#else #else
BOOST_CHECK(!error_thrown); BOOST_CHECK(!error_thrown);
#endif #endif
}
} // namespace
BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)
BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
{
#ifdef DEBUG_LOCKORDER
bool prev = g_debug_lockorder_abort;
g_debug_lockorder_abort = false;
#endif
CCriticalSection rmutex1, rmutex2;
TestPotentialDeadLockDetected(rmutex1, rmutex2);
CWaitableCriticalSection mutex1, mutex2;
TestPotentialDeadLockDetected(mutex1, mutex2);
#ifdef DEBUG_LOCKORDER #ifdef DEBUG_LOCKORDER
g_debug_lockorder_abort = prev; g_debug_lockorder_abort = prev;