Skip to content

Commit a59de7f

Browse files
fanquakevijaydasmp
authored andcommitted
Merge bitcoin#28486: test, bench: Initialize and terminate use of Winsock 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
1 parent d5d2d44 commit a59de7f

File tree

2 files changed

+13
-1
lines changed

2 files changed

+13
-1
lines changed

src/test/util/setup_common.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include <test/util/net.h>
4141
#include <test/util/txmempool.h>
4242
#include <txdb.h>
43+
#include <util/check.h>
4344
#include <util/strencodings.h>
4445
#include <util/string.h>
4546
#include <util/thread.h>
@@ -156,6 +157,15 @@ void DashChainstateSetupClose(NodeContext& node)
156157
Assert(node.mempool.get()));
157158
}
158159

160+
struct NetworkSetup
161+
{
162+
NetworkSetup()
163+
{
164+
Assert(SetupNetworking());
165+
}
166+
};
167+
static NetworkSetup g_networksetup_instance;
168+
159169
BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::vector<const char*>& extra_args)
160170
: m_path_root{fs::temp_directory_path() / "test_common_" PACKAGE_NAME / g_insecure_rand_ctx_temp_path.rand256().ToString()},
161171
m_args{}
@@ -201,7 +211,6 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
201211
ECC_Start();
202212
BLSInit();
203213
SetupEnvironment();
204-
SetupNetworking();
205214
InitSignatureCache();
206215
InitScriptExecutionCache();
207216

src/test/util_tests.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1232,6 +1232,9 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory)
12321232
// has released the lock as we would expect by probing it.
12331233
int processstatus;
12341234
BOOST_CHECK_EQUAL(write(fd[1], &LockCommand, 1), 1);
1235+
// The following line invokes the ~CNetCleanup dtor without
1236+
// a paired SetupNetworking call. This is acceptable as long as
1237+
// ~CNetCleanup is a no-op for non-Windows platforms.
12351238
BOOST_CHECK_EQUAL(write(fd[1], &ExitCommand, 1), 1);
12361239
BOOST_CHECK_EQUAL(waitpid(pid, &processstatus, 0), pid);
12371240
BOOST_CHECK_EQUAL(processstatus, 0);

0 commit comments

Comments
 (0)