Merge #20788: net: add RAII socket and use it instead of bare SOCKET

615ba0eb96 test: add Sock unit tests (Vasil Dimov)
7bd21ce1ef style: rename hSocket to sock (Vasil Dimov)
04ae846904 net: use Sock in InterruptibleRecv() and Socks5() (Vasil Dimov)
ba9d73268f net: add RAII socket and use it instead of bare SOCKET (Vasil Dimov)
dec9b5e850 net: move CloseSocket() from netbase to util/sock (Vasil Dimov)
aa17a44551 net: move MillisToTimeval() from netbase to util/time (Vasil Dimov)

Pull request description:

  Introduce a class to manage the lifetime of a socket - when the object
  that contains the socket goes out of scope, the underlying socket will
  be closed.

  In addition, the new `Sock` class has a `Send()`, `Recv()` and `Wait()`
  methods that can be overridden by unit tests to mock the socket
  operations.

  The `Wait()` method also hides the
  `#ifdef USE_POLL poll() #else select() #endif` technique from higher
  level code.

ACKs for top commit:
  laanwj:
    Re-ACK 615ba0eb96
  jonatack:
    re-ACK 615ba0eb96

Tree-SHA512: 3003e6bc0259295ca0265ccdeb1522ee25b4abe66d32e6ceaa51b55e0a999df7ddee765f86ce558a788c1953ee2009bfa149b09d494593f7d799c0d7d930bee8

src/util/sock.cpp

@@ -0,0 +1,149 @@
+// Copyright (c) 2020-2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <compat.h>
+#include <logging.h>
+#include <tinyformat.h>
+#include <util/sock.h>
+#include <util/system.h>
+#include <util/time.h>
+
+#include <codecvt>
+#include <cwchar>
+#include <locale>
+#include <string>
+
+#ifdef USE_POLL
+#include <poll.h>
+#endif
+
+Sock::Sock() : m_socket(INVALID_SOCKET) {}
+
+Sock::Sock(SOCKET s) : m_socket(s) {}
+
+Sock::Sock(Sock&& other)
+{
+    m_socket = other.m_socket;
+    other.m_socket = INVALID_SOCKET;
+}
+
+Sock::~Sock() { Reset(); }
+
+Sock& Sock::operator=(Sock&& other)
+{
+    Reset();
+    m_socket = other.m_socket;
+    other.m_socket = INVALID_SOCKET;
+    return *this;
+}
+
+SOCKET Sock::Get() const { return m_socket; }
+
+SOCKET Sock::Release()
+{
+    const SOCKET s = m_socket;
+    m_socket = INVALID_SOCKET;
+    return s;
+}
+
+void Sock::Reset() { CloseSocket(m_socket); }
+
+ssize_t Sock::Send(const void* data, size_t len, int flags) const
+{
+    return send(m_socket, static_cast<const char*>(data), len, flags);
+}
+
+ssize_t Sock::Recv(void* buf, size_t len, int flags) const
+{
+    return recv(m_socket, static_cast<char*>(buf), len, flags);
+}
+
+bool Sock::Wait(std::chrono::milliseconds timeout, Event requested) const
+{
+#ifdef USE_POLL
+    pollfd fd;
+    fd.fd = m_socket;
+    fd.events = 0;
+    if (requested & RECV) {
+        fd.events |= POLLIN;
+    }
+    if (requested & SEND) {
+        fd.events |= POLLOUT;
+    }
+
+    return poll(&fd, 1, count_milliseconds(timeout)) != SOCKET_ERROR;
+#else
+    if (!IsSelectableSocket(m_socket)) {
+        return false;
+    }
+
+    fd_set fdset_recv;
+    fd_set fdset_send;
+    FD_ZERO(&fdset_recv);
+    FD_ZERO(&fdset_send);
+
+    if (requested & RECV) {
+        FD_SET(m_socket, &fdset_recv);
+    }
+
+    if (requested & SEND) {
+        FD_SET(m_socket, &fdset_send);
+    }
+
+    timeval timeout_struct = MillisToTimeval(timeout);
+
+    return select(m_socket + 1, &fdset_recv, &fdset_send, nullptr, &timeout_struct) != SOCKET_ERROR;
+#endif /* USE_POLL */
+}
+
+#ifdef WIN32
+std::string NetworkErrorString(int err)
+{
+    wchar_t buf[256];
+    buf[0] = 0;
+    if(FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_MAX_WIDTH_MASK,
+            nullptr, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+            buf, ARRAYSIZE(buf), nullptr))
+    {
+        return strprintf("%s (%d)", std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>,wchar_t>().to_bytes(buf), err);
+    }
+    else
+    {
+        return strprintf("Unknown error (%d)", err);
+    }
+}
+#else
+std::string NetworkErrorString(int err)
+{
+    char buf[256];
+    buf[0] = 0;
+    /* Too bad there are two incompatible implementations of the
+     * thread-safe strerror. */
+    const char *s;
+#ifdef STRERROR_R_CHAR_P /* GNU variant can return a pointer outside the passed buffer */
+    s = strerror_r(err, buf, sizeof(buf));
+#else /* POSIX variant always returns message in buffer */
+    s = buf;
+    if (strerror_r(err, buf, sizeof(buf)))
+        buf[0] = 0;
+#endif
+    return strprintf("%s (%d)", s, err);
+}
+#endif
+
+bool CloseSocket(SOCKET& hSocket)
+{
+    if (hSocket == INVALID_SOCKET)
+        return false;
+#ifdef WIN32
+    int ret = closesocket(hSocket);
+#else
+    int ret = close(hSocket);
+#endif
+    if (ret) {
+        LogPrintf("Socket close failed: %d. Error: %s\n", hSocket, NetworkErrorString(WSAGetLastError()));
+    }
+    hSocket = INVALID_SOCKET;
+    return ret != SOCKET_ERROR;
+}

