From ccc3f267bada5e51a027e2703d686777ac6589f5 Mon Sep 17 00:00:00 2001
From: Patrick Strateman <patrick.strateman@gmail.com>
Date: Tue, 25 Sep 2018 15:32:07 -0400
Subject: [PATCH] Move GenerateSelectSet logic to private method.

This separates the socket event collection logic from the logic
deciding which events we're interested in at all.
---
 src/net.cpp | 109 ++++++++++++++++++++++++++++++----------------------
 src/net.h   |   1 +
 2 files changed, 63 insertions(+), 47 deletions(-)

diff --git a/src/net.cpp b/src/net.cpp
index 0503426cc2..50dcdfb78a 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1344,40 +1344,10 @@ void CConnman::InactivityCheck(CNode *pnode)
     }
 }
 
-void CConnman::SocketHandler()
+bool CConnman::GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set)
 {
-    //
-    // Find which sockets have data to receive
-    //
-    struct timeval timeout;
-    timeout.tv_sec  = 0;
-    timeout.tv_usec = SELECT_TIMEOUT_MILLISECONDS * 1000; // frequency to poll pnode->vSend
-
-    fd_set fdsetRecv;
-    fd_set fdsetSend;
-    fd_set fdsetError;
-    FD_ZERO(&fdsetRecv);
-    FD_ZERO(&fdsetSend);
-    FD_ZERO(&fdsetError);
-    SOCKET hSocketMax = 0;
-    bool have_fds = false;
-
-#ifndef WIN32
-    // We add a pipe to the read set so that the select() call can be woken up from the outside
-    // This is done when data is available for sending and at the same time optimistic sending was disabled
-    // when pushing the data.
-    // This is currently only implemented for POSIX compliant systems. This means that Windows will fall back to
-    // timing out after 50ms and then trying to send. This is ok as we assume that heavy-load daemons are usually
-    // run on Linux and friends.
-    FD_SET(wakeupPipe[0], &fdsetRecv);
-    hSocketMax = std::max(hSocketMax, (SOCKET)wakeupPipe[0]);
-    have_fds = true;
-#endif
-
     for (const ListenSocket& hListenSocket : vhListenSocket) {
-        FD_SET(hListenSocket.socket, &fdsetRecv);
-        hSocketMax = std::max(hSocketMax, hListenSocket.socket);
-        have_fds = true;
+        recv_set.insert(hListenSocket.socket);
     }
 
     {
@@ -1406,36 +1376,81 @@ void CConnman::SocketHandler()
             if (pnode->hSocket == INVALID_SOCKET)
                 continue;
 
-            FD_SET(pnode->hSocket, &fdsetError);
-            hSocketMax = std::max(hSocketMax, pnode->hSocket);
-            have_fds = true;
-
+            error_set.insert(pnode->hSocket);
             if (select_send) {
-                FD_SET(pnode->hSocket, &fdsetSend);
+                send_set.insert(pnode->hSocket);
                 continue;
             }
             if (select_recv) {
-                FD_SET(pnode->hSocket, &fdsetRecv);
+                recv_set.insert(pnode->hSocket);
             }
         }
     }
 
+    return !recv_set.empty() || !send_set.empty() || !error_set.empty();
+}
+
+void CConnman::SocketHandler()
+{
+    std::set<SOCKET> recv_select_set, send_select_set, error_select_set;
+    if (!GenerateSelectSet(recv_select_set, send_select_set, error_select_set)) {
+        interruptNet.sleep_for(std::chrono::milliseconds(SELECT_TIMEOUT_MILLISECONDS));
+        return;
+    }
+
+    //
+    // Find which sockets have data to receive
+    //
+    struct timeval timeout;
+    timeout.tv_sec  = 0;
+    timeout.tv_usec = SELECT_TIMEOUT_MILLISECONDS * 1000; // frequency to poll pnode->vSend
+
+    fd_set fdsetRecv;
+    fd_set fdsetSend;
+    fd_set fdsetError;
+    FD_ZERO(&fdsetRecv);
+    FD_ZERO(&fdsetSend);
+    FD_ZERO(&fdsetError);
+    SOCKET hSocketMax = 0;
+
+    for (SOCKET hSocket : recv_select_set) {
+        FD_SET(hSocket, &fdsetRecv);
+        hSocketMax = std::max(hSocketMax, hSocket);
+    }
+
+    for (SOCKET hSocket : send_select_set) {
+        FD_SET(hSocket, &fdsetSend);
+        hSocketMax = std::max(hSocketMax, hSocket);
+    }
+
+    for (SOCKET hSocket : error_select_set) {
+        FD_SET(hSocket, &fdsetError);
+        hSocketMax = std::max(hSocketMax, hSocket);
+    }
+
+#ifndef WIN32
+    // We add a pipe to the read set so that the select() call can be woken up from the outside
+    // This is done when data is available for sending and at the same time optimistic sending was disabled
+    // when pushing the data.
+    // This is currently only implemented for POSIX compliant systems. This means that Windows will fall back to
+    // timing out after 50ms and then trying to send. This is ok as we assume that heavy-load daemons are usually
+    // run on Linux and friends.
+    FD_SET(wakeupPipe[0], &fdsetRecv);
+    hSocketMax = std::max(hSocketMax, (SOCKET)wakeupPipe[0]);
+#endif
+
     wakeupSelectNeeded = true;
-    int nSelect = select(have_fds ? hSocketMax + 1 : 0,
-                         &fdsetRecv, &fdsetSend, &fdsetError, &timeout);
+    int nSelect = select(hSocketMax + 1, &fdsetRecv, &fdsetSend, &fdsetError, &timeout);
     wakeupSelectNeeded = false;
     if (interruptNet)
         return;
 
     if (nSelect == SOCKET_ERROR)
     {
-        if (have_fds)
-        {
-            int nErr = WSAGetLastError();
-            LogPrintf("socket select error %s\n", NetworkErrorString(nErr));
-            for (unsigned int i = 0; i <= hSocketMax; i++)
-                FD_SET(i, &fdsetRecv);
-        }
+        int nErr = WSAGetLastError();
+        LogPrintf("socket select error %s\n", NetworkErrorString(nErr));
+        for (unsigned int i = 0; i <= hSocketMax; i++)
+            FD_SET(i, &fdsetRecv);
         FD_ZERO(&fdsetSend);
         FD_ZERO(&fdsetError);
         if (!interruptNet.sleep_for(std::chrono::milliseconds(SELECT_TIMEOUT_MILLISECONDS)))
diff --git a/src/net.h b/src/net.h
index aac44d5820..2eded7d2f0 100644
--- a/src/net.h
+++ b/src/net.h
@@ -477,6 +477,7 @@ private:
     void DisconnectNodes();
     void NotifyNumConnectionsChanged();
     void InactivityCheck(CNode *pnode);
+    bool GenerateSelectSet(std::set<SOCKET> &recv_set, std::set<SOCKET> &send_set, std::set<SOCKET> &error_set);
     void SocketHandler();
     void ThreadSocketHandler();
     void ThreadDNSAddressSeed();