`AddrManSelectByNetwork` benchmark fails on Windows

hebasto commented at 8:48 pm on November 25, 2023: member

When compiling without external signer support, the resulted bench_bitcoin.exe fails on Windows:

0>.\bench_bitcoin.exe -filter=AddrManSelectByNetwork
1Error: bad optional access

Steps to reproduce the failure:

cross compiling on Ubuntu 22.04:

0$ make -j $(nproc) -C depends HOST=x86_64-w64-mingw32 NO_QT=1
1$ ./autogen.sh
2$ ./configure CONFIG_SITE=$PWD/depends/x86_64-w64-mingw32/share/config.site --disable-external-signer
3$ make -j $(nproc)

compiling natively on Windows:

Apply the diff as follows:

 0--- a/build_msvc/bitcoin_config.h.in
 1+++ b/build_msvc/bitcoin_config.h.in
 2@@ -41,9 +41,6 @@
 3 /* Define this symbol to enable ZMQ functions */
 4 #define ENABLE_ZMQ 1
 5 
 6-/* define if external signer support is enabled (requires Boost::Process) */
 7-#define ENABLE_EXTERNAL_SIGNER /**/
 8-
 9 /* Define to 1 if you have the declaration of `be16toh', and to 0 if you
10    don't. */
11 #define HAVE_DECL_BE16TOH 0

Then

0>py -3 build_msvc\msvc-autogen.py
1>msbuild build_msvc\bitcoin.sln -property:Configuration=Release -verbosity:minimal
2>src\bench_bitcoin.exe -filter=AddrManSelectByNetwork
3Error: bad optional access

The failure does not manifest itself when running bench_bitcoin.exe using Wine on Linux.

hebasto added the label Windows on Nov 25, 2023

hebasto added the label Tests on Nov 25, 2023

theuni commented at 4:57 pm on November 27, 2023: member

Yep, this makes sense to me.

See boost asio: https://github.com/boostorg/asio/blob/develop/include/boost/asio/detail/impl/winsock_init.ipp#L39

And a static instance of that is created here: https://github.com/boostorg/asio/blob/develop/include/boost/asio/detail/winsock_init.hpp#L116

And I see those being included by<boost/process.hpp> from run_command.cpp. So I’m guessing boost has been quietly initializing our network stack :(

Edit:

0$ objdump -tC common/libbitcoin_common_a-run_command.o | grep WSA
1[1059](sec  0)(fl 0x00)(ty    0)(scl   2) (nx 0) 0x0000000000000000 __imp_WSACleanup
2[1060](sec  0)(fl 0x00)(ty    0)(scl   2) (nx 0) 0x0000000000000000 __imp_WSAStartup

fanquake commented at 5:03 pm on November 27, 2023: member

YUCK.

fanquake commented at 10:00 am on November 28, 2023: member

I think the only logical thing to do here is to disable external signer for windows, and fixup our code. Looking upstream at Boost Process, maintainence cetainly hasn’t improved, and it looks like it’s only leaning further into using Boost ASIO.

fanquake referenced this in commit 798b2bd492 on Nov 29, 2023

fanquake referenced this in commit 3d415322b2 on Nov 29, 2023

fanquake referenced this in commit 02895a89f6 on Nov 29, 2023

fanquake closed this on Nov 29, 2023

fanquake referenced this in commit 9ff589c60c on Nov 29, 2023

AddrManSelectByNetwork benchmark fails on Windows #28940

`AddrManSelectByNetwork` benchmark fails on Windows #28940