diff --git a/configure.ac b/configure.ac index 7f0c5dbd7ac..28015d005de 100644 --- a/configure.ac +++ b/configure.ac @@ -381,7 +381,7 @@ if test "x$enable_werror" = "xyes"; then AX_CHECK_COMPILE_FLAG([-Werror=gnu],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=gnu"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=vla],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=vla"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=switch],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=switch"],,[[$CXXFLAG_WERROR]]) - AX_CHECK_COMPILE_FLAG([-Werror=thread-safety-analysis],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety-analysis"],,[[$CXXFLAG_WERROR]]) + AX_CHECK_COMPILE_FLAG([-Werror=thread-safety],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=unused-variable],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=unused-variable"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=date-time],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=date-time"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=return-type],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=return-type"],,[[$CXXFLAG_WERROR]]) @@ -401,7 +401,7 @@ if test "x$CXXFLAGS_overridden" = "xno"; then AX_CHECK_COMPILE_FLAG([-Wvla],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wswitch],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wswitch"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wformat-security],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat-security"],,[[$CXXFLAG_WERROR]]) - AX_CHECK_COMPILE_FLAG([-Wthread-safety-analysis],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety-analysis"],,[[$CXXFLAG_WERROR]]) + AX_CHECK_COMPILE_FLAG([-Wthread-safety],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wrange-loop-analysis],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wrange-loop-analysis"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wredundant-decls],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wredundant-decls"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wunused-variable],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wunused-variable"],,[[$CXXFLAG_WERROR]]) diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 263d863cfa0..a47709cd82c 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -105,13 +105,12 @@ ReadStatus PartiallyDownloadedBlock::InitData(const CBlockHeaderAndShortTxIDs& c std::vector have_txn(txn_available.size()); { LOCK(pool->cs); - const std::vector >& vTxHashes = pool->vTxHashes; - for (size_t i = 0; i < vTxHashes.size(); i++) { - uint64_t shortid = cmpctblock.GetShortID(vTxHashes[i].first); + for (size_t i = 0; i < pool->vTxHashes.size(); i++) { + uint64_t shortid = cmpctblock.GetShortID(pool->vTxHashes[i].first); std::unordered_map::iterator idit = shorttxids.find(shortid); if (idit != shorttxids.end()) { if (!have_txn[idit->second]) { - txn_available[idit->second] = vTxHashes[i].second->GetSharedTx(); + txn_available[idit->second] = pool->vTxHashes[i].second->GetSharedTx(); have_txn[idit->second] = true; mempool_count++; } else { diff --git a/src/blockencodings.h b/src/blockencodings.h index 9ec1beeaf7a..326db1b4a7c 100644 --- a/src/blockencodings.h +++ b/src/blockencodings.h @@ -126,7 +126,7 @@ class PartiallyDownloadedBlock { protected: std::vector txn_available; size_t prefilled_count = 0, mempool_count = 0, extra_count = 0; - CTxMemPool* pool; + const CTxMemPool* pool; public: CBlockHeader header; explicit PartiallyDownloadedBlock(CTxMemPool* poolIn) : pool(poolIn) {} diff --git a/src/logging.cpp b/src/logging.cpp index 41c6f5c9321..fe58ae9e73e 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -41,7 +41,7 @@ static int FileWriteStr(const std::string &str, FILE *fp) bool BCLog::Logger::StartLogging() { - LockGuard scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); assert(m_buffering); assert(m_fileout == nullptr); @@ -80,7 +80,7 @@ bool BCLog::Logger::StartLogging() void BCLog::Logger::DisconnectTestLogger() { - LockGuard scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); m_buffering = true; if (m_fileout != nullptr) fclose(m_fileout); m_fileout = nullptr; @@ -246,7 +246,7 @@ namespace BCLog { void BCLog::Logger::LogPrintStr(const std::string& str) { - LockGuard scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); std::string str_prefixed = LogEscapeMessage(str); if (m_log_threadnames && m_started_new_line) { diff --git a/src/logging.h b/src/logging.h index c55f5819163..7e646ef67a6 100644 --- a/src/logging.h +++ b/src/logging.h @@ -62,7 +62,7 @@ namespace BCLog { class Logger { private: - mutable std::mutex m_cs; // Can not use Mutex from sync.h because in debug mode it would cause a deadlock when a potential deadlock was detected + mutable StdMutex m_cs; // Can not use Mutex from sync.h because in debug mode it would cause a deadlock when a potential deadlock was detected FILE* m_fileout GUARDED_BY(m_cs) = nullptr; std::list m_msgs_before_open GUARDED_BY(m_cs); @@ -100,14 +100,14 @@ namespace BCLog { /** Returns whether logs will be written to any output */ bool Enabled() const { - LockGuard scoped_lock(m_cs); + StdLockGuard scoped_lock(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) { - LockGuard scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); m_print_callbacks.push_back(std::move(fun)); return --m_print_callbacks.end(); } @@ -115,7 +115,7 @@ namespace BCLog { /** Delete a connection */ void DeleteCallback(std::list>::iterator it) { - LockGuard scoped_lock(m_cs); + StdLockGuard scoped_lock(m_cs); m_print_callbacks.erase(it); } diff --git a/src/sync.cpp b/src/sync.cpp index c3312b5a00e..9abdedbed42 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -219,12 +219,15 @@ static bool LockHeld(void* mutex) return false; } -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) +template +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) { if (LockHeld(cs)) return; tfm::format(std::cerr, "Assertion failed: lock %s not held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld()); abort(); } +template void AssertLockHeldInternal(const char*, const char*, int, Mutex*); +template void AssertLockHeldInternal(const char*, const char*, int, RecursiveMutex*); void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) { diff --git a/src/sync.h b/src/sync.h index 0c6f0ef0a7f..60e5a87aec1 100644 --- a/src/sync.h +++ b/src/sync.h @@ -52,7 +52,8 @@ void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs void LeaveCritical(); void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line); std::string LocksHeld(); -void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs); +template +void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs); void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs); void DeleteLock(void* cs); @@ -66,7 +67,8 @@ extern bool g_debug_lockorder_abort; void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {} void static inline LeaveCritical() {} void static inline CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {} -void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} +template +void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, MutexType* cs) ASSERT_EXCLUSIVE_LOCK(cs) {} void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {} void static inline DeleteLock(void* cs) {} #endif diff --git a/src/threadsafety.h b/src/threadsafety.h index 81f86eac3a6..942aa3fdcdb 100644 --- a/src/threadsafety.h +++ b/src/threadsafety.h @@ -56,13 +56,19 @@ #define ASSERT_EXCLUSIVE_LOCK(...) #endif // __GNUC__ -// LockGuard provides an annotated version of lock_guard for us -// should only be used when sync.h Mutex/LOCK/etc aren't usable -class SCOPED_LOCKABLE LockGuard : public std::lock_guard +// StdMutex provides an annotated version of std::mutex for us, +// and should only be used when sync.h Mutex/LOCK/etc are not usable. +class LOCKABLE StdMutex : public std::mutex +{ +}; + +// 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 LockGuard(std::mutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard(cs) { } - ~LockGuard() UNLOCK_FUNCTION() {}; + explicit StdLockGuard(StdMutex& cs) EXCLUSIVE_LOCK_FUNCTION(cs) : std::lock_guard(cs) {} + ~StdLockGuard() UNLOCK_FUNCTION() {} }; #endif // BITCOIN_THREADSAFETY_H diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 7bf3d169c3b..d5f6d63a46e 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -746,7 +746,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) // the user could have gotten from another RPC command prior to now wallet.BlockUntilSyncedToCurrentChain(); - LOCK2(pwallet->cs_wallet, spk_man.cs_KeyStore); + LOCK2(wallet.cs_wallet, spk_man.cs_KeyStore); EnsureWalletIsUnlocked(&wallet); @@ -769,7 +769,7 @@ UniValue dumpwallet(const JSONRPCRequest& request) std::map mapKeyBirth; const std::map& mapKeyPool = spk_man.GetAllReserveKeys(); - pwallet->GetKeyBirthTimes(mapKeyBirth); + wallet.GetKeyBirthTimes(mapKeyBirth); std::set scripts = spk_man.GetCScripts();