chore: remove scaffolding (remove default args, make explicit choice)

This was acceptable while we were refining earlier commits but default
args could in combination with future backports result in behavior
changes that were unanticipated, we should clear the air right now.

We are introducing a new global, `g_socket_events_mode` to allow us to
influence Wait(Many)() calls as and when needed. A global is acceptable
as it's lightweight, a parameter that is not configurable during runtime
except during initial startup and because the structure in which it is
ordinarily stored is far heavier.

Co-authored-by: UdjinM6 <[email protected]>

Author: kwvg

src/i2p.cpp

@@ -160,7 +160,7 @@ bool Session::Accept(Connection& conn)
 
     while (!*m_interrupt) {
         Sock::Event occurred;
-        if (!conn.sock->Wait(MAX_WAIT_FOR_IO, Sock::RECV, SocketEventsParams(), &occurred)) {
+        if (!conn.sock->Wait(MAX_WAIT_FOR_IO, Sock::RECV, SocketEventsParams{::g_socket_events_mode}, &occurred)) {
             errmsg = "wait on socket failed";
             break;
         }

src/init.cpp

@@ -480,21 +480,6 @@ static void OnRPCStopped()
     LogPrint(BCLog::RPC, "RPC stopped.\n");
 }
 
-std::string GetSupportedSocketEventsStr()
-{
-    std::string strSupportedModes = "'select'";
-#ifdef USE_POLL
-    strSupportedModes += ", 'poll'";
-#endif
-#ifdef USE_EPOLL
-    strSupportedModes += ", 'epoll'";
-#endif
-#ifdef USE_KQUEUE
-    strSupportedModes += ", 'kqueue'";
-#endif
-    return strSupportedModes;
-}
-
 void SetupServerArgs(ArgsManager& argsman)
 {
     SetupHelpOptions(argsman);
@@ -2533,6 +2518,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         return InitError(strprintf(_("Invalid -socketevents ('%s') specified. Only these modes are supported: %s"), sem_str, GetSupportedSocketEventsStr()));
     }
     connOptions.socketEventsMode = sem;
+    ::g_socket_events_mode = sem;
 
     const std::string& i2psam_arg = args.GetArg("-i2psam", "");
     if (!i2psam_arg.empty()) {

src/masternode/node.cpp

@@ -156,7 +156,7 @@ void CActiveMasternodeManager::InitInternal(const CBlockIndex* pindex)
     // Check socket connectivity
     LogPrintf("CActiveMasternodeManager::Init -- Checking inbound connection to '%s'\n", m_info.service.ToStringAddrPort());
     std::unique_ptr<Sock> sock{ConnectDirectly(m_info.service, /*manual_connection=*/true)};
-    bool fConnected{sock && sock->IsSelectable()};
+    bool fConnected{sock && sock->IsSelectable(/*is_select=*/::g_socket_events_mode == SocketEventsMode::Select)};
     sock = std::make_unique<Sock>(INVALID_SOCKET);
     if (!fConnected && Params().RequireRoutableExternalIP()) {
         m_state = MasternodeState::SOME_ERROR;

src/net.cpp

@@ -1928,7 +1928,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
         return;
     }
 
-    if (!sock->IsSelectable()) {
+    if (!sock->IsSelectable(/*is_select=*/::g_socket_events_mode == SocketEventsMode::Select)) {
         LogPrintf("%s: non-selectable socket\n", strDropped);
         return;
     }

src/netbase.cpp

@@ -336,7 +336,7 @@ static IntrRecvError InterruptibleRecv(uint8_t* data, size_t len, int timeout, c
                 // we're approaching the end of the specified total timeout
                 const auto remaining = std::chrono::milliseconds{endTime - curTime};
                 const auto timeout = std::min(remaining, std::chrono::milliseconds{MAX_WAIT_FOR_IO});
-                if (!sock.Wait(timeout, Sock::RECV)) {
+                if (!sock.Wait(timeout, Sock::RECV, SocketEventsParams{::g_socket_events_mode})) {
                     return IntrRecvError::NetworkError;
                 }
             } else {
@@ -514,7 +514,7 @@ std::unique_ptr<Sock> CreateSockOS(sa_family_t address_family)
 
     // Ensure that waiting for I/O on this socket won't result in undefined
     // behavior.
-    if (!sock->IsSelectable()) {
+    if (!sock->IsSelectable(/*is_select=*/::g_socket_events_mode == SocketEventsMode::Select)) {
         LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n");
         return nullptr;
     }
@@ -572,7 +572,7 @@ static bool ConnectToSocket(const Sock& sock, struct sockaddr* sockaddr, socklen
             // synchronously to check for successful connection with a timeout.
             const Sock::Event requested = Sock::RECV | Sock::SEND;
             Sock::Event occurred;
-            if (!sock.Wait(std::chrono::milliseconds{nConnectTimeout}, requested, SocketEventsParams(), &occurred)) {
+            if (!sock.Wait(std::chrono::milliseconds{nConnectTimeout}, requested, SocketEventsParams{::g_socket_events_mode}, &occurred)) {
                 LogPrintf("wait for connect to %s failed: %s\n",
                           dest_str,
                           NetworkErrorString(WSAGetLastError()));

src/test/fuzz/util.h

@@ -89,9 +89,9 @@ class FuzzedSock : public Sock
 
     bool IsSelectable(bool is_select) const override;
 
-    bool Wait(std::chrono::milliseconds timeout, Event requested, SocketEventsParams event_params = SocketEventsParams(), Event* occurred = nullptr) const override;
+    bool Wait(std::chrono::milliseconds timeout, Event requested, SocketEventsParams event_params, Event* occurred = nullptr) const override;
 
-    bool WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_per_sock, SocketEventsParams event_params = SocketEventsParams()) const override;
+    bool WaitMany(std::chrono::milliseconds timeout, EventsPerSock& events_per_sock, SocketEventsParams event_params) const override;
 
     bool IsConnected(std::string& errmsg) const override;
 };

src/test/sock_tests.cpp

@@ -121,7 +121,7 @@ BOOST_AUTO_TEST_CASE(wait)
     Sock sock0(s[0]);
     Sock sock1(s[1]);
 
-    std::thread waiter([&sock0]() { (void)sock0.Wait(24h, Sock::RECV); });
+    std::thread waiter([&sock0]() { (void)sock0.Wait(24h, Sock::RECV, SocketEventsParams{::g_socket_events_mode}); });
 
     BOOST_REQUIRE_EQUAL(sock1.Send("a", 1, 0), 1);
 

src/test/util/net.h

@@ -202,7 +202,7 @@ class StaticContentsSock : public Sock
 
     bool Wait(std::chrono::milliseconds timeout,
               Event requested,
-              SocketEventsParams event_params = SocketEventsParams(),
+              SocketEventsParams event_params,
               Event* occurred = nullptr) const override
     {
         if (occurred != nullptr) {
@@ -213,7 +213,7 @@ class StaticContentsSock : public Sock
 
     bool WaitMany(std::chrono::milliseconds timeout,
                   EventsPerSock& events_per_sock,
-                  SocketEventsParams event_params = SocketEventsParams()) const override
+                  SocketEventsParams event_params) const override
     {
         for (auto& [sock, events] : events_per_sock) {
             (void)sock;

src/test/util/setup_common.cpp

@@ -189,6 +189,15 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
     m_node.addrman = std::make_unique<AddrMan>(*m_node.netgroupman,
                                                /*deterministic=*/false,
                                                m_node.args->GetIntArg("-checkaddrman", 0));
+
+    std::string sem_str = m_args.GetArg("-socketevents", DEFAULT_SOCKETEVENTS);
+    ::g_socket_events_mode = SEMFromString(sem_str);
+    if (::g_socket_events_mode == SocketEventsMode::Unknown) {
+        throw std::runtime_error(
+            strprintf("Invalid -socketevents ('%s') specified. Only these modes are supported: %s",
+                      sem_str, GetSupportedSocketEventsStr()));
+    }
+
     m_node.connman = std::make_unique<CConnman>(0x1337, 0x1337, *m_node.addrman, *m_node.netgroupman); // Deterministic randomness for tests.
 
     fCheckBlockIndex = true;
@@ -209,6 +218,7 @@ BasicTestingSetup::~BasicTestingSetup()
     m_node.cpoolman.reset();
     m_node.mnhf_manager.reset();
     m_node.evodb.reset();
+    ::g_socket_events_mode = SocketEventsMode::Unknown;
     m_node.connman.reset();
     m_node.addrman.reset();
     m_node.netgroupman.reset();
@@ -326,6 +336,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
     {
         CConnman::Options options;
         options.m_msgproc = m_node.peerman.get();
+        options.socketEventsMode = ::g_socket_events_mode;
         m_node.connman->Init(options);
     }
 

src/util/sock.cpp

@@ -32,6 +32,8 @@
 #include <poll.h>
 #endif
 
+SocketEventsMode g_socket_events_mode{SocketEventsMode::Unknown};
+
 static inline bool IOErrorIsPermanent(int err)
 {
     return err != WSAEAGAIN && err != WSAEINTR && err != WSAEWOULDBLOCK && err != WSAEINPROGRESS;
@@ -162,10 +164,17 @@ bool Sock::Wait(std::chrono::milliseconds timeout, Event requested, SocketEvents
 {
     EventsPerSock events_per_sock{std::make_pair(m_socket, Events{requested})};
 
+    // We need to ensure we are only using a level-triggered mode because we are expecting
+    // a direct correlation between the events reported and the one socket we are querying
     if (auto [sem, _, __] = event_params; sem != SocketEventsMode::Poll && sem != SocketEventsMode::Select) {
-        // We need to ensure we are only using a level-triggered mode because we are expecting
-        // a direct correlation between the events reported and the one socket we are querying
-        event_params = SocketEventsParams();
+        // We will use a compatible fallback events mode if we didn't specify a valid option
+        event_params = SocketEventsParams{
+#ifdef USE_POLL
+            SocketEventsMode::Poll
+#else
+            SocketEventsMode::Select
+#endif /* USE_POLL */
+        };
     }
     if (!WaitMany(timeout, events_per_sock, event_params)) {
         return false;
@@ -435,7 +444,7 @@ void Sock::SendComplete(const std::string& data,
         // Wait for a short while (or the socket to become ready for sending) before retrying
         // if nothing was sent.
         const auto wait_time = std::min(deadline - now, std::chrono::milliseconds{MAX_WAIT_FOR_IO});
-        (void)Wait(wait_time, SEND);
+        (void)Wait(wait_time, SEND, SocketEventsParams{::g_socket_events_mode});
     }
 }
 
@@ -518,7 +527,7 @@ std::string Sock::RecvUntilTerminator(uint8_t terminator,
 
         // Wait for a short while (or the socket to become ready for reading) before retrying.
         const auto wait_time = std::min(deadline - now, std::chrono::milliseconds{MAX_WAIT_FOR_IO});
-        (void)Wait(wait_time, RECV);
+        (void)Wait(wait_time, RECV, SocketEventsParams{::g_socket_events_mode});
     }
 }