Skip to content

Commit 320e518

Browse files
author
MarcoFalke
committed
Merge bitcoin/bitcoin#21750: net: remove unnecessary check of CNode::cs_vSend
9096b13 net: remove unnecessary check of CNode::cs_vSend (Vasil Dimov) Pull request description: It is not possible to have a node in `CConnman::vNodesDisconnected` and its reference count to be incremented - all `CNode::AddRef()` are done either before the node is added to `CConnman::vNodes` or while holding `CConnman::cs_vNodes` and the object being in `CConnman::vNodes`. So, the object being in `CConnman::vNodesDisconnected` and its reference count being zero means that it is not and will not start to be used by other threads. So, the lock of `CNode::cs_vSend` in `CConnman::DisconnectNodes()` will always succeed and is not necessary. Indeed all locks of `CNode::cs_vSend` are done either when the reference count is >0 or under the protection of `CConnman::cs_vNodes` and the node being in `CConnman::vNodes`. ACKs for top commit: MarcoFalke: review ACK 9096b13 🏧 jnewbery: utACK 9096b13 Tree-SHA512: 910899cdcdc8934642eb0c40fcece8c3b01b7e20a0b023966b9d6972db6a885cb3a9a04e9562bae14d5833967e45e2ecb3687b94d495060c3da4b1f2afb0ac8f
2 parents 77d569c + 9096b13 commit 320e518

File tree

1 file changed

+3
-12
lines changed

1 file changed

+3
-12
lines changed

src/net.cpp

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,19 +1224,10 @@ void CConnman::DisconnectNodes()
12241224
std::list<CNode*> vNodesDisconnectedCopy = vNodesDisconnected;
12251225
for (CNode* pnode : vNodesDisconnectedCopy)
12261226
{
1227-
// wait until threads are done using it
1227+
// Destroy the object only after other threads have stopped using it.
12281228
if (pnode->GetRefCount() <= 0) {
1229-
bool fDelete = false;
1230-
{
1231-
TRY_LOCK(pnode->cs_vSend, lockSend);
1232-
if (lockSend) {
1233-
fDelete = true;
1234-
}
1235-
}
1236-
if (fDelete) {
1237-
vNodesDisconnected.remove(pnode);
1238-
DeleteNode(pnode);
1239-
}
1229+
vNodesDisconnected.remove(pnode);
1230+
DeleteNode(pnode);
12401231
}
12411232
}
12421233
}

0 commit comments

Comments
 (0)