Skip to content

Commit c5e3e74

Browse files
committed
sync: Improve CheckLastCritical()
This commit adds actual lock stack logging if check fails.
1 parent da957cd commit c5e3e74

File tree

2 files changed

+26
-16
lines changed

2 files changed

+26
-16
lines changed

src/sync.cpp

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -228,20 +228,28 @@ template void EnterCritical(const char*, const char*, int, boost::mutex*, bool);
228228

229229
void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line)
230230
{
231-
{
232-
LockData& lockdata = GetLockData();
233-
std::lock_guard<std::mutex> lock(lockdata.dd_mutex);
234-
235-
const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()];
236-
if (!lock_stack.empty()) {
237-
const auto& lastlock = lock_stack.back();
238-
if (lastlock.first == cs) {
239-
lockname = lastlock.second.Name();
240-
return;
241-
}
231+
LockData& lockdata = GetLockData();
232+
std::lock_guard<std::mutex> lock(lockdata.dd_mutex);
233+
234+
const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()];
235+
if (!lock_stack.empty()) {
236+
const auto& lastlock = lock_stack.back();
237+
if (lastlock.first == cs) {
238+
lockname = lastlock.second.Name();
239+
return;
242240
}
243241
}
244-
throw std::system_error(EPERM, std::generic_category(), strprintf("%s:%s %s was not most recent critical section locked", file, line, guardname));
242+
243+
LogPrintf("INCONSISTENT LOCK ORDER DETECTED\n");
244+
LogPrintf("Current lock order (least recent first) is:\n");
245+
for (const LockStackItem& i : lock_stack) {
246+
LogPrintf(" %s\n", i.second.ToString());
247+
}
248+
if (g_debug_lockorder_abort) {
249+
tfm::format(std::cerr, "%s:%s %s was not most recent critical section locked, details in debug log.\n", file, line, guardname);
250+
abort();
251+
}
252+
throw std::logic_error(strprintf("%s was not most recent critical section locked", guardname));
245253
}
246254

247255
void LeaveCritical()

src/test/reverselock_tests.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,14 @@ BOOST_AUTO_TEST_CASE(reverselock_errors)
4848
WAIT_LOCK(mutex, lock);
4949

5050
#ifdef DEBUG_LOCKORDER
51+
bool prev = g_debug_lockorder_abort;
52+
g_debug_lockorder_abort = false;
53+
5154
// Make sure trying to reverse lock a previous lock fails
52-
try {
53-
REVERSE_LOCK(lock2);
54-
BOOST_CHECK(false); // REVERSE_LOCK(lock2) succeeded
55-
} catch(...) { }
55+
BOOST_CHECK_EXCEPTION(REVERSE_LOCK(lock2), std::logic_error, HasReason("lock2 was not most recent critical section locked"));
5656
BOOST_CHECK(lock2.owns_lock());
57+
58+
g_debug_lockorder_abort = prev;
5759
#endif
5860

5961
// Make sure trying to reverse lock an unlocked lock fails

0 commit comments

Comments
 (0)