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

Summary:
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 is a partial backport of [[bitcoin/bitcoin#21943 | core#21943]]
bitcoin/bitcoin@75e8bf5

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, roqqit

Reviewed By: roqqit

Differential Revision: https://reviews.bitcoinabc.org/D17106

src/net.cpp

@@ -1779,18 +1779,12 @@ void CConnman::SocketHandler() {
         }
     }
 
+    const NodesSnapshot snap{*this, /*shuffle=*/false};
+
     //
     // Service each socket
     //
-    std::vector<CNode *> nodes_copy;
-    {
-        LOCK(m_nodes_mutex);
-        nodes_copy = m_nodes;
-        for (CNode *pnode : nodes_copy) {
-            pnode->AddRef();
-        }
-    }
-    for (CNode *pnode : nodes_copy) {
+    for (CNode *pnode : snap.Nodes()) {
         if (interruptNet) {
             return;
         }
@@ -1884,12 +1878,6 @@ void CConnman::SocketHandler() {
             pnode->fDisconnect = true;
         }
     }
-    {
-        LOCK(m_nodes_mutex);
-        for (CNode *pnode : nodes_copy) {
-            pnode->Release();
-        }
-    }
 }
 
 void CConnman::ThreadSocketHandler() {
@@ -2613,54 +2601,39 @@ Mutex NetEventsInterface::g_msgproc_mutex;
 void CConnman::ThreadMessageHandler() {
     LOCK(NetEventsInterface::g_msgproc_mutex);
 
-    FastRandomContext rng;
     while (!flagInterruptMsgProc) {
-        std::vector<CNode *> nodes_copy;
-        {
-            LOCK(m_nodes_mutex);
-            nodes_copy = m_nodes;
-            for (CNode *pnode : nodes_copy) {
-                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 m_nodes list.
-        Shuffle(nodes_copy.begin(), nodes_copy.end(), rng);
-
-        for (CNode *pnode : nodes_copy) {
-            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};
 
-            bool fMoreNodeWork = false;
-            // Receive messages
-            for (auto interface : m_msgproc) {
-                fMoreNodeWork |= interface->ProcessMessages(
-                    *config, pnode, flagInterruptMsgProc);
-            }
-            fMoreWork |= (fMoreNodeWork && !pnode->fPauseSend);
-            if (flagInterruptMsgProc) {
-                return;
-            }
+            for (CNode *pnode : snap.Nodes()) {
+                if (pnode->fDisconnect) {
+                    continue;
+                }
 
-            // Send messages
-            for (auto interface : m_msgproc) {
-                interface->SendMessages(*config, pnode);
-            }
+                bool fMoreNodeWork = false;
+                // Receive messages
+                for (auto interface : m_msgproc) {
+                    fMoreNodeWork |= interface->ProcessMessages(
+                        *config, pnode, flagInterruptMsgProc);
+                }
+                fMoreWork |= (fMoreNodeWork && !pnode->fPauseSend);
+                if (flagInterruptMsgProc) {
+                    return;
+                }
 
-            if (flagInterruptMsgProc) {
-                return;
-            }
-        }
+                // Send messages
+                for (auto interface : m_msgproc) {
+                    interface->SendMessages(*config, pnode);
+                }
 
-        {
-            LOCK(m_nodes_mutex);
-            for (CNode *pnode : nodes_copy) {
-                pnode->Release();
+                if (flagInterruptMsgProc) {
+                    return;
+                }
             }
         }
 

src/net.h

@@ -1354,6 +1354,39 @@ class CConnman {
      */
     bool whitelist_relay;
 
+    /**
+     * RAII helper to atomically create a copy of `m_nodes` 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.m_nodes_mutex);
+                m_nodes_copy = connman.m_nodes;
+                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;
 };