Skip to content

Commit 1382913

Browse files
ryanofskyryanofsky
committed
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.
1 parent ba1f095 commit 1382913

File tree

2 files changed

+53
-37
lines changed

2 files changed

+53
-37
lines changed

‎src/sync.h‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,17 @@ void static inline DeleteLock(void* cs) {}
7171
#define AssertLockNotHeld(cs) AssertLockNotHeldInternal(#cs, __FILE__, __LINE__, &cs)
7272

7373
/**
74-
* Template mixin that adds -Wthread-safety locking
75-
* annotations to a subset of the mutex API.
74+
* Template mixin that adds -Wthread-safety locking annotations and lock order
75+
* checking to a subset of the mutex API.
7676
*/
7777
template <typename PARENT>
7878
class LOCKABLE AnnotatedMixin : public PARENT
7979
{
8080
public:
81+
~AnnotatedMixin() {
82+
DeleteLock((void*)this);
83+
}
84+
8185
void lock() EXCLUSIVE_LOCK_FUNCTION()
8286
{
8387
PARENT::lock();
@@ -92,19 +96,15 @@ class LOCKABLE AnnotatedMixin : public PARENT
9296
{
9397
return PARENT::try_lock();
9498
}
99+
100+
using UniqueLock = std::unique_lock<PARENT>;
95101
};
96102

97103
/**
98104
* Wrapped mutex: supports recursive locking, but no waiting
99105
* TODO: We should move away from using the recursive lock by default.
100106
*/
101-
class CCriticalSection : public AnnotatedMixin<std::recursive_mutex>
102-
{
103-
public:
104-
~CCriticalSection() {
105-
DeleteLock((void*)this);
106-
}
107-
};
107+
typedef AnnotatedMixin<std::recursive_mutex> CCriticalSection;
108108

109109
/** Wrapped mutex: supports waiting but not recursive locking */
110110
typedef AnnotatedMixin<std::mutex> CWaitableCriticalSection;
@@ -119,48 +119,47 @@ typedef std::unique_lock<std::mutex> WaitableLock;
119119
void PrintLockContention(const char* pszName, const char* pszFile, int nLine);
120120
#endif
121121

122-
/** Wrapper around std::unique_lock<CCriticalSection> */
123-
class SCOPED_LOCKABLE CCriticalBlock
122+
/** Wrapper around std::unique_lock style lock for Mutex. */
123+
template <typename Mutex, typename Base = typename Mutex::UniqueLock>
124+
class SCOPED_LOCKABLE CCriticalBlock : public Base
124125
{
125126
private:
126-
std::unique_lock<CCriticalSection> lock;
127-
128127
void Enter(const char* pszName, const char* pszFile, int nLine)
129128
{
130-
EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()));
129+
EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()));
131130
#ifdef DEBUG_LOCKCONTENTION
132-
if (!lock.try_lock()) {
131+
if (!Base::try_lock()) {
133132
PrintLockContention(pszName, pszFile, nLine);
134133
#endif
135-
lock.lock();
134+
Base::lock();
136135
#ifdef DEBUG_LOCKCONTENTION
137136
}
138137
#endif
139138
}
140139

141140
bool TryEnter(const char* pszName, const char* pszFile, int nLine)
142141
{
143-
EnterCritical(pszName, pszFile, nLine, (void*)(lock.mutex()), true);
144-
lock.try_lock();
145-
if (!lock.owns_lock())
142+
EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true);
143+
Base::try_lock();
144+
if (!Base::owns_lock())
146145
LeaveCritical();
147-
return lock.owns_lock();
146+
return Base::owns_lock();
148147
}
149148

