diff --git a/src/logging.cpp b/src/logging.cpp index df6946d6611..bfd48c96211 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -53,7 +53,7 @@ static int FileWriteStr(std::string_view str, FILE *fp) bool BCLog::Logger::StartLogging() { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); assert(m_buffering); assert(m_fileout == nullptr); @@ -97,7 +97,7 @@ bool BCLog::Logger::StartLogging() void BCLog::Logger::DisconnectTestLogger() { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); m_buffering = true; if (m_fileout != nullptr) fclose(m_fileout); m_fileout = nullptr; @@ -111,7 +111,7 @@ void BCLog::Logger::DisconnectTestLogger() void BCLog::Logger::DisableLogging() { { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); assert(m_buffering); assert(m_print_callbacks.empty()); } @@ -159,7 +159,7 @@ bool BCLog::Logger::WillLogCategoryLevel(BCLog::LogFlags category, BCLog::Level if (!WillLogCategory(category)) return false; - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); const auto it{m_category_log_levels.find(category)}; return level >= (it == m_category_log_levels.end() ? LogLevel() : it->second); } @@ -392,7 +392,7 @@ BCLog::LogRateLimiter::Status BCLog::LogRateLimiter::Consume( const SourceLocation& source_loc, const std::string& str) { - StdLockGuard scoped_lock(m_mutex); + STDLOCK(m_mutex); auto& stats{m_source_locations.try_emplace(source_loc, m_max_bytes).first->second}; Status status{stats.m_dropped_bytes > 0 ? Status::STILL_SUPPRESSED : Status::UNSUPPRESSED}; @@ -423,7 +423,7 @@ void BCLog::Logger::FormatLogStrInPlace(std::string& str, BCLog::LogFlags catego void BCLog::Logger::LogPrintStr(std::string_view str, SourceLocation&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit) { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); return LogPrintStr_(str, std::move(source_loc), category, level, should_ratelimit); } @@ -556,7 +556,7 @@ void BCLog::LogRateLimiter::Reset() { decltype(m_source_locations) source_locations; { - StdLockGuard scoped_lock(m_mutex); + STDLOCK(m_mutex); source_locations.swap(m_source_locations); m_suppression_active = false; } @@ -598,7 +598,7 @@ bool BCLog::Logger::SetCategoryLogLevel(std::string_view category_str, std::stri const auto level = GetLogLevel(level_str); if (!level.has_value() || level.value() > MAX_USER_SETABLE_SEVERITY_LEVEL) return false; - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); m_category_log_levels[flag] = level.value(); return true; } diff --git a/src/logging.h b/src/logging.h index 803bdac1c42..005c67fd83c 100644 --- a/src/logging.h +++ b/src/logging.h @@ -192,14 +192,14 @@ namespace BCLog { /** Returns whether logs will be written to any output */ bool Enabled() const EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); return m_buffering || m_print_to_console || m_print_to_file || !m_print_callbacks.empty(); } /** Connect a slot to the print signal and return the connection */ std::list>::iterator PushBackCallback(std::function fun) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); m_print_callbacks.push_back(std::move(fun)); return --m_print_callbacks.end(); } @@ -207,13 +207,13 @@ namespace BCLog { /** Delete a connection */ void DeleteCallback(std::list>::iterator it) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); m_print_callbacks.erase(it); } - size_t NumConnections() + size_t NumConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); return m_print_callbacks.size(); } @@ -224,7 +224,7 @@ namespace BCLog { void SetRateLimiting(std::shared_ptr limiter) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); m_limiter = std::move(limiter); } @@ -240,17 +240,17 @@ namespace BCLog { std::unordered_map CategoryLevels() const EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); return m_category_log_levels; } void SetCategoryLogLevel(const std::unordered_map& levels) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); m_category_log_levels = levels; } - void AddCategoryLogLevel(LogFlags category, Level level) + void AddCategoryLogLevel(LogFlags category, Level level) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { - StdLockGuard scoped_lock(m_cs); + STDLOCK(m_cs); m_category_log_levels[category] = level; } bool SetCategoryLogLevel(std::string_view category_str, std::string_view level_str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs); diff --git a/src/sync.cpp b/src/sync.cpp index 0e5c623d212..6d740866d49 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -87,10 +88,10 @@ using LockOrders = std::map; using InvLockOrders = std::set; struct LockData { - LockStacks m_lock_stacks; - LockOrders lockorders; - InvLockOrders invlockorders; - std::mutex dd_mutex; + LockStacks m_lock_stacks GUARDED_BY(dd_mutex); + LockOrders lockorders GUARDED_BY(dd_mutex); + InvLockOrders invlockorders GUARDED_BY(dd_mutex); + StdMutex dd_mutex; }; LockData& GetLockData() { @@ -166,7 +167,7 @@ static void push_lock(MutexType* c, const CLockLocation& locklocation) std::is_base_of_v; LockData& lockdata = GetLockData(); - std::lock_guard lock(lockdata.dd_mutex); + STDLOCK(lockdata.dd_mutex); LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; lock_stack.emplace_back(c, locklocation); @@ -206,7 +207,7 @@ static void push_lock(MutexType* c, const CLockLocation& locklocation) static void pop_lock() { LockData& lockdata = GetLockData(); - std::lock_guard lock(lockdata.dd_mutex); + STDLOCK(lockdata.dd_mutex); LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; lock_stack.pop_back(); @@ -226,7 +227,7 @@ template void EnterCritical(const char*, const char*, int, std::recursive_mutex* void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) { LockData& lockdata = GetLockData(); - std::lock_guard lock(lockdata.dd_mutex); + STDLOCK(lockdata.dd_mutex); const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; if (!lock_stack.empty()) { @@ -257,7 +258,7 @@ void LeaveCritical() static std::string LocksHeld() { LockData& lockdata = GetLockData(); - std::lock_guard lock(lockdata.dd_mutex); + STDLOCK(lockdata.dd_mutex); const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; std::string result; @@ -269,7 +270,7 @@ static std::string LocksHeld() static bool LockHeld(void* mutex) { LockData& lockdata = GetLockData(); - std::lock_guard lock(lockdata.dd_mutex); + STDLOCK(lockdata.dd_mutex); const LockStack& lock_stack = lockdata.m_lock_stacks[std::this_thread::get_id()]; for (const LockStackItem& i : lock_stack) { @@ -302,7 +303,7 @@ template void AssertLockNotHeldInternal(const char*, const char*, int, Recursive void DeleteLock(void* cs) { LockData& lockdata = GetLockData(); - std::lock_guard lock(lockdata.dd_mutex); + STDLOCK(lockdata.dd_mutex); 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) { @@ -321,7 +322,7 @@ void DeleteLock(void* cs) bool LockStackEmpty() { LockData& lockdata = GetLockData(); - std::lock_guard lock(lockdata.dd_mutex); + STDLOCK(lockdata.dd_mutex); const auto it = lockdata.m_lock_stacks.find(std::this_thread::get_id()); if (it == lockdata.m_lock_stacks.end()) { return true; diff --git a/src/util/stdmutex.h b/src/util/stdmutex.h index 7a3e3039546..2cc05013a59 100644 --- a/src/util/stdmutex.h +++ b/src/util/stdmutex.h @@ -10,6 +10,8 @@ // Thread Safety Analysis and provides appropriate annotation macros. #include // IWYU pragma: export +#include + #include // StdMutex provides an annotated version of std::mutex for us, @@ -23,15 +25,21 @@ public: //! with the ! operator, to indicate that a mutex should not be held. const StdMutex& operator!() const { return *this; } #endif // __clang__ + + // StdMutex::Guard provides an annotated version of std::lock_guard for us. + class SCOPED_LOCKABLE Guard : public std::lock_guard + { + public: + explicit Guard(StdMutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard(cs) {} + ~Guard() UNLOCK_FUNCTION() = default; + }; + + static inline StdMutex& CheckNotHeld(StdMutex& cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) LOCK_RETURNED(cs) { return cs; } }; -// StdLockGuard provides an annotated version of std::lock_guard for us, -// and should only be used when sync.h Mutex/LOCK/etc are not usable. -class SCOPED_LOCKABLE StdLockGuard : public std::lock_guard -{ -public: - explicit StdLockGuard(StdMutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard(cs) {} - ~StdLockGuard() UNLOCK_FUNCTION() = default; -}; +// Provide STDLOCK(..) wrapper around StdMutex::Guard that checks the lock is not already held +#define STDLOCK(cs) StdMutex::Guard UNIQUE_NAME(criticalblock){StdMutex::CheckNotHeld(cs)} + +using StdLockGuard = StdMutex::Guard; // TODO: remove, provided for backwards compat only #endif // BITCOIN_UTIL_STDMUTEX_H