merge bitcoin#25421: convert standalone IsSelectableSocket() and SetSocketNonBlocking() to Sock methods

Author: kwvg

src/compat/compat.h

@@ -109,14 +109,6 @@ typedef char* sockopt_arg_type;
 #define USE_KQUEUE
 #endif
 
-bool static inline IsSelectableSocket(const SOCKET& s) {
-#if defined(USE_POLL) || defined(WIN32)
-    return true;
-#else
-    return (s < FD_SETSIZE);
-#endif
-}
-
 // MSG_NOSIGNAL is not available on some platforms, if it doesn't exist define it as 0
 #if !defined(MSG_NOSIGNAL)
 #define MSG_NOSIGNAL 0

src/masternode/node.cpp

@@ -162,7 +162,7 @@ void CActiveMasternodeManager::InitInternal(const CBlockIndex* pindex)
         LogPrintf("CActiveMasternodeManager::Init -- ERROR: %s\n", m_error);
         return;
     }
-    bool fConnected = ConnectSocketDirectly(m_info.service, *sock, nConnectTimeout, true) && IsSelectableSocket(sock->Get());
+    bool fConnected = ConnectSocketDirectly(m_info.service, *sock, nConnectTimeout, true) && sock->IsSelectable();
     sock->Reset();
 
     if (!fConnected && Params().RequireRoutableExternalIP()) {

src/net.cpp

@@ -2020,8 +2020,7 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
         return;
     }
 