src/util/sock.h

@@ -0,0 +1,118 @@
+// Copyright (c) 2020-2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_UTIL_SOCK_H
+#define BITCOIN_UTIL_SOCK_H
+
+#include <compat.h>
+
+#include <chrono>
+#include <string>
+
+/**
+ * RAII helper class that manages a socket. Mimics `std::unique_ptr`, but instead of a pointer it
+ * contains a socket and closes it automatically when it goes out of scope.
+ */
+class Sock
+{
+public:
+    /**
+     * Default constructor, creates an empty object that does nothing when destroyed.
+     */
+    Sock();
+
+    /**
+     * Take ownership of an existent socket.
+     */
+    explicit Sock(SOCKET s);
+
+    /**
+     * Copy constructor, disabled because closing the same socket twice is undesirable.
+     */
+    Sock(const Sock&) = delete;
+
+    /**
+     * Move constructor, grab the socket from another object and close ours (if set).
+     */
+    Sock(Sock&& other);
+
+    /**
+     * Destructor, close the socket or do nothing if empty.
+     */
+    virtual ~Sock();
+
+    /**
+     * Copy assignment operator, disabled because closing the same socket twice is undesirable.
+     */
+    Sock& operator=(const Sock&) = delete;
+
+    /**
+     * Move assignment operator, grab the socket from another object and close ours (if set).
+     */
+    virtual Sock& operator=(Sock&& other);
+
+    /**
+     * Get the value of the contained socket.
+     * @return socket or INVALID_SOCKET if empty
+     */
+    virtual SOCKET Get() const;
+
+    /**
+     * Get the value of the contained socket and drop ownership. It will not be closed by the
+     * destructor after this call.
+     * @return socket or INVALID_SOCKET if empty
+     */
+    virtual SOCKET Release();
+
+    /**
+     * Close if non-empty.
+     */
+    virtual void Reset();
+
+    /**
+     * send(2) wrapper. Equivalent to `send(this->Get(), data, len, flags);`. Code that uses this
+     * wrapper can be unit-tested if this method is overridden by a mock Sock implementation.
+     */
+    virtual ssize_t Send(const void* data, size_t len, int flags) const;
+
+    /**
+     * recv(2) wrapper. Equivalent to `recv(this->Get(), buf, len, flags);`. Code that uses this
+     * wrapper can be unit-tested if this method is overridden by a mock Sock implementation.
+     */
+    virtual ssize_t Recv(void* buf, size_t len, int flags) const;
+
+    using Event = uint8_t;
+
+    /**
+     * If passed to `Wait()`, then it will wait for readiness to read from the socket.
+     */
+    static constexpr Event RECV = 0b01;
+
+    /**
+     * If passed to `Wait()`, then it will wait for readiness to send to the socket.
+     */
+    static constexpr Event SEND = 0b10;
+
+    /**
+     * Wait for readiness for input (recv) or output (send).
+     * @param[in] timeout Wait this much for at least one of the requested events to occur.
+     * @param[in] requested Wait for those events, bitwise-or of `RECV` and `SEND`.
+     * @return true on success and false otherwise
+     */
+    virtual bool Wait(std::chrono::milliseconds timeout, Event requested) const;
+
+private:
+    /**
+     * Contained socket. `INVALID_SOCKET` designates the object is empty.
+     */
+    SOCKET m_socket;
+};
+
+/** Return readable error string for a network error code */
+std::string NetworkErrorString(int err);
+
+/** Close socket and set hSocket to INVALID_SOCKET */
+bool CloseSocket(SOCKET& hSocket);
+
+#endif // BITCOIN_UTIL_SOCK_H

