highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-10 22:18:54 +01:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
95975dd08d sync: detect double lock from the same thread (Vasil Dimov)
4df6567e4c 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 95975dd08d
  hebasto:
    re-ACK 95975dd08d

Tree-SHA512: 375c62db7819e348bfaecc3bd82a7907fcd8f5af24f7d637ac82f3f16789da9fc127dbd0e37158a08e0dcbba01a55c6635caf1d8e9e827cf5a3747f7690a498e
This commit is contained in:
Wladimir J. van der Laan
2020-11-25 17:01:57 +01:00
parent 19b8071eae 95975dd08d
commit 50091592dd
3 changed files with 112 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
src/sync.cpp
View File

@@ -13,10 +13,14 @@
#include <util/strencodings.h>
#include <util/threadnames.h>
#include <boost/thread/mutex.hpp>
#include <map>
#include <mutex>
#include <set>
#include <system_error>
#include <thread>
#include <type_traits>
#include <unordered_map>
#include <utility>
#include <vector>
@@ -135,16 +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 push_lock(void* c, const CLockLocation& locklocation)
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))
@@ -175,10 +213,16 @@ static void pop_lock()
}
}
void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry)
template <typename MutexType>
void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry)
{
push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry, util::ThreadGetInternalName()));
}
template void EnterCritical(const char*, const char*, int, Mutex*, bool);
template void EnterCritical(const char*, const char*, int, RecursiveMutex*, bool);
template void EnterCritical(const char*, const char*, int, std::mutex*, bool);
template void EnterCritical(const char*, const char*, int, std::recursive_mutex*, bool);
template void EnterCritical(const char*, const char*, int, boost::mutex*, bool);
void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line)
{