Skip to content

Commit cb23fe0

Browse files
committed
[skip ci] sync: Check precondition in LEAVE_CRITICAL_SECTION() macro
This change reveals a bug in the wallet_tests/CreateWalletFromFile test, that will be fixed in the following commit.
1 parent c5e3e74 commit cb23fe0

File tree

2 files changed

+43
-4
lines changed

2 files changed

+43
-4
lines changed

src/sync.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -242,10 +242,12 @@ using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove
242242
(cs).lock(); \
243243
}
244244

245-
#define LEAVE_CRITICAL_SECTION(cs) \
246-
{ \
247-
(cs).unlock(); \
248-
LeaveCritical(); \
245+
#define LEAVE_CRITICAL_SECTION(cs) \
246+
{ \
247+
std::string lockname; \
248+
CheckLastCritical((void*)(&cs), lockname, #cs, __FILE__, __LINE__); \
249+
(cs).unlock(); \
250+
LeaveCritical(); \
249251
}
250252

251253
//! Run code while locking a mutex.

src/test/sync_tests.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,19 @@ void TestDoubleLock(bool should_throw)
6262
g_debug_lockorder_abort = prev;
6363
}
6464
#endif /* DEBUG_LOCKORDER */
65+
66+
template <typename MutexType>
67+
void TestInconsistentLockOrderDetected(MutexType& mutex1, MutexType& mutex2) NO_THREAD_SAFETY_ANALYSIS
68+
{
69+
ENTER_CRITICAL_SECTION(mutex1);
70+
ENTER_CRITICAL_SECTION(mutex2);
71+
#ifdef DEBUG_LOCKORDER
72+
BOOST_CHECK_EXCEPTION(LEAVE_CRITICAL_SECTION(mutex1), std::logic_error, HasReason("mutex1 was not most recent critical section locked"));
73+
#endif // DEBUG_LOCKORDER
74+
LEAVE_CRITICAL_SECTION(mutex2);
75+
LEAVE_CRITICAL_SECTION(mutex1);
76+
BOOST_CHECK(LockStackEmpty());
77+
}
6578
} // namespace
6679

6780
BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)
@@ -108,4 +121,28 @@ BOOST_AUTO_TEST_CASE(double_lock_recursive_mutex)
108121
}
109122
#endif /* DEBUG_LOCKORDER */
110123

124+
BOOST_AUTO_TEST_CASE(inconsistent_lock_order_detected)
125+
{
126+
#ifdef DEBUG_LOCKORDER
127+
bool prev = g_debug_lockorder_abort;
128+
g_debug_lockorder_abort = false;
129+
#endif // DEBUG_LOCKORDER
130+
131+
RecursiveMutex rmutex1, rmutex2;
132+
TestInconsistentLockOrderDetected(rmutex1, rmutex2);
133+
// By checking lock order consistency (CheckLastCritical) before any unlocking (LeaveCritical)
134+
// the lock tracking data must not have been broken by exception.
135+
TestInconsistentLockOrderDetected(rmutex1, rmutex2);
136+
137+
Mutex mutex1, mutex2;
138+
TestInconsistentLockOrderDetected(mutex1, mutex2);
139+
// By checking lock order consistency (CheckLastCritical) before any unlocking (LeaveCritical)
140+
// the lock tracking data must not have been broken by exception.
141+
TestInconsistentLockOrderDetected(mutex1, mutex2);
142+
143+
#ifdef DEBUG_LOCKORDER
144+
g_debug_lockorder_abort = prev;
145+
#endif // DEBUG_LOCKORDER
146+
}
147+
111148
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)