Implemented utility functions for copying/releasing vNodes vector (#1382)

* Implemented utility functions for copying/releasing vNodes vector

* Refactor code to use new utility functions CopyNodeVector/ReleaseNodeVector

src/darksend.cpp

@@ -2379,22 +2379,12 @@ bool CDarksendQueue::CheckSignature(const CPubKey& pubKeyMasternode)
 
 bool CDarksendQueue::Relay()
 {
-    std::vector<CNode*> vNodesCopy;
+    std::vector<CNode*> vNodesCopy = CopyNodeVector();
-    {
-        LOCK(cs_vNodes);
-        vNodesCopy = vNodes;
-        BOOST_FOREACH(CNode* pnode, vNodesCopy)
-            pnode->AddRef();
-    }
     BOOST_FOREACH(CNode* pnode, vNodesCopy)
         if(pnode->nVersion >= MIN_PRIVATESEND_PEER_PROTO_VERSION)
             pnode->PushMessage(NetMsgType::DSQUEUE, (*this));
 
-    {
+    ReleaseNodeVector(vNodesCopy);
-        LOCK(cs_vNodes);
-        BOOST_FOREACH(CNode* pnode, vNodesCopy)
-            pnode->Release();
-    }
     return true;
 }
 

src/masternode-sync.cpp

@@ -17,13 +17,6 @@
 class CMasternodeSync;
 CMasternodeSync masternodeSync;
 
-void ReleaseNodes(const std::vector<CNode*> &vNodesCopy)
-{
-    LOCK(cs_vNodes);
-    BOOST_FOREACH(CNode* pnode, vNodesCopy)
-        pnode->Release();
-}
-
 bool CMasternodeSync::CheckNodeHeight(CNode* pnode, bool fDisconnectStuckNodes)
 {
     CNodeStateStats stats;
@@ -96,13 +89,7 @@ bool CMasternodeSync::IsBlockchainSynced(bool fBlockAccepted)
     if(fCheckpointsEnabled && pCurrentBlockIndex->nHeight < Checkpoints::GetTotalBlocksEstimate(Params().Checkpoints()))
         return false;
 
-    std::vector<CNode*> vNodesCopy;
+    std::vector<CNode*> vNodesCopy = CopyNodeVector();
-    {
-        LOCK(cs_vNodes);
-        vNodesCopy = vNodes;
-        BOOST_FOREACH(CNode* pnode, vNodesCopy)
-            pnode->AddRef();
-    }
 
     // We have enough peers and assume most of them are synced
     if(vNodes.size() >= MASTERNODE_SYNC_ENOUGH_PEERS) {
@@ -117,12 +104,12 @@ bool CMasternodeSync::IsBlockchainSynced(bool fBlockAccepted)
             if(nNodesAtSameHeight >= MASTERNODE_SYNC_ENOUGH_PEERS) {
                 LogPrintf("CMasternodeSync::IsBlockchainSynced -- found enough peers on the same height as we are, done\n");
                 fBlockchainSynced = true;
-                ReleaseNodes(vNodesCopy);
+                ReleaseNodeVector(vNodesCopy);
                 return true;
             }
         }
     }
-    ReleaseNodes(vNodesCopy);
+    ReleaseNodeVector(vNodesCopy);
 
     // wait for at least one new block to be accepted
     if(!fFirstBlockAccepted) return false;
@@ -280,15 +267,9 @@ void CMasternodeSync::ProcessTick()
                 LogPrintf("CMasternodeSync::ProcessTick -- WARNING: not enough data, restarting sync\n");
                 Reset();
             } else {
-                std::vector<CNode*> vNodesCopy;
+                std::vector<CNode*> vNodesCopy = CopyNodeVector();
-                {
-                    LOCK(cs_vNodes);
-                    vNodesCopy = vNodes;
-                    BOOST_FOREACH(CNode* pnode, vNodesCopy)
-                        pnode->AddRef();
-                }
                 governance.RequestGovernanceObjectVotes(vNodesCopy);
-                ReleaseNodes(vNodesCopy);
+                ReleaseNodeVector(vNodesCopy);
                 return;
             }
         }
