Skip to content

refactor: replace RecursiveMutex with Mutex in meta store/info and add thread-safety annotations; avoid re-entrant locking #6868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PastaPastaPasta merged 2 commits into from
Oct 8, 2025
Merged

refactor: replace RecursiveMutex with Mutex in meta store/info and add thread-safety annotations; avoid re-entrant locking #6868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
41e4324
masternode: replace RecursiveMutex with Mutex in meta store/info and …
PastaPastaPasta Sep 30, 2025
191c79c
refactor: update thread-safety annotations to use EXCLUSIVE_LOCKS_REQ…
PastaPastaPasta Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/masternode/meta.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
auto it = metaInfos.find(proTxHash);
Expand Down Expand Up @@ -115,28 +115,28 @@ bool CMasternodeMetaMan::AddGovernanceVote(const uint256& proTxHash, const uint2
return true;
}

void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObjectHash)
void CMasternodeMetaMan::RemoveGovernanceObject(const uint256& nGovernanceObjectHash) EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
for(const auto& p : metaInfos) {
p.second->RemoveGovernanceObject(nGovernanceObjectHash);
}
Comment on lines 121 to 123
Copy link

@coderabbitai coderabbitai bot Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Run clang-format on this block.

CI flagged the formatting here (for(const …)); please run clang-format so the spacing matches the project style.

🤖 Prompt for AI Agents 
In src/masternode/meta.cpp around lines 121 to 123, the for-loop formatting
doesn't match project style (CI flagged spacing in "for(const auto& p :
metaInfos)"); run clang-format or reformat this block so spacing/indentation
follow the repository's style guide (e.g., add a space after 'for' and align
braces) and commit the formatted changes.

}

std::vector<uint256> CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes()
std::vector<uint256> CMasternodeMetaMan::GetAndClearDirtyGovernanceObjectHashes() EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
std::vector<uint256> 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 EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
return m_seen_platform_bans.exists(inv_hash);
}

std::optional<PlatformBanMessage> CMasternodeMetaMan::GetPlatformBan(const uint256& inv_hash) const
std::optional<PlatformBanMessage> CMasternodeMetaMan::GetPlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs)
Copy link
Collaborator

@kwvg kwvg Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: duplicate annotation, annotation in header is sufficient

{
LOCK(cs);
PlatformBanMessage ret;
Expand All @@ -147,13 +147,13 @@ std::optional<PlatformBanMessage> CMasternodeMetaMan::GetPlatformBan(const uint2
return ret;
}

void CMasternodeMetaMan::RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg)
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
std::string MasternodeMetaStore::ToString() const EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
return strprintf("Masternodes: meta infos object count: %d, nDsqCount: %d", metaInfos.size(), nDsqCount);
Expand Down
24 changes: 12 additions & 12 deletions src/masternode/meta.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class CMasternodeMetaInfo
friend class CMasternodeMetaMan;

private:
mutable RecursiveMutex cs;
mutable Mutex cs;

uint256 proTxHash GUARDED_BY(cs);

Expand Down Expand Up @@ -127,14 +127,14 @@ class MasternodeMetaStore
protected:
static const std::string SERIALIZATION_VERSION_STRING;

mutable RecursiveMutex cs;
mutable Mutex cs;
std::map<uint256, CMasternodeMetaInfoPtr> metaInfos GUARDED_BY(cs);
// keep track of dsq count to prevent masternodes from gaming coinjoin queue
std::atomic<int64_t> nDsqCount{0};

public:
template<typename Stream>
void Serialize(Stream &s) const
void Serialize(Stream &s) const EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
std::vector<CMasternodeMetaInfo> tmpMetaInfo;
Expand All @@ -145,7 +145,7 @@ class MasternodeMetaStore
}

template<typename Stream>
void Unserialize(Stream &s)
void Unserialize(Stream &s) EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
Clear();

Expand All @@ -163,14 +163,14 @@ class MasternodeMetaStore
}
}

void Clear()
void Clear() EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);

metaInfos.clear();
}

std::string ToString() const;
std::string ToString() const EXCLUSIVE_LOCKS_REQUIRED(!cs);
};

/**
Expand Down Expand Up @@ -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) EXCLUSIVE_LOCKS_REQUIRED(!cs);

int64_t GetDsqCount() const { return nDsqCount; }
int64_t GetDsqThreshold(const uint256& proTxHash, int nMnCount);
Expand All @@ -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) EXCLUSIVE_LOCKS_REQUIRED(!cs);

std::vector<uint256> GetAndClearDirtyGovernanceObjectHashes();
std::vector<uint256> GetAndClearDirtyGovernanceObjectHashes() EXCLUSIVE_LOCKS_REQUIRED(!cs);

bool AlreadyHavePlatformBan(const uint256& inv_hash) const;
std::optional<PlatformBanMessage> GetPlatformBan(const uint256& inv_hash) const;
void RememberPlatformBan(const uint256& inv_hash, PlatformBanMessage&& msg);
bool AlreadyHavePlatformBan(const uint256& inv_hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs);
std::optional<PlatformBanMessage> 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
Loading