masternode-meta.*: add thread annotations, atomic usage, remove unneeded locks (#4466)

* masternode-meta.*: add thread annotations, atomic usage, remove unneeded locks

* masternode-meta.cpp lock cs for metaInfos

* Implement (un)serialization for atomic

* partial revert

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

src/masternode/masternode-meta.cpp

@@ -62,7 +62,6 @@ CMasternodeMetaInfoPtr CMasternodeMetaMan::GetMetaInfo(const uint256& proTxHash,
 // masternodes before we ever see a masternode that we know already mixed someone's funds earlier.
 int64_t CMasternodeMetaMan::GetDsqThreshold(const uint256& proTxHash, int nMnCount)
 {
-    LOCK(cs);
     auto metaInfo = GetMetaInfo(proTxHash);
     if (metaInfo == nullptr) {
         // return a threshold which is slightly above nDsqCount i.e. a no-go
@@ -73,26 +72,20 @@ int64_t CMasternodeMetaMan::GetDsqThreshold(const uint256& proTxHash, int nMnCou
 
 void CMasternodeMetaMan::AllowMixing(const uint256& proTxHash)
 {
-    LOCK(cs);
     auto mm = GetMetaInfo(proTxHash);
     nDsqCount++;
-    LOCK(mm->cs);
-    mm->nLastDsq = nDsqCount;
+    mm->nLastDsq = nDsqCount.load();
     mm->nMixingTxCount = 0;
 }
 
 void CMasternodeMetaMan::DisallowMixing(const uint256& proTxHash)
 {
-    LOCK(cs);
     auto mm = GetMetaInfo(proTxHash);
-
-    LOCK(mm->cs);
     mm->nMixingTxCount++;
 }
 
 bool CMasternodeMetaMan::AddGovernanceVote(const uint256& proTxHash, const uint256& nGovernanceObjectHash)
 {
-    LOCK(cs);
     auto mm = GetMetaInfo(proTxHash);
     mm->AddGovernanceVote(nGovernanceObjectHash);
     return true;
@@ -124,7 +117,7 @@ void CMasternodeMetaMan::Clear()
 std::string CMasternodeMetaMan::ToString() const
 {
     std::ostringstream info;
-
+    LOCK(cs);
     info << "Masternodes: meta infos object count: " << (int)metaInfos.size() <<
          ", nDsqCount: " << (int)nDsqCount;
     return info.str();

src/masternode/masternode-meta.h

@@ -9,6 +9,7 @@
 
 #include <univalue.h>
 
+#include <atomic>
 #include <uint256.h>
 #include <sync.h>
 
@@ -25,28 +26,28 @@ class CMasternodeMetaInfo
 private:
     mutable CCriticalSection cs;
 
-    uint256 proTxHash;
+    uint256 proTxHash GUARDED_BY(cs);
 
     //the dsq count from the last dsq broadcast of this node
-    int64_t nLastDsq = 0;
-    int nMixingTxCount = 0;
+    std::atomic<int64_t> nLastDsq{0};
+    std::atomic<int> nMixingTxCount{0};
 
     // KEEP TRACK OF GOVERNANCE ITEMS EACH MASTERNODE HAS VOTE UPON FOR RECALCULATION
-    std::map<uint256, int> mapGovernanceObjectsVotedOn;
+    std::map<uint256, int> mapGovernanceObjectsVotedOn GUARDED_BY(cs);
 
-    int64_t lastOutboundAttempt = 0;
-    int64_t lastOutboundSuccess = 0;
+    std::atomic<int64_t> lastOutboundAttempt{0};
+    std::atomic<int64_t> lastOutboundSuccess{0};
 
 public:
     CMasternodeMetaInfo() = default;
     explicit CMasternodeMetaInfo(const uint256& _proTxHash) : proTxHash(_proTxHash) {}
     CMasternodeMetaInfo(const CMasternodeMetaInfo& ref) :
         proTxHash(ref.proTxHash),
-        nLastDsq(ref.nLastDsq),
-        nMixingTxCount(ref.nMixingTxCount),
+        nLastDsq(ref.nLastDsq.load()),
+        nMixingTxCount(ref.nMixingTxCount.load()),
         mapGovernanceObjectsVotedOn(ref.mapGovernanceObjectsVotedOn),
-        lastOutboundAttempt(ref.lastOutboundAttempt),
-        lastOutboundSuccess(ref.lastOutboundSuccess)
+        lastOutboundAttempt(ref.lastOutboundAttempt.load()),
+        lastOutboundSuccess(ref.lastOutboundSuccess.load())
     {
     }
 
@@ -67,8 +68,8 @@ public:
 
 public:
     const uint256& GetProTxHash() const { LOCK(cs); return proTxHash; }
-    int64_t GetLastDsq() const { LOCK(cs); return nLastDsq; }
-    int GetMixingTxCount() const { LOCK(cs); return nMixingTxCount; }
+    int64_t GetLastDsq() const { return nLastDsq; }
+    int GetMixingTxCount() const { return nMixingTxCount; }
 
     bool IsValidForMixingTxes() const { return GetMixingTxCount() <= MASTERNODE_MAX_MIXING_TXES; }
 
@@ -77,10 +78,10 @@ public:
 
     void RemoveGovernanceObject(const uint256& nGovernanceObjectHash);
 
-    void SetLastOutboundAttempt(int64_t t) { LOCK(cs); lastOutboundAttempt = t; }
-    int64_t GetLastOutboundAttempt() const { LOCK(cs); return lastOutboundAttempt; }
-    void SetLastOutboundSuccess(int64_t t) { LOCK(cs); lastOutboundSuccess = t; }
-    int64_t GetLastOutboundSuccess() const { LOCK(cs); return lastOutboundSuccess; }
+    void SetLastOutboundAttempt(int64_t t) { lastOutboundAttempt = t; }
+    int64_t GetLastOutboundAttempt() const { return lastOutboundAttempt; }
+    void SetLastOutboundSuccess(int64_t t) { lastOutboundSuccess = t; }
+    int64_t GetLastOutboundSuccess() const { return lastOutboundSuccess; }
 };
 typedef std::shared_ptr<CMasternodeMetaInfo> CMasternodeMetaInfoPtr;
 
@@ -91,11 +92,11 @@ private:
 
     mutable CCriticalSection cs;
 
-    std::map<uint256, CMasternodeMetaInfoPtr> metaInfos;
-    std::vector<uint256> vecDirtyGovernanceObjectHashes;
+    std::map<uint256, CMasternodeMetaInfoPtr> metaInfos GUARDED_BY(cs);
+    std::vector<uint256> vecDirtyGovernanceObjectHashes GUARDED_BY(cs);
 
     // keep track of dsq count to prevent masternodes from gaming coinjoin queue
-    int64_t nDsqCount = 0;
+    std::atomic<int64_t> nDsqCount{0};
 
 public:
     template<typename Stream>
@@ -130,7 +131,7 @@ public:
 public:
     CMasternodeMetaInfoPtr GetMetaInfo(const uint256& proTxHash, bool fCreate = true);
 
-    int64_t GetDsqCount() const { LOCK(cs); return nDsqCount; }
+    int64_t GetDsqCount() const { return nDsqCount; }
     int64_t GetDsqThreshold(const uint256& proTxHash, int nMnCount);
 
     void AllowMixing(const uint256& proTxHash);

src/serialize.h

@@ -10,6 +10,7 @@
 
 #include <algorithm>
 #include <assert.h>
+#include <atomic>
 #include <ios>
 #include <limits>
 #include <list>
@@ -879,6 +880,12 @@ template<typename Stream, typename T> void Unserialize(Stream& os, std::shared_p
 template<typename Stream, typename T> void Serialize(Stream& os, const std::unique_ptr<const T>& p);
 template<typename Stream, typename T> void Unserialize(Stream& os, std::unique_ptr<const T>& p);
 
+/**
+ * atomic
+ */
+template<typename Stream, typename T> void Serialize(Stream& os, const std::atomic<T>& a);
+template<typename Stream, typename T> void Unserialize(Stream& is, std::atomic<T>& a);
+
 
 
 /**
@@ -1311,6 +1318,25 @@ void Unserialize(Stream& is, std::shared_ptr<T>& p)
 
 
 
+/**
+ * atomic
+ */
+template<typename Stream, typename T>
+void Serialize(Stream& os, const std::atomic<T>& a)
+{
+    Serialize(os, a.load());
+}
+
+template<typename Stream, typename T>
+void Unserialize(Stream& is, std::atomic<T>& a)
+{
+    T val;
+    Unserialize(is, val);
+    a.store(val);
+}
+
+
+
 /**
  * Support for SERIALIZE_METHODS and READWRITE macro.
  */