Merge #19337: sync: detect double lock from the same thread

95975dd08d8fdaaeaf28e0d06b861ce2748c17b6 sync: detect double lock from the same thread (Vasil Dimov)
4df6567e4cbb4677e8048de2f8008612e1b860b9 sync: make EnterCritical() & push_lock() type safe (Vasil Dimov)

Pull request description:

  Double lock of the same (non-recursive) mutex from the same thread would produce an undefined behavior. Detect this from `DEBUG_LOCKORDER` and react similarly to the deadlock detection.

  This came up during discussion in another, related PR: https://github.com/bitcoin/bitcoin/pull/19238#discussion_r442394521.

ACKs for top commit:
  laanwj:
    code review ACK 95975dd08d8fdaaeaf28e0d06b861ce2748c17b6
  hebasto:
    re-ACK 95975dd08d8fdaaeaf28e0d06b861ce2748c17b6

Tree-SHA512: 375c62db7819e348bfaecc3bd82a7907fcd8f5af24f7d637ac82f3f16789da9fc127dbd0e37158a08e0dcbba01a55c6635caf1d8e9e827cf5a3747f7690a498e

1 files changed, 55 insertions, 0 deletions

diff --git a/src/test/sync_tests.cpp b/src/test/sync_tests.cpp
index 19029ebd3c..6c14867211 100644
--- a/src/test/sync_tests.cpp
+++ b/src/test/sync_tests.cpp
@@ -6,6 +6,9 @@
 #include <test/util/setup_common.h>
 
 #include <boost/test/unit_test.hpp>
+#include <boost/thread/mutex.hpp>
+
+#include <mutex>
 
 namespace {
 template <typename MutexType>
@@ -29,6 +32,38 @@ void TestPotentialDeadLockDetected(MutexType& mutex1, MutexType& mutex2)
     BOOST_CHECK(!error_thrown);
     #endif
 }
+
+#ifdef DEBUG_LOCKORDER
+template <typename MutexType>
+void TestDoubleLock2(MutexType& m)
+{
+    ENTER_CRITICAL_SECTION(m);
+    LEAVE_CRITICAL_SECTION(m);
+}
+
+template <typename MutexType>
+void TestDoubleLock(bool should_throw)
+{
+    const bool prev = g_debug_lockorder_abort;
+    g_debug_lockorder_abort = false;
+
+    MutexType m;
+    ENTER_CRITICAL_SECTION(m);
+    if (should_throw) {
+        BOOST_CHECK_EXCEPTION(
+            TestDoubleLock2(m), std::logic_error, [](const std::logic_error& e) {
+                return strcmp(e.what(), "double lock detected") == 0;
+            });
+    } else {
+        BOOST_CHECK_NO_THROW(TestDoubleLock2(m));
+    }
+    LEAVE_CRITICAL_SECTION(m);
+
+    BOOST_CHECK(LockStackEmpty());
+
+    g_debug_lockorder_abort = prev;
+}
+#endif /* DEBUG_LOCKORDER */
 } // namespace
 
 BOOST_FIXTURE_TEST_SUITE(sync_tests, BasicTestingSetup)
@@ -55,4 +90,24 @@ BOOST_AUTO_TEST_CASE(potential_deadlock_detected)
     #endif
 }
 
+/* Double lock would produce an undefined behavior. Thus, we only do that if
+ * DEBUG_LOCKORDER is activated to detect it. We don't want non-DEBUG_LOCKORDER
+ * build to produce tests that exhibit known undefined behavior. */
+#ifdef DEBUG_LOCKORDER
+BOOST_AUTO_TEST_CASE(double_lock_mutex)
+{
+    TestDoubleLock<Mutex>(true /* should throw */);
+}
+
+BOOST_AUTO_TEST_CASE(double_lock_boost_mutex)
+{
+    TestDoubleLock<boost::mutex>(true /* should throw */);
+}
+
+BOOST_AUTO_TEST_CASE(double_lock_recursive_mutex)
+{
+    TestDoubleLock<RecursiveMutex>(false /* should not throw */);
+}
+#endif /* DEBUG_LOCKORDER */
+
 BOOST_AUTO_TEST_SUITE_END()