Merge bitcoin/bitcoin#28486: test, bench: Initialize and terminate use of Winsock properly

fd4c6a10f2 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: 459272d639/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 https://github.com/bitcoin/bitcoin/issues/28940.

ACKs for top commit:
  maflcko:
    lgtm ACK fd4c6a10f2

Tree-SHA512: d360eaf776943f7f7a35ed5a5f9f3228d9e3d18eb824e5997cdc8eadddf466abe9f2da4910ee3bb86bf5411061e758259f7e1ec344f234ef7996f1bf8781dcda
This commit is contained in:
fanquake 2023-11-29 17:10:28 +00:00
commit dd73c22976
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
3 changed files with 14 additions and 2 deletions

View file

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

View file

@ -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);

View file

@ -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);