aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorHennadii Stepanov <32963518+hebasto@users.noreply.github.com>2020-09-20 11:38:03 +0300
committerHennadii Stepanov <32963518+hebasto@users.noreply.github.com>2020-12-10 20:46:39 +0200
commitcb23fe01c125e1820f3c37348e06d98c93e6aec2 (patch)
treec0552f04ef7d8af776d7b5947c03ece0db5707f1 /src
parentc5e3e74f70c29ac8852903ef425f5f327d5da969 (diff)
downloadbitcoin-cb23fe01c125e1820f3c37348e06d98c93e6aec2.tar.xz
[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.
Diffstat (limited to 'src')
-rw-r--r--src/sync.h10
-rw-r--r--src/test/sync_tests.cpp37
2 files changed, 43 insertions, 4 deletions
diff --git a/src/sync.h b/src/sync.h
index 0948083c7f..749bf5575c 100644
--- a/src/sync.h
+++ b/src/sync.h
@@ -242,10 +242,12 @@ using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove
(cs).lock(); \
}
-#define LEAVE_CRITICAL_SECTION(cs) \
- { \
- (cs).unlock(); \
- LeaveCritical(); \
+#define LEAVE_CRITICAL_SECTION(cs) \
+ { \
+ std::string lockname; \
+ CheckLastCritical((void*)(&cs), lockname, #cs, __FILE__, __LINE__); \
+ (cs).unlock(); \
+ LeaveCritical(); \
}
//! Run code while locking a mutex.
diff --git a/src/test/sync_tests.cpp b/src/test/sync_tests.cpp
index 14145ced7e..71275f69d9 100644
--- a/src/test/sync_tests.cpp
+++ b/src/test/sync_tests.cpp
@@ -62,6 +62,19 @@ void TestDoubleLock(bool should_throw)
g_debug_lockorder_abort = prev;
}
#endif /* DEBUG_LOCKORDER */
+
+template <typename MutexType>
+void TestInconsistentLockOrderDetected(MutexType& mutex1, MutexType& mutex2) NO_THREAD_SAFETY_ANALYSIS
+{
+ ENTER_CRITICAL_SECTION(mutex1);
+ ENTER_CRITICAL_SECTION(mutex2);
+#ifdef DEBUG_LOCKORDER
+ BOOST_CHECK_EXCEPTION(LEAVE_CRITICAL_SECTION(mutex1), std::logic_error, HasReason("mutex1 was not most recent critical section locked"));
+#endif // DEBUG_LOCKORDER
+ LEAVE_CRITICAL_SECTION(mutex2);
+ LEAVE_CRITICAL_SECTION(mutex1);
+ BOOST_CHECK(LockStackEmpty());
+}
} // namespace
BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)
@@ -108,4 +121,28 @@ BOOST_AUTO_TEST_CASE(double_lock_recursive_mutex)
}
#endif /* DEBUG_LOCKORDER */
+BOOST_AUTO_TEST_CASE(inconsistent_lock_order_detected)
+{
+#ifdef DEBUG_LOCKORDER
+ bool prev = g_debug_lockorder_abort;
+ g_debug_lockorder_abort = false;
+#endif // DEBUG_LOCKORDER
+
+ RecursiveMutex rmutex1, rmutex2;
+ TestInconsistentLockOrderDetected(rmutex1, rmutex2);
+ // By checking lock order consistency (CheckLastCritical) before any unlocking (LeaveCritical)
+ // the lock tracking data must not have been broken by exception.
+ TestInconsistentLockOrderDetected(rmutex1, rmutex2);
+
+ Mutex mutex1, mutex2;
+ TestInconsistentLockOrderDetected(mutex1, mutex2);
+ // By checking lock order consistency (CheckLastCritical) before any unlocking (LeaveCritical)
+ // the lock tracking data must not have been broken by exception.
+ TestInconsistentLockOrderDetected(mutex1, mutex2);
+
+#ifdef DEBUG_LOCKORDER
+ g_debug_lockorder_abort = prev;
+#endif // DEBUG_LOCKORDER
+}
+
BOOST_AUTO_TEST_SUITE_END()