150149
public:
151-
CCriticalBlock(CCriticalSection& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : lock(mutexIn, std::defer_lock)
150+
CCriticalBlock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : Base(mutexIn, std::defer_lock)
152151
{
153152
if (fTry)
154153
TryEnter(pszName, pszFile, nLine);
155154
else
156155
Enter(pszName, pszFile, nLine);
157156
}
158157

159-
CCriticalBlock(CCriticalSection* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn)
158+
CCriticalBlock(Mutex* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn)
160159
{
161160
if (!pmutexIn) return;
162161

163-
lock = std::unique_lock<CCriticalSection>(*pmutexIn, std::defer_lock);
162+
*static_cast<Base*>(this) = Base(*pmutexIn, std::defer_lock);
164163
if (fTry)
165164
TryEnter(pszName, pszFile, nLine);
166165
else
@@ -169,22 +168,28 @@ class SCOPED_LOCKABLE CCriticalBlock
169168

170169
~CCriticalBlock() UNLOCK_FUNCTION()
171170
{
172-
if (lock.owns_lock())
171+
if (Base::owns_lock())
173172
LeaveCritical();
174173
}
175174

176175
operator bool()
177176
{
178-
return lock.owns_lock();
177+
return Base::owns_lock();
179178
}
180179
};
181180

181+
template<typename MutexArg>
182+
using DebugLock = CCriticalBlock<typename std::remove_reference<typename std::remove_pointer<MutexArg>::type>::type>;
183+
182184
#define PASTE(x, y) x ## y
183185
#define PASTE2(x, y) PASTE(x, y)
184186

185-
#define LOCK(cs) CCriticalBlock PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__)
186-
#define LOCK2(cs1, cs2) CCriticalBlock criticalblock1(cs1, #cs1, __FILE__, __LINE__), criticalblock2(cs2, #cs2, __FILE__, __LINE__)
187-
#define TRY_LOCK(cs, name) CCriticalBlock name(cs, #cs, __FILE__, __LINE__, true)
187+
#define LOCK(cs) DebugLock<decltype(cs)> PASTE2(criticalblock, __COUNTER__)(cs, #cs, __FILE__, __LINE__)
188+
#define LOCK2(cs1, cs2) \
189+
DebugLock<decltype(cs1)> criticalblock1(cs1, #cs1, __FILE__, __LINE__); \
190+
DebugLock<decltype(cs2)> criticalblock2(cs2, #cs2, __FILE__, __LINE__);
191+
#define TRY_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__, true)
192+
#define WAIT_LOCK(cs, name) DebugLock<decltype(cs)> name(cs, #cs, __FILE__, __LINE__)
188193

189194
#define ENTER_CRITICAL_SECTION(cs) \
190195
{ \

‎src/test/sync_tests.cpp‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,10 @@
77

88
#include <boost/test/unit_test.hpp>
99

10-
BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)
11-
12-
BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
10+
namespace {
11+
template <typename MutexType>
12+
void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2)
1313
{
14-
#ifdef DEBUG_LOCKORDER
15-
bool prev = g_debug_lockorder_abort;
16-
g_debug_lockorder_abort = false;
17-
#endif
18-
19-
CCriticalSection mutex1, mutex2;
2014
{
2115
LOCK2(mutex1, mutex2);
2216
}
@@ -32,6 +26,23 @@ BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
3226
#else
3327
BOOST_CHECK(!error_thrown);
3428
#endif
29+
}
30+
} // namespace
31+
32+
BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)
33+
34+
BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
35+
{
36+
#ifdef DEBUG_LOCKORDER
37+
bool prev = g_debug_lockorder_abort;
38+
g_debug_lockorder_abort = false;
39+
#endif
40+
41+
CCriticalSection rmutex1, rmutex2;
42+
TestPotentialDeadLockDetected(rmutex1, rmutex2);
43+
44+
CWaitableCriticalSection mutex1, mutex2;
45+
TestPotentialDeadLockDetected(mutex1, mutex2);
3546

3647
#ifdef DEBUG_LOCKORDER
3748
g_debug_lockorder_abort = prev;

0 commit comments

Comments
 (0)