From 9096b13a4764873511b65f32a005ce4738b0d81c Mon Sep 17 00:00:00 2001
From: Vasil Dimov <vd@FreeBSD.org>
Date: Thu, 22 Apr 2021 11:06:13 +0200
Subject: [PATCH] net: remove unnecessary check of CNode::cs_vSend

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`.
---
 src/net.cpp | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index 5aa267f0d7a..f7d102a4df5 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);
             }
         }
     }