@@ -324,13 +305,7 @@ void CMasternodeSync::ProcessTick()
         SwitchToNextAsset();
     }
 
-    std::vector<CNode*> vNodesCopy;
+    std::vector<CNode*> vNodesCopy = CopyNodeVector();
-    {
-        LOCK(cs_vNodes);
-        vNodesCopy = vNodes;
-        BOOST_FOREACH(CNode* pnode, vNodesCopy)
-            pnode->AddRef();
-    }
 
     BOOST_FOREACH(CNode* pnode, vNodesCopy)
     {
@@ -355,7 +330,7 @@ void CMasternodeSync::ProcessTick()
                 nRequestedMasternodeAssets = MASTERNODE_SYNC_FINISHED;
             }
             nRequestedMasternodeAttempt++;
-            ReleaseNodes(vNodesCopy);
+            ReleaseNodeVector(vNodesCopy);
             return;
         }
 
@@ -391,11 +366,11 @@ void CMasternodeSync::ProcessTick()
                         LogPrintf("CMasternodeSync::ProcessTick -- ERROR: failed to sync %s\n", GetAssetName());
                         // there is no way we can continue without masternode list, fail here and try later
                         Fail();
-                        ReleaseNodes(vNodesCopy);
+                        ReleaseNodeVector(vNodesCopy);
                         return;
                     }
                     SwitchToNextAsset();
-                    ReleaseNodes(vNodesCopy);
+                    ReleaseNodeVector(vNodesCopy);
                     return;
                 }
 
@@ -408,7 +383,7 @@ void CMasternodeSync::ProcessTick()
 
                 mnodeman.DsegUpdate(pnode);
 
-                ReleaseNodes(vNodesCopy);
+                ReleaseNodeVector(vNodesCopy);
                 return; //this will cause each peer to get one request each six seconds for the various assets we need
             }
 
@@ -425,11 +400,11 @@ void CMasternodeSync::ProcessTick()
                         LogPrintf("CMasternodeSync::ProcessTick -- ERROR: failed to sync %s\n", GetAssetName());
                         // probably not a good idea to proceed without winner list
                         Fail();
-                        ReleaseNodes(vNodesCopy);
+                        ReleaseNodeVector(vNodesCopy);
                         return;
                     }
                     SwitchToNextAsset();
-                    ReleaseNodes(vNodesCopy);
+                    ReleaseNodeVector(vNodesCopy);
                     return;
                 }
 
@@ -439,7 +414,7 @@ void CMasternodeSync::ProcessTick()
                 if(nRequestedMasternodeAttempt > 1 && mnpayments.IsEnoughData()) {
                     LogPrintf("CMasternodeSync::ProcessTick -- nTick %d nRequestedMasternodeAssets %d -- found enough data\n", nTick, nRequestedMasternodeAssets);
                     SwitchToNextAsset();
-                    ReleaseNodes(vNodesCopy);
+                    ReleaseNodeVector(vNodesCopy);
                     return;
                 }
 
@@ -455,7 +430,7 @@ void CMasternodeSync::ProcessTick()
                 // ask node for missing pieces only (old nodes will not be asked)
                 mnpayments.RequestLowDataPaymentBlocks(pnode);
 
-                ReleaseNodes(vNodesCopy);
+                ReleaseNodeVector(vNodesCopy);
                 return; //this will cause each peer to get one request each six seconds for the various assets we need
             }
 
@@ -472,7 +447,7 @@ void CMasternodeSync::ProcessTick()
                         // it's kind of ok to skip this for now, hopefully we'll catch up later?
                     }
                     SwitchToNextAsset();
-                    ReleaseNodes(vNodesCopy);
+                    ReleaseNodeVector(vNodesCopy);
                     return;
                 }
 
@@ -501,7 +476,7 @@ void CMasternodeSync::ProcessTick()
                             // reset nTimeNoObjectsLeft to be able to use the same condition on resync
                             nTimeNoObjectsLeft = 0;
                             SwitchToNextAsset();
