refactor: move SOCKET addition/removal from interest list to ETE

Additionally, log errors if removal from interest list fails (which is possible if it was already removed or socket is invalid).
dashpay · May 14, 2024 · 3a9f386 · 3a9f386
1 parent 212df06
commit 3a9f386
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 35 deletions.
diff --git a/src/net.cpp b/src/net.cpp
@@ -3132,30 +3132,10 @@ bool CConnman::BindListenPort(const CService& addrBind, bilingual_str& strError,
         return false;
     }
 
-#ifdef USE_KQUEUE
-    if (socketEventsMode == SocketEventsMode::KQueue) {
-        struct kevent event;
-        EV_SET(&event, sock->Get(), EVFILT_READ, EV_ADD, 0, 0, nullptr);
-        if (kevent(Assert(m_edge_trig_events)->m_fd, &event, 1, nullptr, 0, nullptr) != 0) {
-            strError = strprintf(_("Error: failed to add socket to kqueue fd (kevent returned error %s)"), NetworkErrorString(WSAGetLastError()));
-            LogPrintf("%s\n", strError.original);
-            return false;
-        }
-    }
-#endif
-
-#ifdef USE_EPOLL
-    if (socketEventsMode == SocketEventsMode::EPoll) {
-        epoll_event event;
-        event.data.fd = sock->Get();
-        event.events = EPOLLIN;
-        if (epoll_ctl(Assert(m_edge_trig_events)->m_fd, EPOLL_CTL_ADD, sock->Get(), &event) != 0) {
-            strError = strprintf(_("Error: failed to add socket to epoll fd (epoll_ctl returned error %s)"), NetworkErrorString(WSAGetLastError()));
-            LogPrintf("%s\n", strError.original);
-            return false;
-        }
+    if (m_edge_trig_events && !m_edge_trig_events->AddSocket(sock->Get())) {
+        LogPrintf("Error: EdgeTriggeredEvents::AddSocket() failed\n");
+        return false;
     }
-#endif
 
     vhListenSocket.push_back(ListenSocket(sock->Release(), permissions));
 
@@ -3553,23 +3533,15 @@ void CConnman::StopNodes()
         for (CNode *pnode : m_nodes)
             pnode->CloseSocketDisconnect(this);
     }
-    for (ListenSocket& hListenSocket : vhListenSocket)
+    for (ListenSocket& hListenSocket : vhListenSocket) {
         if (hListenSocket.socket != INVALID_SOCKET) {
-#ifdef USE_KQUEUE
-            if (socketEventsMode == SocketEventsMode::KQueue) {
-                struct kevent event;
-                EV_SET(&event, hListenSocket.socket, EVFILT_READ, EV_DELETE, 0, 0, nullptr);
-                kevent(Assert(m_edge_trig_events)->m_fd, &event, 1, nullptr, 0, nullptr);
+            if (m_edge_trig_events && !m_edge_trig_events->RemoveSocket(hListenSocket.socket)) {
+                LogPrintf("EdgeTriggeredEvents::RemoveSocket() failed\n");
             }
-#endif
-#ifdef USE_EPOLL
-            if (socketEventsMode == SocketEventsMode::EPoll) {
-                epoll_ctl(Assert(m_edge_trig_events)->m_fd, EPOLL_CTL_DEL, hListenSocket.socket, nullptr);
-            }
-#endif
             if (!CloseSocket(hListenSocket.socket))
                 LogPrintf("CloseSocket(hListenSocket) failed with error %s\n", NetworkErrorString(WSAGetLastError()));
         }
+    }
 
     // clean up some globals (to help leak detection)
     std::vector<CNode*> nodes;

diff --git a/src/util/edge.cpp b/src/util/edge.cpp
@@ -58,3 +58,70 @@ EdgeTriggeredEvents::~EdgeTriggeredEvents()
 #endif /* defined(USE_KQUEUE) || defined(USE_EPOLL) */
     }
 }
+
+bool EdgeTriggeredEvents::AddSocket(SOCKET socket) const
+{
+    assert(m_valid);
+
+    if (m_mode == SocketEventsMode::EPoll) {
+#ifdef USE_EPOLL
+        epoll_event event;
+        event.data.fd = socket;
+        event.events = EPOLLIN;
+        if (epoll_ctl(m_fd, EPOLL_CTL_ADD, socket, &event) != 0) {
+            LogPrintf("Failed to add socket to epoll fd (epoll_ctl returned error %s)\n",
+                      NetworkErrorString(WSAGetLastError()));
+            return false;
+        }
+#else
+        assert(false);
+#endif /* USE_EPOLL */
+    } else if (m_mode == SocketEventsMode::KQueue) {
+#ifdef USE_KQUEUE
+        struct kevent event;
+        EV_SET(&event, socket, EVFILT_READ, EV_ADD, 0, 0, nullptr);
+        if (kevent(m_fd, &event, 1, nullptr, 0, nullptr) != 0) {
+            LogPrintf("Failed to add socket to kqueue fd (kevent returned error %s)\n",
+                      NetworkErrorString(WSAGetLastError()));
+            return false;
+        }
+#else
+        assert(false);
+#endif /* USE_KQUEUE */
+    } else {
+        assert(false);
+    }
+    return true;
+}
+
+bool EdgeTriggeredEvents::RemoveSocket(SOCKET socket) const
+{
+    assert(m_valid);
+
+    if (m_mode == SocketEventsMode::EPoll) {
+#ifdef USE_EPOLL
+        if (epoll_ctl(m_fd, EPOLL_CTL_DEL, socket, nullptr) != 0) {
+            LogPrintf("Failed to remove socket from epoll fd (epoll_ctl returned error %s)\n",
+                      NetworkErrorString(WSAGetLastError()));
+            return false;
+        }
+#else
+        assert(false);
+#endif /* USE_EPOLL */
+    } else if (m_mode == SocketEventsMode::KQueue) {
+#ifdef USE_KQUEUE
+        struct kevent event;
+        EV_SET(&event, socket, EVFILT_READ, EV_DELETE, 0, 0, nullptr);
+        if (kevent(m_fd, &event, 1, nullptr, 0, nullptr) != 0) {
+            LogPrintf("Failed to remove socket from kqueue fd (kevent returned error %s)\n",
+                      NetworkErrorString(WSAGetLastError()));
+            return false;
+        }
+#else
+        assert(false);
+#endif /* USE_KQUEUE */
+    } else {
+        assert(false);
+    }
+    return true;
+}
diff --git a/src/util/edge.h b/src/util/edge.h
@@ -5,6 +5,8 @@
 #ifndef BITCOIN_UTIL_EDGE_H
 #define BITCOIN_UTIL_EDGE_H
 
+#include <compat.h>
+
 #include <cstdint>
 
 enum class SocketEventsMode : int8_t;
@@ -19,6 +21,11 @@ class EdgeTriggeredEvents
     explicit EdgeTriggeredEvents(SocketEventsMode events_mode);
     ~EdgeTriggeredEvents();
 
+    /* Add socket to interest list */
+    bool AddSocket(SOCKET socket) const;
+    /* Remove socket from interest list */
+    bool RemoveSocket(SOCKET socket) const;
+
     bool IsValid() const { return m_valid; }
 
 public: