diff --git a/src/logging.cpp b/src/logging.cpp --- a/src/logging.cpp +++ b/src/logging.cpp @@ -22,8 +22,8 @@ * shutdown trying to access the logger. When the shutdown sequence is fully * audited and tested, explicit destruction of these objects can be * implemented by changing this from a raw pointer to a std::unique_ptr. - * Since the destructor is never called, the logger and all its members must - * have a trivial destructor. + * Since the ~Logger() destructor is never called, the Logger class and all + * its subclasses must have implicitly-defined destructors. * * This method of initialization was originally introduced in * ee3374234c60aba2cc4c5cd5cac1c0aefc2d817c. diff --git a/src/sync.cpp b/src/sync.cpp --- a/src/sync.cpp +++ b/src/sync.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -13,6 +14,10 @@ #include #include #include +#include +#include +#include +#include #ifdef DEBUG_LOCKCONTENTION void PrintLockContention(const char *pszName, const char *pszFile, int nLine) { @@ -56,36 +61,37 @@ int sourceLine; }; -typedef std::vector> LockStack; -typedef std::map, LockStack> LockOrders; -typedef std::set> InvLockOrders; +using LockStackItem = std::pair; +using LockStack = std::vector; +using LockStacks = std::unordered_map; -struct LockData { - // Very ugly hack: as the global constructs and destructors run single - // threaded, we use this boolean to know whether LockData still exists, - // as DeleteLock can get called by global RecursiveMutex destructors - // after LockData disappears. - bool available; - LockData() : available(true) {} - ~LockData() { available = false; } +using LockPair = std::pair; +using LockOrders = std::map; +using InvLockOrders = std::set; +struct LockData { + LockStacks m_lock_stacks; LockOrders lockorders; InvLockOrders invlockorders; std::mutex dd_mutex; }; + LockData &GetLockData() { - static LockData lockdata; - return lockdata; + // This approach guarantees that the object is not destroyed until after its + // last use. The operating system automatically reclaims all the memory in a + // program's heap when that program exits. + // Since the ~LockData() destructor is never called, the LockData class and + // all its subclasses must have implicitly-defined destructors. + static LockData &lock_data = *new LockData(); + return lock_data; } -static thread_local LockStack g_lockstack; - -static void -potential_deadlock_detected(const std::pair &mismatch, - const LockStack &s1, const LockStack &s2) { +static void potential_deadlock_detected(const LockPair &mismatch, + const LockStack &s1, + const LockStack &s2) { LogPrintf("POTENTIAL DEADLOCK DETECTED\n"); LogPrintf("Previous lock order was:\n"); - for (const std::pair &i : s2) { + for (const LockStackItem &i : s2) { if (i.first == mismatch.first) { LogPrintfToBeContinued(" (1)"); } @@ -95,7 +101,7 @@ LogPrintf(" %s\n", i.second.ToString()); } LogPrintf("Current lock order is:\n"); - for (const std::pair &i : s1) { + for (const LockStackItem &i : s1) { if (i.first == mismatch.first) { LogPrintfToBeContinued(" (1)"); } @@ -119,20 +125,20 @@ LockData &lockdata = GetLockData(); std::lock_guard lock(lockdata.dd_mutex); - g_lockstack.push_back(std::make_pair(c, locklocation)); - - for (const std::pair &i : g_lockstack) { + 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; } - std::pair p1 = std::make_pair(i.first, c); + const LockPair p1 = std::make_pair(i.first, c); if (lockdata.lockorders.count(p1)) { continue; } - lockdata.lockorders.emplace(p1, g_lockstack); + lockdata.lockorders.emplace(p1, lock_stack); - std::pair p2 = std::make_pair(c, i.first); + const LockPair p2 = std::make_pair(c, i.first); lockdata.invlockorders.insert(p2); if (lockdata.lockorders.count(p2)) { potential_deadlock_detected(p1, lockdata.lockorders[p2], @@ -142,7 +148,14 @@ } static void pop_lock() { - g_lockstack.pop_back(); + LockData &lockdata = GetLockData(); + std::lock_guard lock(lockdata.dd_mutex); + + LockStack &lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; + lock_stack.pop_back(); + if (lock_stack.empty()) { + lockdata.m_lock_stacks.erase(std::this_thread::get_id()); + } } void EnterCritical(const char *pszName, const char *pszFile, int nLine, @@ -153,11 +166,18 @@ void CheckLastCritical(void *cs, std::string &lockname, const char *guardname, const char *file, int line) { - if (!g_lockstack.empty()) { - const auto &lastlock = g_lockstack.back(); - if (lastlock.first == cs) { - lockname = lastlock.second.Name(); - return; + { + LockData &lockdata = GetLockData(); + std::lock_guard lock(lockdata.dd_mutex); + + const LockStack &lock_stack = + lockdata.m_lock_stacks[std::this_thread::get_id()]; + if (!lock_stack.empty()) { + const auto &lastlock = lock_stack.back(); + if (lastlock.first == cs) { + lockname = lastlock.second.Name(); + return; + } } } throw std::system_error( @@ -171,19 +191,37 @@ } std::string LocksHeld() { + LockData &lockdata = GetLockData(); + std::lock_guard lock(lockdata.dd_mutex); + + const LockStack &lock_stack = + lockdata.m_lock_stacks[std::this_thread::get_id()]; std::string result; - for (const std::pair &i : g_lockstack) { + for (const LockStackItem &i : lock_stack) { result += i.second.ToString() + std::string("\n"); } return result; } +static bool LockHeld(void *mutex) { + LockData &lockdata = GetLockData(); + std::lock_guard lock(lockdata.dd_mutex); + + const LockStack &lock_stack = + lockdata.m_lock_stacks[std::this_thread::get_id()]; + for (const LockStackItem &i : lock_stack) { + if (i.first == mutex) { + return true; + } + } + + return false; +} + void AssertLockHeldInternal(const char *pszName, const char *pszFile, int nLine, void *cs) { - for (const std::pair &i : g_lockstack) { - if (i.first == cs) { - return; - } + if (LockHeld(cs)) { + return; } tfm::format(std::cerr, "Assertion failed: lock %s not held in %s:%i; locks held:\n%s", @@ -193,37 +231,29 @@ void AssertLockNotHeldInternal(const char *pszName, const char *pszFile, int nLine, void *cs) { - for (const std::pair &i : g_lockstack) { - if (i.first == cs) { - tfm::format( - std::cerr, + if (!LockHeld(cs)) { + return; + } + tfm::format(std::cerr, "Assertion failed: lock %s held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); - abort(); - } - } + abort(); } void DeleteLock(void *cs) { LockData &lockdata = GetLockData(); - if (!lockdata.available) { - // We're already shutting down. - return; - } - std::lock_guard lock(lockdata.dd_mutex); - std::pair item = std::make_pair(cs, nullptr); + const LockPair item = std::make_pair(cs, nullptr); LockOrders::iterator it = lockdata.lockorders.lower_bound(item); while (it != lockdata.lockorders.end() && it->first.first == cs) { - std::pair invitem = + const LockPair invitem = std::make_pair(it->first.second, it->first.first); lockdata.invlockorders.erase(invitem); lockdata.lockorders.erase(it++); } InvLockOrders::iterator invit = lockdata.invlockorders.lower_bound(item); while (invit != lockdata.invlockorders.end() && invit->first == cs) { - std::pair invinvitem = - std::make_pair(invit->second, invit->first); + const LockPair invinvitem = std::make_pair(invit->second, invit->first); lockdata.lockorders.erase(invinvitem); lockdata.invlockorders.erase(invit++); }