aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorVasil Dimov <vd@FreeBSD.org>2021-04-26 16:22:07 +0200
committerVasil Dimov <vd@FreeBSD.org>2021-11-18 13:29:23 +0100
commit75e8bf55f5a014faada7712a9640dc35e8c86f15 (patch)
treee008c898e12afe42bf673a15a48de49028c0803d /src
parentb9cf505bdfade074f3d2b0473d56d2cbc2711cb3 (diff)
downloadbitcoin-75e8bf55f5a014faada7712a9640dc35e8c86f15.tar.xz
net: dedup and RAII-fy the creation of a copy of CConnman::vNodes
The following pattern was duplicated in CConnman: ```cpp lock create a copy of vNodes, add a reference to each one unlock ... use the copy ... lock release each node from the copy unlock ``` Put that code in a RAII helper that reduces it to: ```cpp create snapshot "snap" ... use the copy ... // release happens when "snap" goes out of scope ```
Diffstat (limited to 'src')
-rw-r--r--src/net.cpp72
-rw-r--r--src/net.h37
2 files changed, 60 insertions, 49 deletions
diff --git a/src/net.cpp b/src/net.cpp
index ad558dd598..165a28c04e 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1513,18 +1513,12 @@ void CConnman::SocketHandler()
}
}
+ const NodesSnapshot snap{*this, /*shuffle=*/false};
+
//
// Service each socket
//
- std::vector<CNode*> vNodesCopy;
- {
- LOCK(cs_vNodes);
- vNodesCopy = vNodes;
- for (CNode* pnode : vNodesCopy)
- pnode->AddRef();
- }
- for (CNode* pnode : vNodesCopy)
- {
+ for (CNode* pnode : snap.Nodes()) {
if (interruptNet)
return;
@@ -1606,11 +1600,6 @@ void CConnman::SocketHandler()
if (InactivityCheck(*pnode)) pnode->fDisconnect = true;
}
- {
- LOCK(cs_vNodes);
- for (CNode* pnode : vNodesCopy)
- pnode->Release();
- }
}
void CConnman::ThreadSocketHandler()
@@ -2224,49 +2213,34 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
void CConnman::ThreadMessageHandler()
{
SetSyscallSandboxPolicy(SyscallSandboxPolicy::MESSAGE_HANDLER);
- FastRandomContext rng;
while (!flagInterruptMsgProc)
{
- std::vector<CNode*> vNodesCopy;
- {
- LOCK(cs_vNodes);
- vNodesCopy = vNodes;
- for (CNode* pnode : vNodesCopy) {
- pnode->AddRef();
- }
- }
-
bool fMoreWork = false;
- // Randomize the order in which we process messages from/to our peers.
- // This prevents attacks in which an attacker exploits having multiple
- // consecutive connections in the vNodes list.
- Shuffle(vNodesCopy.begin(), vNodesCopy.end(), rng);
-
- for (CNode* pnode : vNodesCopy)
{
- if (pnode->fDisconnect)
- continue;
+ // Randomize the order in which we process messages from/to our peers.
+ // This prevents attacks in which an attacker exploits having multiple
+ // consecutive connections in the vNodes list.
+ const NodesSnapshot snap{*this, /*shuffle=*/true};
- // Receive messages
- bool fMoreNodeWork = m_msgproc->ProcessMessages(pnode, flagInterruptMsgProc);
- fMoreWork |= (fMoreNodeWork && !pnode->fPauseSend);
- if (flagInterruptMsgProc)
- return;
- // Send messages
- {
- LOCK(pnode->cs_sendProcessing);
- m_msgproc->SendMessages(pnode);
- }
+ for (CNode* pnode : snap.Nodes()) {
+ if (pnode->fDisconnect)
+ continue;
- if (flagInterruptMsgProc)
- return;
- }
+ // Receive messages
+ bool fMoreNodeWork = m_msgproc->ProcessMessages(pnode, flagInterruptMsgProc);
+ fMoreWork |= (fMoreNodeWork && !pnode->fPauseSend);
+ if (flagInterruptMsgProc)
+ return;
+ // Send messages
+ {
+ LOCK(pnode->cs_sendProcessing);
+ m_msgproc->SendMessages(pnode);
+ }
- {
- LOCK(cs_vNodes);
- for (CNode* pnode : vNodesCopy)
- pnode->Release();
+ if (flagInterruptMsgProc)
+ return;
+ }
}
WAIT_LOCK(mutexMsgProc, lock);
diff --git a/src/net.h b/src/net.h
index 48dfb3043f..7b97b98ae5 100644
--- a/src/net.h
+++ b/src/net.h
@@ -1177,6 +1177,43 @@ private:
*/
std::vector<CService> m_onion_binds;
+ /**
+ * RAII helper to atomically create a copy of `vNodes` and add a reference
+ * to each of the nodes. The nodes are released when this object is destroyed.
+ */
+ class NodesSnapshot
+ {
+ public:
+ explicit NodesSnapshot(const CConnman& connman, bool shuffle)
+ {
+ {
+ LOCK(connman.cs_vNodes);
+ m_nodes_copy = connman.vNodes;
+ for (auto& node : m_nodes_copy) {
+ node->AddRef();
+ }
+ }
+ if (shuffle) {
+ Shuffle(m_nodes_copy.begin(), m_nodes_copy.end(), FastRandomContext{});
+ }
+ }
+
+ ~NodesSnapshot()
+ {
+ for (auto& node : m_nodes_copy) {
+ node->Release();
+ }
+ }
+
+ const std::vector<CNode*>& Nodes() const
+ {
+ return m_nodes_copy;
+ }
+
+ private:
+ std::vector<CNode*> m_nodes_copy;
+ };
+
friend struct CConnmanTest;
friend struct ConnmanTestMsg;
};