foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

net: dedup and RAII-fy the creation of a copy of CConnman::vNodes

Browse source 
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
```
This commit is contained in:
Vasil Dimov 2021-04-26 16:22:07 +02:00
parent b9cf505bdf
commit 75e8bf55f5
No known key found for this signature in database
GPG key ID: 54DF06F64B55CBBF
2 changed files with 61 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

74
src/net.cpp
View file

@ -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;
// 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);
}
if (flagInterruptMsgProc)
return;
}
if (flagInterruptMsgProc)
return;
}
{
LOCK(cs_vNodes);
for (CNode* pnode : vNodesCopy)
pnode->Release();
}
WAIT_LOCK(mutexMsgProc, lock);

37
src/net.h
View file

@ -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;
};