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

191c79c refactor: update thread-safety annotations to use EXCLUSIVE_LOCKS_REQUIRED for various methods in meta store and info (pasta)
41e4324 masternode: replace RecursiveMutex with Mutex in meta store/info and add thread-safety annotations (LOCKS_EXCLUDED); avoid re-entrant locking (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  Convert mutex from Recursive to non-recursive

  ## What was done?
    _Describe your changes in detail_

  ## How Has This Been Tested?
    _Please describe in detail how you tested your changes._

    _Include details of your testing environment, and the tests you ran
  to see how your change affects other areas of the code, etc._

  ## Breaking Changes
    _Please describe any breaking changes your code introduces_

  ## Checklist:
    _Go over all the following points, and put an `x` in all the boxes that apply._
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 191c79c
  kwvg:
    utACK 191c79c

Tree-SHA512: 93255cf4c5d99c3dbd7f05f7f55156b9b56bd2e2b4b9618f92cea94c2d95dbc5bbcba7cb4351846b05286642ddf4b4c8340bd803edfeb4cb068df3c7b6e42e59

Author: PastaPastaPasta

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) EXCLUSIVE_LOCKS_REQUIRED(!cs)
 {
     LOCK(cs);
     auto it = metaInfos.find(proTxHash);
@@ -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);
     }
 }
 
-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)
 {
     LOCK(cs);
     PlatformBanMessage ret;
@@ -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);

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<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;
@@ -145,7 +145,7 @@ class MasternodeMetaStore
     }
 
     template<typename Stream>
-    void Unserialize(Stream &s)
+    void Unserialize(Stream &s) EXCLUSIVE_LOCKS_REQUIRED(!cs)
     {
         Clear();
 
@@ -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);
 };
 
 /**
@@ -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);
@@ -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