From 41e4324a103f889ce724aaafe9b28612cd48cc0c Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 30 Sep 2025 18:24:02 -0500 Subject: [PATCH 1/2] masternode: replace RecursiveMutex with Mutex in meta store/info and add thread-safety annotations (LOCKS_EXCLUDED); avoid re-entrant locking --- src/masternode/meta.cpp | 14 +++++++------- src/masternode/meta.h | 24 ++++++++++++------------ 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/masternode/meta.cpp b/src/masternode/meta.cpp index adb2aaa30259..783615c1b222 100644 --- a/src/masternode/meta.cpp +++ b/src/masternode/meta.cpp @@ -66,7 +66,7 @@ void CMasternodeMetaInfo::RemoveGovernanceObject(const uint256& nGovernanceObjec mapGovernanceObjectsVotedOn.erase(nGovernanceObjectHash); } -CMasternodeMetaInfoPtr CMasternodeMetaMan::GetMetaInfo(const uint256& proTxHash, bool fCreate) +CMasternodeMetaInfoPtr CMasternodeMetaMan::GetMetaInfo(const uint256& proTxHash, bool fCreate) LOCKS_EXCLUDED(cs) { LOCK(cs); auto it = metaInfos.find(proTxHash); @@ -115,7 +115,7 @@ bool CMasternodeMetaMan::AddGovernanceVote(const uint256& proTxHash, const uint2 return true; } -void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObjectHash) +void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObjectHash) LOCKS_EXCLUDED(cs) { LOCK(cs); for(const auto& p : metaInfos) { @@ -123,20 +123,20 @@ void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObject } } -std::vector CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes() +std::vector CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes() LOCKS_EXCLUDED(cs) { std::vector vecTmp; WITH_LOCK(cs, vecTmp.swap(vecDirtyGovernanceObjectHashes)); return vecTmp; } -bool CMasternodeMetaMan::AlreadyHavePlatformBan(const uint256& inv_hash) const +bool CMasternodeMetaMan::AlreadyHavePlatformBan(const uint256& inv_hash) const LOCKS_EXCLUDED(cs) { LOCK(cs); return m_seen_platform_bans.exists(inv_hash); } -std::optional CMasternodeMetaMan::GetPlatformBan(const uint256& inv_hash) const +std::optional CMasternodeMetaMan::GetPlatformBan(const uint256& inv_hash) const LOCKS_EXCLUDED(cs) { LOCK(cs); PlatformBanMessage ret; @@ -147,13 +147,13 @@ std::optional CMasternodeMetaMan::GetPlatformBan(const uint2 return ret; } -void CMasternodeMetaMan::RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) +void CMasternodeMetaMan::RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) LOCKS_EXCLUDED(cs) { LOCK(cs); m_seen_platform_bans.insert(inv_hash, std::move(msg)); } -std::string MasternodeMetaStore::ToString() const +std::string MasternodeMetaStore::ToString() const LOCKS_EXCLUDED(cs) { LOCK(cs); return strprintf("Masternodes: meta infos object count: %d, nDsqCount: %d", metaInfos.size(), nDsqCount); diff --git a/src/masternode/meta.h b/src/masternode/meta.h index e06a10cceed9..39e4902cf446 100644 --- a/src/masternode/meta.h +++ b/src/masternode/meta.h @@ -35,7 +35,7 @@ class CMasternodeMetaInfo friend class CMasternodeMetaMan; private: - mutable RecursiveMutex cs; + mutable Mutex cs; uint256 proTxHash GUARDED_BY(cs); @@ -127,14 +127,14 @@ class MasternodeMetaStore protected: static const std::string SERIALIZATION_VERSION_STRING; - mutable RecursiveMutex cs; + mutable Mutex cs; std::map metaInfos GUARDED_BY(cs); // keep track of dsq count to prevent masternodes from gaming coinjoin queue std::atomic nDsqCount{0}; public: template - void Serialize(Stream &s) const + void Serialize(Stream &s) const LOCKS_EXCLUDED(cs) { LOCK(cs); std::vector tmpMetaInfo; @@ -145,7 +145,7 @@ class MasternodeMetaStore } template - void Unserialize(Stream &s) + void Unserialize(Stream &s) LOCKS_EXCLUDED(cs) { Clear(); @@ -163,14 +163,14 @@ class MasternodeMetaStore } } - void Clear() + void Clear() LOCKS_EXCLUDED(cs) { LOCK(cs); metaInfos.clear(); } - std::string ToString() const; + std::string ToString() const LOCKS_EXCLUDED(cs); }; /** @@ -233,7 +233,7 @@ class CMasternodeMetaMan : public MasternodeMetaStore bool IsValid() const { return is_valid; } - CMasternodeMetaInfoPtr GetMetaInfo(const uint256& proTxHash, bool fCreate = true); + CMasternodeMetaInfoPtr GetMetaInfo(const uint256& proTxHash, bool fCreate = true) LOCKS_EXCLUDED(cs); int64_t GetDsqCount() const { return nDsqCount; } int64_t GetDsqThreshold(const uint256& proTxHash, int nMnCount); @@ -242,13 +242,13 @@ class CMasternodeMetaMan : public MasternodeMetaStore void DisallowMixing(const uint256& proTxHash); bool AddGovernanceVote(const uint256& proTxHash, const uint256& nGovernanceObjectHash); - void RemoveGovernanceObject(const uint256& nGovernanceObjectHash); + void RemoveGovernanceObject(const uint256& nGovernanceObjectHash) LOCKS_EXCLUDED(cs); - std::vector GetAndClearDirtyGovernanceObjectHashes(); + std::vector GetAndClearDirtyGovernanceObjectHashes() LOCKS_EXCLUDED(cs); - bool AlreadyHavePlatformBan(const uint256& inv_hash) const; - std::optional GetPlatformBan(const uint256& inv_hash) const; - void RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg); + bool AlreadyHavePlatformBan(const uint256& inv_hash) const LOCKS_EXCLUDED(cs); + std::optional GetPlatformBan(const uint256& inv_hash) const LOCKS_EXCLUDED(cs); + void RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) LOCKS_EXCLUDED(cs); }; #endif // BITCOIN_MASTERNODE_META_H From 191c79c813fcfb278aedac10c44314b38598d2f7 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 3 Oct 2025 09:48:53 -0500 Subject: [PATCH 2/2] refactor: update thread-safety annotations to use EXCLUSIVE_LOCKS_REQUIRED for various methods in meta store and info --- src/masternode/meta.cpp | 14 +++++++------- src/masternode/meta.h | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/masternode/meta.cpp b/src/masternode/meta.cpp index 783615c1b222..93dda0796f13 100644 --- a/src/masternode/meta.cpp +++ b/src/masternode/meta.cpp @@ -66,7 +66,7 @@ void CMasternodeMetaInfo::RemoveGovernanceObject(const uint256& nGovernanceObjec mapGovernanceObjectsVotedOn.erase(nGovernanceObjectHash); } -CMasternodeMetaInfoPtr CMasternodeMetaMan::GetMetaInfo(const uint256& proTxHash, bool fCreate) LOCKS_EXCLUDED(cs) +CMasternodeMetaInfoPtr CMasternodeMetaMan::GetMetaInfo(const uint256& proTxHash, bool fCreate) EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); auto it = metaInfos.find(proTxHash); @@ -115,7 +115,7 @@ bool CMasternodeMetaMan::AddGovernanceVote(const uint256& proTxHash, const uint2 return true; } -void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObjectHash) LOCKS_EXCLUDED(cs) +void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObjectHash) EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); for(const auto& p : metaInfos) { @@ -123,20 +123,20 @@ void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObject } } -std::vector CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes() LOCKS_EXCLUDED(cs) +std::vector CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes() EXCLUSIVE_LOCKS_REQUIRED(!cs) { std::vector vecTmp; WITH_LOCK(cs, vecTmp.swap(vecDirtyGovernanceObjectHashes)); return vecTmp; } -bool CMasternodeMetaMan::AlreadyHavePlatformBan(const uint256& inv_hash) const LOCKS_EXCLUDED(cs) +bool CMasternodeMetaMan::AlreadyHavePlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); return m_seen_platform_bans.exists(inv_hash); } -std::optional CMasternodeMetaMan::GetPlatformBan(const uint256& inv_hash) const LOCKS_EXCLUDED(cs) +std::optional CMasternodeMetaMan::GetPlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); PlatformBanMessage ret; @@ -147,13 +147,13 @@ std::optional CMasternodeMetaMan::GetPlatformBan(const uint2 return ret; } -void CMasternodeMetaMan::RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) LOCKS_EXCLUDED(cs) +void CMasternodeMetaMan::RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); m_seen_platform_bans.insert(inv_hash, std::move(msg)); } -std::string MasternodeMetaStore::ToString() const LOCKS_EXCLUDED(cs) +std::string MasternodeMetaStore::ToString() const EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); return strprintf("Masternodes: meta infos object count: %d, nDsqCount: %d", metaInfos.size(), nDsqCount); diff --git a/src/masternode/meta.h b/src/masternode/meta.h index 39e4902cf446..8d8df8d0df92 100644 --- a/src/masternode/meta.h +++ b/src/masternode/meta.h @@ -134,7 +134,7 @@ class MasternodeMetaStore public: template - void Serialize(Stream &s) const LOCKS_EXCLUDED(cs) + void Serialize(Stream &s) const EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); std::vector tmpMetaInfo; @@ -145,7 +145,7 @@ class MasternodeMetaStore } template - void Unserialize(Stream &s) LOCKS_EXCLUDED(cs) + void Unserialize(Stream &s) EXCLUSIVE_LOCKS_REQUIRED(!cs) { Clear(); @@ -163,14 +163,14 @@ class MasternodeMetaStore } } - void Clear() LOCKS_EXCLUDED(cs) + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!cs) { LOCK(cs); metaInfos.clear(); } - std::string ToString() const LOCKS_EXCLUDED(cs); + std::string ToString() const EXCLUSIVE_LOCKS_REQUIRED(!cs); }; /** @@ -233,7 +233,7 @@ class CMasternodeMetaMan : public MasternodeMetaStore bool IsValid() const { return is_valid; } - CMasternodeMetaInfoPtr GetMetaInfo(const uint256& proTxHash, bool fCreate = true) LOCKS_EXCLUDED(cs); + CMasternodeMetaInfoPtr GetMetaInfo(const uint256& proTxHash, bool fCreate = true) EXCLUSIVE_LOCKS_REQUIRED(!cs); int64_t GetDsqCount() const { return nDsqCount; } int64_t GetDsqThreshold(const uint256& proTxHash, int nMnCount); @@ -242,13 +242,13 @@ class CMasternodeMetaMan : public MasternodeMetaStore void DisallowMixing(const uint256& proTxHash); bool AddGovernanceVote(const uint256& proTxHash, const uint256& nGovernanceObjectHash); - void RemoveGovernanceObject(const uint256& nGovernanceObjectHash) LOCKS_EXCLUDED(cs); + void RemoveGovernanceObject(const uint256& nGovernanceObjectHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::vector GetAndClearDirtyGovernanceObjectHashes() LOCKS_EXCLUDED(cs); + std::vector GetAndClearDirtyGovernanceObjectHashes() EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool AlreadyHavePlatformBan(const uint256& inv_hash) const LOCKS_EXCLUDED(cs); - std::optional GetPlatformBan(const uint256& inv_hash) const LOCKS_EXCLUDED(cs); - void RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) LOCKS_EXCLUDED(cs); + bool AlreadyHavePlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); + std::optional GetPlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); + void RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg) EXCLUSIVE_LOCKS_REQUIRED(!cs); }; #endif // BITCOIN_MASTERNODE_META_H