net: keep reference to each node during socket wait

Create the snapshot of `CConnman::vNodes` to operate on earlier in
`CConnman::SocketHandler()`, before calling `CConnman::SocketEvents()`
and pass the `vNodes` copy from the snapshot to `SocketEvents()`.

This will keep the refcount of each node incremented during
`SocketEvents()` so that the `CNode` object is not destroyed before
`SocketEvents()` has finished.

Currently in `SocketEvents()` we only remember file descriptor numbers
(when not holding `CConnman::cs_vNodes`) which is safe, but we will
change this to remember pointers to `CNode::m_sock`.

1 files changed, 18 insertions, 11 deletions

diff --git a/src/net.cpp b/src/net.cpp
index 165a28c04e..6fab80f976 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1331,16 +1331,17 @@ bool CConnman::InactivityCheck(const CNode& node) const
     return false;
 }
 
-bool CConnman::GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set)
+bool CConnman::GenerateSelectSet(const std::vector<CNode*>& nodes,
+                                 std::set<SOCKET>& recv_set,
+                                 std::set<SOCKET>& send_set,
+                                 std::set<SOCKET>& error_set)
 {
     for (const ListenSocket& hListenSocket : vhListenSocket) {
         recv_set.insert(hListenSocket.socket);
     }
 
     {
-        LOCK(cs_vNodes);
-        for (CNode* pnode : vNodes)
-        {
+        for (CNode* pnode : nodes) {
             // Implement the following logic:
             // * If there is data to send, select() for sending data. As this only
             //   happens when optimistic write failed, we choose to first drain the
@@ -1378,10 +1379,13 @@ bool CConnman::GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &s
 }
 
 #ifdef USE_POLL
-void CConnman::SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set)
+void CConnman::SocketEvents(const std::vector<CNode*>& nodes,
+                            std::set<SOCKET>& recv_set,
+                            std::set<SOCKET>& send_set,
+                            std::set<SOCKET>& error_set)
 {
     std::set<SOCKET> recv_select_set, send_select_set, error_select_set;
-    if (!GenerateSelectSet(recv_select_set, send_select_set, error_select_set)) {
+    if (!GenerateSelectSet(nodes, recv_select_set, send_select_set, error_select_set)) {
         interruptNet.sleep_for(std::chrono::milliseconds(SELECT_TIMEOUT_MILLISECONDS));
         return;
     }
@@ -1420,10 +1424,13 @@ void CConnman::SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_s
     }
 }
 #else
-void CConnman::SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set)
+void CConnman::SocketEvents(const std::vector<CNode*>& nodes,
+                            std::set<SOCKET>& recv_set,
+                            std::set<SOCKET>& send_set,
+                            std::set<SOCKET>& error_set)
 {
     std::set<SOCKET> recv_select_set, send_select_set, error_select_set;
-    if (!GenerateSelectSet(recv_select_set, send_select_set, error_select_set)) {
+    if (!GenerateSelectSet(nodes, recv_select_set, send_select_set, error_select_set)) {
         interruptNet.sleep_for(std::chrono::milliseconds(SELECT_TIMEOUT_MILLISECONDS));
         return;
     }
@@ -1497,8 +1504,10 @@ void CConnman::SocketEvents(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_s
 
 void CConnman::SocketHandler()
 {
+    const NodesSnapshot snap{*this, /*shuffle=*/false};
+
     std::set<SOCKET> recv_set, send_set, error_set;
-    SocketEvents(recv_set, send_set, error_set);
+    SocketEvents(snap.Nodes(), recv_set, send_set, error_set);
 
     if (interruptNet) return;
 
@@ -1513,8 +1522,6 @@ void CConnman::SocketHandler()
         }
     }
 
-    const NodesSnapshot snap{*this, /*shuffle=*/false};
-
     //
     // Service each socket
     //