src/util/time.cpp

@@ -7,6 +7,7 @@
 #include <config/bitcoin-config.h>
 #endif
 
+#include <compat.h>
 #include <util/time.h>
 
 #include <util/check.h>
@@ -117,3 +118,16 @@ int64_t ParseISO8601DateTime(const std::string& str)
         return 0;
     return (ptime - epoch).total_seconds();
 }
+
+struct timeval MillisToTimeval(int64_t nTimeout)
+{
+    struct timeval timeout;
+    timeout.tv_sec  = nTimeout / 1000;
+    timeout.tv_usec = (nTimeout % 1000) * 1000;
+    return timeout;
+}
+
+struct timeval MillisToTimeval(std::chrono::milliseconds ms)
+{
+    return MillisToTimeval(count_milliseconds(ms));
+}

src/util/time.h

@@ -6,6 +6,8 @@
 #ifndef BITCOIN_UTIL_TIME_H
 #define BITCOIN_UTIL_TIME_H
 
+#include <compat.h>
+
 #include <chrono>
 #include <stdint.h>
 #include <string>
@@ -25,6 +27,7 @@ void UninterruptibleSleep(const std::chrono::microseconds& n);
  * interface that doesn't support std::chrono (e.g. RPC, debug log, or the GUI)
  */
 inline int64_t count_seconds(std::chrono::seconds t) { return t.count(); }
+inline int64_t count_milliseconds(std::chrono::milliseconds t) { return t.count(); }
 inline int64_t count_microseconds(std::chrono::microseconds t) { return t.count(); }
 
 /**
@@ -57,4 +60,14 @@ std::string FormatISO8601DateTime(int64_t nTime);
 std::string FormatISO8601Date(int64_t nTime);
 int64_t ParseISO8601DateTime(const std::string& str);
 
+/**
+ * Convert milliseconds to a struct timeval for e.g. select.
+ */
+struct timeval MillisToTimeval(int64_t nTimeout);
+
+/**
+ * Convert milliseconds to a struct timeval for e.g. select.
+ */
+struct timeval MillisToTimeval(std::chrono::milliseconds ms);
+
 #endif // BITCOIN_UTIL_TIME_H