From db6e2a3526a4cd75ebd749df05f328d506801688 Mon Sep 17 00:00:00 2001 From: Tim Flynn Date: Sat, 4 Mar 2017 14:10:43 -0500 Subject: [PATCH 1/2] Implemented utility functions for copying/releasing vNodes vector --- src/net.cpp | 20 ++++++++++++++++++++ src/net.h | 4 ++++ 2 files changed, 24 insertions(+) diff --git a/src/net.cpp b/src/net.cpp index b53787e65f82..16da52a56f48 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2743,3 +2743,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 CopyNodeVector() +{ + std::vector 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& vecNodes) +{ + for(size_t i = 0; i < vecNodes.size(); ++i) { + CNode* pnode = vecNodes[i]; + pnode->Release(); + } +} diff --git a/src/net.h b/src/net.h index 76bf4901b658..c2cf2c6927ee 100644 --- a/src/net.h +++ b/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 CopyNodeVector(); + +void ReleaseNodeVector(const std::vector& vecNodes); + #endif // BITCOIN_NET_H From 61a204ea2903cab1bd52d7f67970ba3d2dfba795 Mon Sep 17 00:00:00 2001 From: Tim Flynn Date: Sun, 5 Mar 2017 13:30:22 -0500 Subject: [PATCH 2/2] Refactor code to use new utility functions CopyNodeVector/ReleaseNodeVector --- src/darksend.cpp | 14 ++-------- src/masternode-sync.cpp | 61 ++++++++++++----------------------------- src/net.cpp | 29 +++----------------- 3 files changed, 24 insertions(+), 80 deletions(-) diff --git a/src/darksend.cpp b/src/darksend.cpp index e335ce5490a9..2b16d3832c58 100644 --- a/src/darksend.cpp +++ b/src/darksend.cpp @@ -2379,22 +2379,12 @@ bool CDarksendQueue::CheckSignature(const CPubKey& pubKeyMasternode) bool CDarksendQueue::Relay() { - std::vector vNodesCopy; - { - LOCK(cs_vNodes); - vNodesCopy = vNodes; - BOOST_FOREACH(CNode* pnode, vNodesCopy) - pnode->AddRef(); - } + std::vector vNodesCopy = CopyNodeVector(); BOOST_FOREACH(CNode* pnode, vNodesCopy) if(pnode->nVersion >= MIN_PRIVATESEND_PEER_PROTO_VERSION) pnode->PushMessage(NetMsgType::DSQUEUE, (*this)); - { - LOCK(cs_vNodes); - BOOST_FOREACH(CNode* pnode, vNodesCopy) - pnode->Release(); - } + ReleaseNodeVector(vNodesCopy); return true; } diff --git a/src/masternode-sync.cpp b/src/masternode-sync.cpp index 35252e9a789c..a4a6a48e89e1 100644 --- a/src/masternode-sync.cpp +++ b/src/masternode-sync.cpp @@ -17,13 +17,6 @@ class CMasternodeSync; CMasternodeSync masternodeSync; -void ReleaseNodes(const std::vector &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 vNodesCopy; - { - LOCK(cs_vNodes); - vNodesCopy = vNodes; - BOOST_FOREACH(CNode* pnode, vNodesCopy) - pnode->AddRef(); - } + std::vector vNodesCopy = CopyNodeVector(); // 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 vNodesCopy; - { - LOCK(cs_vNodes); - vNodesCopy = vNodes; - BOOST_FOREACH(CNode* pnode, vNodesCopy) - pnode->AddRef(); - } + std::vector vNodesCopy = CopyNodeVector(); governance.RequestGovernanceObjectVotes(vNodesCopy); - ReleaseNodes(vNodesCopy); + ReleaseNodeVector(vNodesCopy); return; } } @@ -324,13 +305,7 @@ void CMasternodeSync::ProcessTick() SwitchToNextAsset(); } - std::vector vNodesCopy; - { - LOCK(cs_vNodes); - vNodesCopy = vNodes; - BOOST_FOREACH(CNode* pnode, vNodesCopy) - pnode->AddRef(); - } + std::vector vNodesCopy = CopyNodeVector(); 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) diff --git a/src/net.cpp b/src/net.cpp index 16da52a56f48..e3653ff3c16c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1211,13 +1211,7 @@ void ThreadSocketHandler() // // Service each socket // - vector vNodesCopy; - { - LOCK(cs_vNodes); - vNodesCopy = vNodes; - BOOST_FOREACH(CNode* pnode, vNodesCopy) - pnode->AddRef(); - } + vector vNodesCopy = CopyNodeVector(); BOOST_FOREACH(CNode* pnode, vNodesCopy) { boost::this_thread::interruption_point(); @@ -1306,11 +1300,7 @@ void ThreadSocketHandler() } } } - { - LOCK(cs_vNodes); - BOOST_FOREACH(CNode* pnode, vNodesCopy) - pnode->Release(); - } + ReleaseNodeVector(vNodesCopy); } } @@ -1785,14 +1775,7 @@ void ThreadMessageHandler() SetThreadPriority(THREAD_PRIORITY_BELOW_NORMAL); while (true) { - vector vNodesCopy; - { - LOCK(cs_vNodes); - vNodesCopy = vNodes; - BOOST_FOREACH(CNode* pnode, vNodesCopy) { - pnode->AddRef(); - } - } + vector vNodesCopy = CopyNodeVector(); bool fSleep = true; @@ -1829,11 +1812,7 @@ void ThreadMessageHandler() boost::this_thread::interruption_point(); } - { - LOCK(cs_vNodes); - BOOST_FOREACH(CNode* pnode, vNodesCopy) - pnode->Release(); - } + ReleaseNodeVector(vNodesCopy); if (fSleep) messageHandlerCondition.timed_wait(lock, boost::posix_time::microsec_clock::universal_time() + boost::posix_time::milliseconds(100));