Merge bitcoin/bitcoin#21563: net: Restrict period when cs_vNodes mutex is locked

8c8237a4a10feb2ac9ce46f67b5d14bf879b670f net, refactor: Fix style in CConnman::StopNodes (Hennadii Stepanov)
229ac1892d807a1eea5a7c24ae0fe27dc913b1bd net: Combine two loops into one, and update comments (Hennadii Stepanov)
a3d090d1103cd6c25daf07afdf4e65febca6d3f7 net: Restrict period when cs_vNodes mutex is locked (Hennadii Stepanov)

Pull request description:

  This PR restricts the period when the `cs_vNodes` mutex is locked, prevents the only case when `cs_vNodes` could be locked before the `::cs_main`.

  This change makes the explicit locking of recursive mutexes in the explicit order redundant.

ACKs for top commit:
  jnewbery:
    utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f
  vasild:
    ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f
  ajtowns:
    utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f - logic seems sound
  MarcoFalke:
    review ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f 👢

Tree-SHA512: a8277924339622b188b12d260a100adf5d82781634cf974320cf6007341f946a7ff40351137c2f5369aed0d318f38aac2d32965c9b619432440d722a4e78bb73

1 files changed, 1 insertions, 14 deletions

diff --git a/src/init.cpp b/src/init.cpp
index cb056c85eb..bb5b144802 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -194,20 +194,7 @@ void Shutdown(NodeContext& node)
     // Because these depend on each-other, we make sure that neither can be
     // using the other before destroying them.
     if (node.peerman) UnregisterValidationInterface(node.peerman.get());
-    // Follow the lock order requirements:
-    // * CheckForStaleTipAndEvictPeers locks cs_main before indirectly calling GetExtraFullOutboundCount
-    //   which locks cs_vNodes.
-    // * ProcessMessage locks cs_main and g_cs_orphans before indirectly calling ForEachNode which
-    //   locks cs_vNodes.
-    // * CConnman::Stop calls DeleteNode, which calls FinalizeNode, which locks cs_main and calls
-    //   EraseOrphansFor, which locks g_cs_orphans.
-    //
-    // Thus the implicit locking order requirement is: (1) cs_main, (2) g_cs_orphans, (3) cs_vNodes.
-    if (node.connman) {
-        node.connman->StopThreads();
-        LOCK2(::cs_main, ::g_cs_orphans);
-        node.connman->StopNodes();
-    }
+    if (node.connman) node.connman->Stop();
 
     StopTorControl();