Skip to content

Commit

Permalink
Make LOCK, LOCK2, TRY_LOCK work with CWaitableCriticalSection
Browse files Browse the repository at this point in the history
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.

Cherry-picked from: 1382913
  • Loading branch information
ryanofsky authored and xanimo committed Apr 3, 2024
1 parent 68ae283 commit 01b9ee1
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 37 deletions.
61 changes: 33 additions & 28 deletions src/sync.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,17 @@ void static inline DeleteLock(void* cs) {}
#define AssertLockHeld(cs) AssertLockHeldInternal(#cs, __FILE__, __LINE__, &cs)

/**
* Template mixin that adds -Wthread-safety locking
* annotations to a subset of the mutex API.
* Template mixin that adds -Wthread-safety locking annotations and lock order
* checking to a subset of the mutex API.
*/
template <typename PARENT>
class LOCKABLE AnnotatedMixin : public PARENT
{
public:
~AnnotatedMixin() {
DeleteLock((void*)this);
}

void lock() EXCLUSIVE_LOCK_FUNCTION()
{
PARENT::lock();
Expand All @@ -89,19 +93,15 @@ class LOCKABLE AnnotatedMixin : public PARENT
{
return PARENT::try_lock();
}

using UniqueLock = std::unique_lock<PARENT>;
};

/**
* Wrapped mutex: supports recursive locking, but no waiting
* TODO: We should move away from using the recursive lock by default.
*/
class CCriticalSection : public AnnotatedMixin<std::recursive_mutex>
{
public:
~CCriticalSection() {
DeleteLock((void*)this);
}
};
typedef AnnotatedMixin<std::recursive_mutex> CCriticalSection;

typedef CCriticalSection CDynamicCriticalSection;
/** Wrapped mutex: supports waiting but not recursive locking */
Expand All @@ -117,48 +117,47 @@ typedef std::unique_lock<std::mutex> WaitableLock;
void PrintLockContention(const char* pszName, const char* pszFile, int nLine);
#endif

/** Wrapper around std::unique_lock<CCriticalSection> */
class SCOPED_LOCKABLE CCriticalBlock
/** Wrapper around std::unique_lock style lock for Mutex. */
template <typename Mutex, typename Base = typename Mutex::UniqueLock>
class SCOPED_LOCKABLE CCriticalBlock : public Base
{
private:
std::unique_lock<CCriticalSection> lock;

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
if (!lock.try_lock()) {
if (!Base::try_lock()) {
PrintLockContention(pszName, pszFile, nLine);
#endif
lock.lock();
Base::lock();
#ifdef DEBUG_LOCKCONTENTION
}
#endif
}

bool TryEnter(const char* pszName, const char* pszFile, int nLine)
{
EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()), true);
lock.try_lock();
if (!lock.owns_lock())
EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true);
Base::try_lock();
if (!Base::owns_lock())
LeaveCritical();
return lock.owns_lock();
return Base::owns_lock();
}

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)
TryEnter(pszName, pszFile, nLine);
else
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;

lock = std::unique_lock<CCriticalSection>(*pmutexIn, std::defer_lock);
*static_cast<Base*>(this) = Base(*pmutexIn, std::defer_lock);
if (fTry)
TryEnter(pszName, pszFile, nLine);
else
Expand All @@ -167,22 +166,28 @@ class SCOPED_LOCKABLE CCriticalBlock

~CCriticalBlock() UNLOCK_FUNCTION()
{
if (lock.owns_lock())
if (Base::owns_lock())
LeaveCritical();
}

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 PASTE2(x, y) PASTE(x, y)

#define LOCK(cs) CCriticalBlock PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__)
#define LOCK2(cs1, cs2) CCriticalBlock criticalblock1(cs1, #cs1, __FILE__, __LINE__), criticalblock2(cs2, #cs2, __FILE__, __LINE__)
#define TRY_LOCK(cs, name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, true)
#define LOCK(cs) DebugLock<decltype(cs)> PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__)
#define LOCK2(cs1, cs2) \
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) \
{ \
Expand Down
29 changes: 20 additions & 9 deletions src/test/sync_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,10 @@

#include <boost/test/unit_test.hpp>

BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)

BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
namespace {
template <typename MutexType>
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);
}
Expand All @@ -32,6 +26,23 @@ BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
#else
BOOST_CHECK(!error_thrown);
#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
g_debug_lockorder_abort = prev;
Expand Down

0 comments on commit 01b9ee1

Please sign in to comment.