From: Gavin Andresen Date: Tue, 29 Mar 2011 15:32:16 +0000 (-0400) Subject: Compile with DEBUG_LOCKORDER to detect inconsistent lock orderings that can cause... X-Git-Tag: v0.4.0-unstable~227^2~47 X-Git-Url: https://git.novaco.in/?p=novacoin.git;a=commitdiff_plain;h=865ed8a1e5c587468a40756d46bcbc1c5a12bb06 Compile with DEBUG_LOCKORDER to detect inconsistent lock orderings that can cause deadlocks --- diff --git a/src/net.h b/src/net.h index 37fab7f..3b098e1 100644 --- a/src/net.h +++ b/src/net.h @@ -677,7 +677,7 @@ public: void BeginMessage(const char* pszCommand) { - cs_vSend.Enter(); + cs_vSend.Enter("cs_vSend", __FILE__, __LINE__); if (nHeaderStart != -1) AbortMessage(); nHeaderStart = vSend.size(); diff --git a/src/util.cpp b/src/util.cpp index 1784448..24c7ed4 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -909,4 +909,122 @@ string FormatFullVersion() +#ifdef DEBUG_LOCKORDER +// +// Early deadlock detection. +// Problem being solved: +// Thread 1 locks A, then B, then C +// Thread 2 locks D, then C, then A +// --> may result in deadlock between the two threads, depending on when they run. +// Solution implemented here: +// Keep track of pairs of locks: (A before B), (A before C), etc. +// Complain if any thread trys to lock in a different order. +// + +struct CLockLocation +{ + std::string mutexName; + std::string sourceFile; + int sourceLine; + + CLockLocation(const char* pszName, const char* pszFile, int nLine) + { + mutexName = pszName; + sourceFile = pszFile; + sourceLine = nLine; + } +}; + +typedef std::vector< std::pair > LockStack; + +static boost::interprocess::interprocess_mutex dd_mutex; +static std::map, LockStack> lockorders; +static boost::thread_specific_ptr lockstack; + + +static void potential_deadlock_detected(const LockStack& s1, const LockStack& s2) +{ + printf("POTENTIAL DEADLOCK DETECTED\n"); + printf("Previous lock order was:\n"); + BOOST_FOREACH(const PAIRTYPE(CCriticalSection*, CLockLocation)& i, s2) + { + printf(" %s %s:%d\n", i.second.mutexName.c_str(), i.second.sourceFile.c_str(), i.second.sourceLine); + } + printf("Current lock order is:\n"); + BOOST_FOREACH(const PAIRTYPE(CCriticalSection*, CLockLocation)& i, s1) + { + printf(" %s %s:%d\n", i.second.mutexName.c_str(), i.second.sourceFile.c_str(), i.second.sourceLine); + } +} + +static void push_lock(CCriticalSection* c, const CLockLocation& locklocation) +{ + bool fOrderOK = true; + if (lockstack.get() == NULL) + lockstack.reset(new LockStack); + + dd_mutex.lock(); + + (*lockstack).push_back(std::make_pair(c, locklocation)); + + BOOST_FOREACH(const PAIRTYPE(CCriticalSection*, CLockLocation)& i, (*lockstack)) + { + if (i.first == c) break; + + std::pair p1 = std::make_pair(i.first, c); + if (lockorders.count(p1)) + continue; + lockorders[p1] = (*lockstack); + + std::pair p2 = std::make_pair(c, i.first); + if (lockorders.count(p2)) + { + potential_deadlock_detected(lockorders[p2], lockorders[p1]); + break; + } + } + dd_mutex.unlock(); +} + +static void pop_lock() +{ + (*lockstack).pop_back(); +} + +void CCriticalSection::Enter(const char* pszName, const char* pszFile, int nLine) +{ + push_lock(this, CLockLocation(pszName, pszFile, nLine)); + mutex.lock(); +} +void CCriticalSection::Leave() +{ + mutex.unlock(); + pop_lock(); +} +bool CCriticalSection::TryEnter(const char* pszName, const char* pszFile, int nLine) +{ + push_lock(this, CLockLocation(pszName, pszFile, nLine)); + bool result = mutex.try_lock(); + if (!result) pop_lock(); + return result; +} + +#else + +void CCriticalSection::Enter(const char*, const char*, int) +{ + mutex.lock(); +} + +void CCriticalSection::Leave() +{ + mutex.unlock(); +} + +bool CCriticalSection::TryEnter(const char*, const char*, int) +{ + bool result = mutex.try_lock(); + return result; +} +#endif /* DEBUG_LOCKORDER */ diff --git a/src/util.h b/src/util.h index 77f08d7..1e4ceb2 100644 --- a/src/util.h +++ b/src/util.h @@ -223,31 +223,17 @@ std::string FormatFullVersion(); -// Wrapper to automatically initialize critical sections +// Wrapper to automatically initialize mutex class CCriticalSection { -#ifdef __WXMSW__ -protected: - CRITICAL_SECTION cs; -public: - explicit CCriticalSection() { InitializeCriticalSection(&cs); } - ~CCriticalSection() { DeleteCriticalSection(&cs); } - void Enter() { EnterCriticalSection(&cs); } - void Leave() { LeaveCriticalSection(&cs); } - bool TryEnter() { return TryEnterCriticalSection(&cs); } -#else protected: boost::interprocess::interprocess_recursive_mutex mutex; public: explicit CCriticalSection() { } ~CCriticalSection() { } - void Enter() { mutex.lock(); } - void Leave() { mutex.unlock(); } - bool TryEnter() { return mutex.try_lock(); } -#endif -public: - const char* pszFile; - int nLine; + void Enter(const char* pszName, const char* pszFile, int nLine); + void Leave(); + bool TryEnter(const char* pszName, const char* pszFile, int nLine); }; // Automatically leave critical section when leaving block, needed for exception safety @@ -255,9 +241,17 @@ class CCriticalBlock { protected: CCriticalSection* pcs; + public: - CCriticalBlock(CCriticalSection& csIn) { pcs = &csIn; pcs->Enter(); } - ~CCriticalBlock() { pcs->Leave(); } + CCriticalBlock(CCriticalSection& csIn, const char* pszName, const char* pszFile, int nLine) + { + pcs = &csIn; + pcs->Enter(pszName, pszFile, nLine); + } + ~CCriticalBlock() + { + pcs->Leave(); + } }; // WARNING: This will catch continue and break! @@ -265,22 +259,32 @@ public: // I'd rather be careful than suffer the other more error prone syntax. // The compiler will optimise away all this loop junk. #define CRITICAL_BLOCK(cs) \ - for (bool fcriticalblockonce=true; fcriticalblockonce; assert("break caught by CRITICAL_BLOCK!" && !fcriticalblockonce), fcriticalblockonce=false) \ - for (CCriticalBlock criticalblock(cs); fcriticalblockonce && (cs.pszFile=__FILE__, cs.nLine=__LINE__, true); fcriticalblockonce=false, cs.pszFile=NULL, cs.nLine=0) + for (bool fcriticalblockonce=true; fcriticalblockonce; assert(("break caught by CRITICAL_BLOCK!" && !fcriticalblockonce)), fcriticalblockonce=false) \ + for (CCriticalBlock criticalblock(cs, #cs, __FILE__, __LINE__); fcriticalblockonce; fcriticalblockonce=false) class CTryCriticalBlock { protected: CCriticalSection* pcs; + public: - CTryCriticalBlock(CCriticalSection& csIn) { pcs = (csIn.TryEnter() ? &csIn : NULL); } - ~CTryCriticalBlock() { if (pcs) pcs->Leave(); } + CTryCriticalBlock(CCriticalSection& csIn, const char* pszName, const char* pszFile, int nLine) + { + pcs = (csIn.TryEnter(pszName, pszFile, nLine) ? &csIn : NULL); + } + ~CTryCriticalBlock() + { + if (pcs) + { + pcs->Leave(); + } + } bool Entered() { return pcs != NULL; } }; #define TRY_CRITICAL_BLOCK(cs) \ - for (bool fcriticalblockonce=true; fcriticalblockonce; assert("break caught by TRY_CRITICAL_BLOCK!" && !fcriticalblockonce), fcriticalblockonce=false) \ - for (CTryCriticalBlock criticalblock(cs); fcriticalblockonce && (fcriticalblockonce = criticalblock.Entered()) && (cs.pszFile=__FILE__, cs.nLine=__LINE__, true); fcriticalblockonce=false, cs.pszFile=NULL, cs.nLine=0) + for (bool fcriticalblockonce=true; fcriticalblockonce; assert(("break caught by TRY_CRITICAL_BLOCK!" && !fcriticalblockonce)), fcriticalblockonce=false) \ + for (CTryCriticalBlock criticalblock(cs, #cs, __FILE__, __LINE__); fcriticalblockonce && (fcriticalblockonce = criticalblock.Entered()); fcriticalblockonce=false)