-    if (!IsSelectableSocket(sock->Get()))
-    {
+    if (!sock->IsSelectable()) {
         LogPrintf("%s: non-selectable socket\n", strDropped);
         return;
     }

src/netbase.cpp

@@ -288,8 +288,7 @@ enum class IntrRecvError {
  *          read.
  *
  * @see This function can be interrupted by calling InterruptSocks5(bool).
- *      Sockets can be made non-blocking with SetSocketNonBlocking(const
- *      SOCKET&).
+ *      Sockets can be made non-blocking with Sock::SetNonBlocking().
  */
 static IntrRecvError InterruptibleRecv(uint8_t* data, size_t len, int timeout, const Sock& sock)
 {
@@ -487,7 +486,7 @@ std::unique_ptr<Sock> CreateSockTCP(const CService& address_family)
 
     // Ensure that waiting for I/O on this socket won't result in undefined
     // behavior.
-    if (!IsSelectableSocket(sock->Get())) {
+    if (!sock->IsSelectable()) {
         LogPrintf("Cannot create connection: non-selectable socket created (fd >= FD_SETSIZE ?)\n");
         return nullptr;
     }
@@ -509,7 +508,7 @@ std::unique_ptr<Sock> CreateSockTCP(const CService& address_family)
     }
 
     // Set the non-blocking option on the socket.
-    if (!SetSocketNonBlocking(sock->Get())) {
+    if (!sock->SetNonBlocking()) {
         LogPrintf("Error setting socket to non-blocking: %s\n", NetworkErrorString(WSAGetLastError()));
         return nullptr;
     }
@@ -701,21 +700,6 @@ bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out)
     return false;
 }
 
-bool SetSocketNonBlocking(const SOCKET& hSocket)
-{
-#ifdef WIN32
-    u_long nOne = 1;
-    if (ioctlsocket(hSocket, FIONBIO, &nOne) == SOCKET_ERROR) {
-#else
-    int fFlags = fcntl(hSocket, F_GETFL, 0);
-    if (fcntl(hSocket, F_SETFL, fFlags | O_NONBLOCK) == SOCKET_ERROR) {
-#endif
-        return false;
-    }
-
-    return true;
-}
-
 void InterruptSocks5(bool interrupt)
 {
     interruptSocks5Recv = interrupt;

src/netbase.h

@@ -224,8 +224,6 @@ bool ConnectSocketDirectly(const CService &addrConnect, const Sock& sock, int nT
  */
 bool ConnectThroughProxy(const Proxy& proxy, const std::string& strDest, uint16_t port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed);
 
-/** Enable non-blocking mode for a socket */
-bool SetSocketNonBlocking(const SOCKET& hSocket);
 void InterruptSocks5(bool interrupt);
 
 /**

src/test/fuzz/util.cpp

@@ -14,7 +14,7 @@
 #include <memory>
 
 FuzzedSock::FuzzedSock(FuzzedDataProvider& fuzzed_data_provider)
-    : m_fuzzed_data_provider{fuzzed_data_provider}
+    : m_fuzzed_data_provider{fuzzed_data_provider}, m_selectable{fuzzed_data_provider.ConsumeBool()}
 {
     m_socket = fuzzed_data_provider.ConsumeIntegralInRange<SOCKET>(INVALID_SOCKET - 1, INVALID_SOCKET);
 }
@@ -257,6 +257,24 @@ int FuzzedSock::GetSockName(sockaddr* name, socklen_t* name_len) const
     return 0;
 }
 
+bool FuzzedSock::SetNonBlocking() const
+{
+    constexpr std::array setnonblocking_errnos{
+        EBADF,
+        EPERM,
+    };
+    if (m_fuzzed_data_provider.ConsumeBool()) {
+        SetFuzzedErrNo(m_fuzzed_data_provider, setnonblocking_errnos);
+        return false;
+    }
+    return true;
+}
+
+bool FuzzedSock::IsSelectable() const
+{
+    return m_selectable;
+}
+
 bool FuzzedSock::Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred) const
 {
     constexpr std::array wait_errnos{

src/test/fuzz/util.h

@@ -53,6 +53,13 @@ class FuzzedSock : public Sock
      */
     mutable std::optional<uint8_t> m_peek_data;
 
+    /**
+     * Whether to pretend that the socket is select(2)-able. This is randomly set in the
+     * constructor. It should remain constant so that repeated calls to `IsSelectable()`
+     * return the same value.
+     */
+    const bool m_selectable;
+
 public:
     explicit FuzzedSock(FuzzedDataProvider& fuzzed_data_provider);
 
@@ -80,6 +87,10 @@ class FuzzedSock : public Sock
 
     int GetSockName(sockaddr* name, socklen_t* name_len) const override;
 
+    bool SetNonBlocking() const override;
+
+    bool IsSelectable() const override;
+
     bool Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred = nullptr) const override;
 
     bool IsConnected(std::string& errmsg) const override;

src/test/util/net.h

@@ -201,6 +201,10 @@ class StaticContentsSock : public Sock
         return 0;
     }
 
+    bool SetNonBlocking() const override { return true; }
+
+    bool IsSelectable() const override { return true; }
+
     bool Wait(std::chrono::milliseconds timeout,
               Event requested,
               Event* occurred = nullptr) const override

src/util/sock.cpp

@@ -125,6 +125,34 @@ int Sock::GetSockName(sockaddr* name, socklen_t* name_len) const
     return getsockname(m_socket, name, name_len);
 }
 
+bool Sock::SetNonBlocking() const
+{
+#ifdef WIN32
+    u_long on{1};
+    if (ioctlsocket(m_socket, FIONBIO, &on) == SOCKET_ERROR) {
+        return false;
+    }
+#else
+    const int flags{fcntl(m_socket, F_GETFL, 0)};
+    if (flags == SOCKET_ERROR) {
+        return false;
+    }
+    if (fcntl(m_socket, F_SETFL, flags | O_NONBLOCK) == SOCKET_ERROR) {
+        return false;
+    }
+#endif
+    return true;
+}
+
+bool Sock::IsSelectable() const
+{
+#if defined(USE_POLL) || defined(WIN32)
+    return true;
+#else
+    return m_socket < FD_SETSIZE;
+#endif
+}
+
 bool Sock::Wait(std::chrono::milliseconds timeout, Event requested, Event* occurred) const
 {
 #ifdef USE_POLL
@@ -154,7 +182,7 @@ bool Sock::Wait(std::chrono::milliseconds timeout, Event requested, Event* occur
 
     return true;
 #else
-    if (!IsSelectableSocket(m_socket)) {
+    if (!IsSelectable()) {
         return false;
     }
 

src/util/sock.h

@@ -190,6 +190,18 @@ class Sock
      */
     [[nodiscard]] virtual int GetSockName(sockaddr* name, socklen_t* name_len) const;
 
+    /**
+     * Set the non-blocking option on the socket.
+     * @return true if set successfully
+     */
+    [[nodiscard]] virtual bool SetNonBlocking() const;
+
+    /**
+     * Check if the underlying socket can be used for `select(2)` (or the `Wait()` method).
+     * @return true if selectable
+     */
+    [[nodiscard]] virtual bool IsSelectable() const;
+
     using Event = uint8_t;
 
     /**