Merge bitcoin#28486: test, bench: Initialize and terminate use of Win…

…sock properly

fd4c6a1 test: Setup networking globally (Hennadii Stepanov)

Pull request description:

  On the master branch, when compiling without external signer support, the `bench_bitcoin.exe` does not initialize Winsock DLL that is required, for example, here: https://github.com/bitcoin/bitcoin/blob/459272d639b9547f68000d2b9a5a0d991d477de5/src/bench/addrman.cpp#L124

  Moreover, Windows docs explicitly [state](https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-wsacleanup) that `WSAStartup` and `WSACleanup` must be balanced:
  > There must be a call to `WSACleanup` for each successful call to `WSAStartup`. Only the final `WSACleanup` function call performs the actual cleanup. The preceding calls simply decrement an internal reference count in the WS2_32.DLL.

  That is not the case for our unit tests because the `SetupNetworking()` call is a part of the `BasicTestingSetup` fixture and is invoked multiple times, while `~CNetCleanup()` is invoked once only, at the end of the test binary execution.

  This PR fixes Winsock DLL initialization and termination.

  More docs:
  - https://learn.microsoft.com/en-us/windows/win32/winsock/initializing-winsock
  - https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-wsastartup
  - https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-wsacleanup

  Fix bitcoin#28940.

ACKs for top commit:
  maflcko:
    lgtm ACK fd4c6a1

Tree-SHA512: d360eaf776943f7f7a35ed5a5f9f3228d9e3d18eb824e5997cdc8eadddf466abe9f2da4910ee3bb86bf5411061e758259f7e1ec344f234ef7996f1bf8781dcda

src/common/system.h

@@ -17,7 +17,7 @@
 int64_t GetStartupTime();
 
 void SetupEnvironment();
-bool SetupNetworking();
+[[nodiscard]] bool SetupNetworking();
 #ifndef WIN32
 std::string ShellEscape(const std::string& arg);
 #endif

src/test/util/setup_common.cpp

@@ -49,6 +49,7 @@
 #include <txdb.h>
 #include <txmempool.h>
 #include <util/chaintype.h>
+#include <util/check.h>
 #include <util/rbf.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -88,6 +89,15 @@ std::ostream& operator<<(std::ostream& os, const uint256& num)
     return os;
 }
 
+struct NetworkSetup
+{
+    NetworkSetup()
+    {
+        Assert(SetupNetworking());
+    }
+};
+static NetworkSetup g_networksetup_instance;
+
 BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vector<const char*>& extra_args)
     : m_path_root{fs::temp_directory_path() / "test_common_" PACKAGE_NAME / g_insecure_rand_ctx_temp_path.rand256().ToString()},
       m_args{}
@@ -130,7 +140,6 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
     LogInstance().StartLogging();
     m_node.kernel = std::make_unique<kernel::Context>();
     SetupEnvironment();
-    SetupNetworking();
 
     ValidationCacheSizes validation_cache_sizes{};
     ApplyArgsManOptions(*m_node.args, validation_cache_sizes);

src/test/util_tests.cpp

@@ -1218,6 +1218,9 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory)
     // has released the lock as we would expect by probing it.
     int processstatus;
     BOOST_CHECK_EQUAL(write(fd[1], &LockCommand, 1), 1);
+    // The following line invokes the ~CNetCleanup dtor without
+    // a paired SetupNetworking call. This is acceptable as long as
+    // ~CNetCleanup is a no-op for non-Windows platforms.
     BOOST_CHECK_EQUAL(write(fd[1], &ExitCommand, 1), 1);
     BOOST_CHECK_EQUAL(waitpid(pid, &processstatus, 0), pid);
     BOOST_CHECK_EQUAL(processstatus, 0);