diff options
author | Vasil Dimov <vd@FreeBSD.org> | 2020-06-20 15:31:55 +0200 |
---|---|---|
committer | Vasil Dimov <vd@FreeBSD.org> | 2020-08-10 18:43:08 +0200 |
commit | 95975dd08d8fdaaeaf28e0d06b861ce2748c17b6 (patch) | |
tree | c2fb3525064d0a722cad4c27cccdef67f41e4647 /src | |
parent | 4df6567e4cbb4677e8048de2f8008612e1b860b9 (diff) | |
download | bitcoin-95975dd08d8fdaaeaf28e0d06b861ce2748c17b6.tar.xz |
sync: detect double lock from the same thread
Double lock of the same (non-recursive) mutex from the same thread
is producing an undefined behavior. Detect this from DEBUG_LOCKORDER
and react similarly to the deadlock detection.
Diffstat (limited to 'src')
-rw-r--r-- | src/sync.cpp | 40 | ||||
-rw-r--r-- | src/test/sync_tests.cpp | 55 |
2 files changed, 92 insertions, 3 deletions
diff --git a/src/sync.cpp b/src/sync.cpp index 7de8439d6f..d020b4e334 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -20,6 +20,7 @@ #include <set> #include <system_error> #include <thread> +#include <type_traits> #include <unordered_map> #include <utility> #include <vector> @@ -138,17 +139,50 @@ static void potential_deadlock_detected(const LockPair& mismatch, const LockStac throw std::logic_error(strprintf("potential deadlock detected: %s -> %s -> %s", mutex_b, mutex_a, mutex_b)); } +static void double_lock_detected(const void* mutex, LockStack& lock_stack) +{ + LogPrintf("DOUBLE LOCK DETECTED\n"); + LogPrintf("Lock order:\n"); + for (const LockStackItem& i : lock_stack) { + if (i.first == mutex) { + LogPrintf(" (*)"); /* Continued */ + } + LogPrintf(" %s\n", i.second.ToString()); + } + if (g_debug_lockorder_abort) { + tfm::format(std::cerr, "Assertion failed: detected double lock at %s:%i, details in debug log.\n", __FILE__, __LINE__); + abort(); + } + throw std::logic_error("double lock detected"); +} + template <typename MutexType> static void push_lock(MutexType* c, const CLockLocation& locklocation) { + constexpr bool is_recursive_mutex = + std::is_base_of<RecursiveMutex, MutexType>::value || + std::is_base_of<std::recursive_mutex, MutexType>::value; + LockData& lockdata = GetLockData(); std::lock_guard<std::mutex> lock(lockdata.dd_mutex); LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; lock_stack.emplace_back(c, locklocation); - for (const LockStackItem& i : lock_stack) { - if (i.first == c) - break; + for (size_t j = 0; j < lock_stack.size() - 1; ++j) { + const LockStackItem& i = lock_stack[j]; + if (i.first == c) { + if (is_recursive_mutex) { + break; + } + // It is not a recursive mutex and it appears in the stack two times: + // at position `j` and at the end (which we added just before this loop). + // Can't allow locking the same (non-recursive) mutex two times from the + // same thread as that results in an undefined behavior. + auto lock_stack_copy = lock_stack; + lock_stack.pop_back(); + double_lock_detected(c, lock_stack_copy); + // double_lock_detected() does not return. + } const LockPair p1 = std::make_pair(i.first, c); if (lockdata.lockorders.count(p1)) 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() |