diff options
author | MarcoFalke <falke.marco@gmail.com> | 2021-05-03 08:13:49 +0200 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2021-05-03 08:13:53 +0200 |
commit | 320e518b9062ea5510abd7876d22dc5bd3352535 (patch) | |
tree | 633e04ecec3c0173d1b1d39d3536f6d83380d937 | |
parent | 77d569ccb506f17978f983fe2c96cce0ba77ba40 (diff) | |
parent | 9096b13a4764873511b65f32a005ce4738b0d81c (diff) | |
download | bitcoin-320e518b9062ea5510abd7876d22dc5bd3352535.tar.xz |
Merge bitcoin/bitcoin#21750: net: remove unnecessary check of CNode::cs_vSend
9096b13a4764873511b65f32a005ce4738b0d81c 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 9096b13a4764873511b65f32a005ce4738b0d81c 🏧
jnewbery:
utACK 9096b13a4764873511b65f32a005ce4738b0d81c
Tree-SHA512: 910899cdcdc8934642eb0c40fcece8c3b01b7e20a0b023966b9d6972db6a885cb3a9a04e9562bae14d5833967e45e2ecb3687b94d495060c3da4b1f2afb0ac8f
-rw-r--r-- | src/net.cpp | 15 |
1 files changed, 3 insertions, 12 deletions
diff --git a/src/net.cpp b/src/net.cpp index fe1a0dfded..f55d3e2418 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1224,19 +1224,10 @@ void CConnman::DisconnectNodes() std::list<CNode*> vNodesDisconnectedCopy = vNodesDisconnected; for (CNode* pnode : vNodesDisconnectedCopy) { - // wait until threads are done using it + // Destroy the object only after other threads have stopped using it. if (pnode->GetRefCount() <= 0) { - bool fDelete = false; - { - TRY_LOCK(pnode->cs_vSend, lockSend); - if (lockSend) { - fDelete = true; - } - } - if (fDelete) { - vNodesDisconnected.remove(pnode); - DeleteNode(pnode); - } + vNodesDisconnected.remove(pnode); + DeleteNode(pnode); } } } |