-                            ReleaseNodes(vNodesCopy);
+                            ReleaseNodeVector(vNodesCopy);
                             return;
                         }
                         nLastTick = nTick;
@@ -516,13 +491,13 @@ void CMasternodeSync::ProcessTick()
 
                 SendGovernanceSyncRequest(pnode);
 
-                ReleaseNodes(vNodesCopy);
+                ReleaseNodeVector(vNodesCopy);
                 return; //this will cause each peer to get one request each six seconds for the various assets we need
             }
         }
     }
     // looped through all nodes, release them
-    ReleaseNodes(vNodesCopy);
+    ReleaseNodeVector(vNodesCopy);
 }
 
 void CMasternodeSync::SendGovernanceSyncRequest(CNode* pnode)

src/net.cpp

@@ -1211,13 +1211,7 @@ void ThreadSocketHandler()
         //
         // Service each socket
         //
-        vector<CNode*> vNodesCopy;
+        vector<CNode*> vNodesCopy = CopyNodeVector();
-        {
-            LOCK(cs_vNodes);
-            vNodesCopy = vNodes;
-            BOOST_FOREACH(CNode* pnode, vNodesCopy)
-                pnode->AddRef();
-        }
         BOOST_FOREACH(CNode* pnode, vNodesCopy)
         {
             boost::this_thread::interruption_point();
@@ -1306,11 +1300,7 @@ void ThreadSocketHandler()
                 }
             }
         }
-        {
+        ReleaseNodeVector(vNodesCopy);
-            LOCK(cs_vNodes);
-            BOOST_FOREACH(CNode* pnode, vNodesCopy)
-                pnode->Release();
-        }
     }
 }
 
@@ -1785,14 +1775,7 @@ void ThreadMessageHandler()
     SetThreadPriority(THREAD_PRIORITY_BELOW_NORMAL);
     while (true)
     {
-        vector<CNode*> vNodesCopy;
+        vector<CNode*> vNodesCopy = CopyNodeVector();
-        {
-            LOCK(cs_vNodes);
-            vNodesCopy = vNodes;
-            BOOST_FOREACH(CNode* pnode, vNodesCopy) {
-                pnode->AddRef();
-            }
-        }
 
         bool fSleep = true;
 
@@ -1829,11 +1812,7 @@ void ThreadMessageHandler()
             boost::this_thread::interruption_point();
         }
 
-        {
+        ReleaseNodeVector(vNodesCopy);
-            LOCK(cs_vNodes);
-            BOOST_FOREACH(CNode* pnode, vNodesCopy)
-                pnode->Release();
-        }
 
         if (fSleep)
             messageHandlerCondition.timed_wait(lock, boost::posix_time::microsec_clock::universal_time() + boost::posix_time::milliseconds(100));
@@ -2743,3 +2722,23 @@ void DumpBanlist()
 int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds) {
     return nNow + (int64_t)(log1p(GetRand(1ULL << 48) * -0.0000000000000035527136788 /* -1/2^48 */) * average_interval_seconds * -1000000.0 + 0.5);
 }
+
+std::vector<CNode*> CopyNodeVector()
+{
+    std::vector<CNode*> vecNodesCopy;
+    LOCK(cs_vNodes);
+    for(size_t i = 0; i < vNodes.size(); ++i) {
+        CNode* pnode = vNodes[i];
+        pnode->AddRef();
+        vecNodesCopy.push_back(pnode);
+    }
+    return vecNodesCopy;
+}
+
+void ReleaseNodeVector(const std::vector<CNode*>& vecNodes)
+{
+    for(size_t i = 0; i < vecNodes.size(); ++i) {
+        CNode* pnode = vecNodes[i];
+        pnode->Release();
+    }
+}

src/net.h

@@ -862,4 +862,8 @@ void DumpBanlist();
 /** Return a timestamp in the future (in microseconds) for exponentially distributed events. */
 int64_t PoissonNextSend(int64_t nNow, int average_interval_seconds);
 
+std::vector<CNode*> CopyNodeVector();
+
+void ReleaseNodeVector(const std::vector<CNode*>& vecNodes);
+
 #endif // BITCOIN_NET_H