mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-22 23:07:59 +01:00
Code Issues Projects Releases Wiki Activity

sync: make EnterCritical() & push_lock() type safe

Browse source 
The functions `EnterCritical()` and `push_lock()` take a pointer to a
mutex, but that pointer used to be of type `void*` because we use a few
different types for mutexes. This `void*` argument was not type safe
because somebody could have send a pointer to anything that is not a
mutex. Furthermore it wouldn't allow to check whether the passed mutex
is recursive or not.

Thus, change the functions to templated ones so that we can implement
stricter checks for non-recursive mutexes. This also simplifies the
callers of `EnterCritical()`.
This commit is contained in:
Vasil Dimov 2020-06-19 20:51:16 +02:00
parent 0f16212c59
commit 4df6567e4c
No known key found for this signature in database
GPG key ID: 54DF06F64B55CBBF
2 changed files with 20 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
src/sync.cpp
View file

@ -13,7 +13,10 @@
#include <util/strencodings.h> #include <util/strencodings.h>
#include <util/threadnames.h> #include <util/threadnames.h>
#include <boost/thread/mutex.hpp>
#include <map> #include <map>
#include <mutex>
#include <set> #include <set>
#include <system_error> #include <system_error>
#include <thread> #include <thread>
@ -135,7 +138,8 @@ 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)); 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) template <typename MutexType>
static void push_lock(MutexType* c, const CLockLocation& locklocation)
{ {
LockData& lockdata = GetLockData(); LockData& lockdata = GetLockData();
std::lock_guard<std::mutex> lock(lockdata.dd_mutex); std::lock_guard<std::mutex> lock(lockdata.dd_mutex);
@ -175,10 +179,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())); 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) void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line)
{ {

14
src/sync.h
View file

@ -48,7 +48,8 @@ LEAVE_CRITICAL_SECTION(mutex); // no RAII
/////////////////////////////// ///////////////////////////////
#ifdef DEBUG_LOCKORDER #ifdef DEBUG_LOCKORDER
void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false); template <typename MutexType>
void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false);
void LeaveCritical(); void LeaveCritical();
void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line);
std::string LocksHeld(); std::string LocksHeld();
@ -65,7 +66,8 @@ bool LockStackEmpty();
*/ */
extern bool g_debug_lockorder_abort; extern bool g_debug_lockorder_abort;
#else #else
inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} template <typename MutexType>
inline void EnterCritical(const char* pszName, const char* pszFile, int nLine, MutexType* cs, bool fTry = false) {}
inline void LeaveCritical() {} inline void LeaveCritical() {}
inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} inline void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {}
template <typename MutexType> template <typename MutexType>
@ -133,7 +135,7 @@ class SCOPED_LOCKABLE UniqueLock : public Base
private: private:
void Enter(const char* pszName, const char* pszFile, int nLine) void Enter(const char* pszName, const char* pszFile, int nLine)
{ {
EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex())); EnterCritical(pszName, pszFile, nLine, Base::mutex());
#ifdef DEBUG_LOCKCONTENTION #ifdef DEBUG_LOCKCONTENTION
if (!Base::try_lock()) { if (!Base::try_lock()) {
PrintLockContention(pszName, pszFile, nLine); PrintLockContention(pszName, pszFile, nLine);
@ -146,7 +148,7 @@ private:
bool TryEnter(const char* pszName, const char* pszFile, int nLine) bool TryEnter(const char* pszName, const char* pszFile, int nLine)
{ {
EnterCritical(pszName, pszFile, nLine, (void*)(Base::mutex()), true); EnterCritical(pszName, pszFile, nLine, Base::mutex(), true);
Base::try_lock(); Base::try_lock();
if (!Base::owns_lock()) if (!Base::owns_lock())
LeaveCritical(); LeaveCritical();
@ -203,7 +205,7 @@ public:
~reverse_lock() { ~reverse_lock() {
templock.swap(lock); templock.swap(lock);
EnterCritical(lockname.c_str(), file.c_str(), line, (void*)lock.mutex()); EnterCritical(lockname.c_str(), file.c_str(), line, lock.mutex());
lock.lock(); lock.lock();
} }
@ -234,7 +236,7 @@ using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove
#define ENTER_CRITICAL_SECTION(cs) \ #define ENTER_CRITICAL_SECTION(cs) \
{ \ { \
EnterCritical(#cs, __FILE__, __LINE__, (void*)(&cs)); \ EnterCritical(#cs, __FILE__, __LINE__, &cs); \
(cs).lock(); \ (cs).lock(); \
} }