Add windows support - #231
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline and AI policy for information on the review process.
If your review is incorrectly listed, please copy-paste ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. LLM Linter (✨ experimental)Possible typos and grammar issues:
2026-08-13 21:14:11 |
|
We have miners using Stratum V2 that are on Windows and would like to see Windows IPC support landing, is there any estimate when they can expect this to materialize? We have a dozen of users who can help with testing if that's a blocker. |
|
@pavlenex testing would definitely be useful. I guess for that to work we'd need a stack of pull requests: this one here in libmultiprocess, one in Bitcoin Core that enables IPC support in the Windows Guix build, and then an SRI pull request that uses it. Testers would then have download (or build themselves) the custom bitcoin core and SRI binaries. I've added Windows support to my v32 wish list: bitcoin/bitcoin#33777 No guarantees obviously. |
|
I'll try to get this PR ready for review this week, split up into smaller commits and with ci passing. From there as Sjors mentioned there is a lot more work to do: more code changes in bitcoin/bitcoin#32387 that need to be made in bitcoin core, enabling IPC into windows builds in bitcoin core, enabling it in windows CI jobs with pycapnp, adding client support, probably adding a windows CI job to this repository. v32 sounds like a good target though and it is very useful to know there is demand for this feature, because it hasn't been a priority so far |
|
PR is split up into commits now and should be reviewable. CI is not passing but failures look like IWYU errors. I also opened bitcoin/bitcoin#35084 with corresponding bitcoin core changes. Windows support for bitcoin core can be tested with bitcoin/bitcoin#32387 which combines both PRs and enables IPC by default in windows builds. Rebased 2975fac -> cb16d2e ( Updated cb16d2e -> e563c96 ( Updated e563c96 -> d9fcac6 ( Added 1 commits d9fcac6 -> a1748e2 ( Updated a1748e2 -> 18fc188 ( Updated 18fc188 -> 7fd5ec4 ( |
e563c96 to
d9fcac6
Compare
|
How realistic is it to add a Windows CI job here? (can be cross-compiled) |
The lifecycle test repeatedly constructs and destroys TPTester to exercise IPC EventLoop teardown. On Windows it hangs intermittently in std::thread::join during teardown of libmultiprocess thread-local state. This is a known libmultiprocess Windows issue, not specific to sv2-tp: bitcoin-core/libmultiprocess#231 bitcoin/bitcoin#32387 The fix lives upstream and rewrites the EventLoop wakeup primitive (raw fd -> KJ stream) and adds shutdownWrite() in ~Connection. Until that lands and is backported into our libmultiprocess subtree, gate this particular test off on _WIN32. The other sv2 unit tests run normally on Windows; only the explicit teardown loop is affected. Assisted-by: GitHub Copilot Assisted-by: Anthropic Claude Opus 4
The template_provider_tests `client_tests`, `fee_timer_blocking_test` and `new_tip_bypasses_fee_timer_test` cases each construct and tear down a TPTester. Like the lifecycle test skipped in the previous commit, that teardown deadlocks intermittently on Windows in std::thread::join during libmultiprocess thread-local state cleanup. This is a known libmultiprocess Windows issue, not specific to sv2-tp: bitcoin-core/libmultiprocess#231 bitcoin/bitcoin#32387 Gate the three TPTester-using cases off on _WIN32 with a TODO. The non-IPC test `block_reserved_weight_floor` continues to run on Windows. Assisted-by: GitHub Copilot Assisted-by: Anthropic Claude Opus 4
…g to named pipe SpawnProcess on Windows creates a named pipe with PIPE_WAIT and then calls ConnectNamedPipe synchronously. If the child exits or crashes before connecting, ConnectNamedPipe blocks forever with no recovery path. Fix by opening the pipe with FILE_FLAG_OVERLAPPED and using WaitForMultipleObjects on both the connect event and the child process handle. If the process handle signals first, the child died without connecting and SpawnProcess throws instead of hanging. Since the pipe is now in overlapped mode, WriteFile also requires an OVERLAPPED structure; use GetOverlappedResult with bWait=TRUE to handle both synchronous and asynchronous completion. Add a Windows-only test that spawns a child which exits immediately without opening the named pipe and asserts SpawnProcess does not block. (bitcoin-core/libmultiprocess#231 (comment)) Co-Authored-By: ViniciusCestarii <viniciuscestari01@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…g to named pipe SpawnProcess on Windows creates a named pipe with PIPE_WAIT and then calls ConnectNamedPipe synchronously. If the child exits or crashes before connecting, ConnectNamedPipe blocks forever with no recovery path. Fix by opening the pipe with FILE_FLAG_OVERLAPPED and using WaitForMultipleObjects on both the connect event and the child process handle. If the process handle signals first, the child died without connecting and SpawnProcess throws instead of hanging. Since the pipe is now in overlapped mode, WriteFile also requires an OVERLAPPED structure; use GetOverlappedResult with bWait=TRUE to handle both synchronous and asynchronous completion. Add a Windows-only test that spawns a child which exits immediately without opening the named pipe and asserts SpawnProcess does not block. (bitcoin-core/libmultiprocess#231 (comment)) Co-Authored-By: ViniciusCestarii <viniciuscestari01@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…g to named pipe SpawnProcess on Windows creates a named pipe with PIPE_WAIT and then calls ConnectNamedPipe synchronously. If the child exits or crashes before connecting, ConnectNamedPipe blocks forever with no recovery path. Fix by opening the pipe with FILE_FLAG_OVERLAPPED and using WaitForMultipleObjects on both the connect event and the child process handle. If the process handle signals first, the child died without connecting and SpawnProcess throws instead of hanging. Since the pipe is now in overlapped mode, WriteFile also requires an OVERLAPPED structure; use GetOverlappedResult with bWait=TRUE to handle both synchronous and asynchronous completion. Add a Windows-only test that spawns a child which exits immediately without opening the named pipe and asserts SpawnProcess does not block. (bitcoin-core/libmultiprocess#231 (comment)) Co-Authored-By: ViniciusCestarii <viniciuscestari01@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove recently introduced SpawnConnectInfo and SpawnConnectInfoToArgsFn type aliases since they are the same on all platforms and might obscure the fact that connect info should be treated as an opaque string. Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the Unix-only UnixListener test helper with a cross-platform SocketListener backed by a std::variant<sockaddr_un> (TCP arm will be added later for Wine compatibility). - test/socketlistener.h: New header with SocketListener class extracted from the pattern of UnixListener but using SocketId/SocketError types, std::filesystem for temp dir cleanup, and mp::CloseSocket for teardown. Uses std::variant<sockaddr_un> so the TCP arm can be appended later without restructuring the class. - test/unixlistener.h: Deleted. - test/listen_tests.cpp: Switch to socketlistener.h and socketlistener. - test/connect_tests.cpp: Switch to socketlistener.h and socketlistener. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move POSIX-only headers in util.cpp into a #ifndef WIN32 block so the file compiles on Windows without modification. Guard the Unix-only helpers (MakeArgv, MaxFd, ChildFail, SpawnError*, ReadSpawnResult, WriteSpawnError, KillAndReapChild) with a single #ifndef WIN32 block. Guard the extern "C" environ declaration likewise. Guard Unix-only includes in spawn_tests.cpp and connect_tests.cpp and wrap the Unix-only spawn tests with #ifndef WIN32. No Windows implementations are added here; those come in a later commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
Add Windows-specific code to support building and running on Windows: - util.h: Guard ProcessId/SocketId/SocketError type aliases with WIN32 ifdefs so they use SOCKET/uintptr_t on Windows and int on Unix. Add winsock2.h include on Windows. - util.cpp: Guard Unix-specific system headers with WIN32 ifdefs. Add Windows-specific includes (windows.h, winsock2.h). Guard MaxFd() with #ifndef WIN32. Add GetCurrentThreadId() branch in ThreadName(). Add win32Socketpair() forward-declare. Add Windows branch in SocketPair() using win32Socketpair(). Add CommandLineFromArgv() helper needed to construct CreateProcess command lines. Add Windows branch in SpawnProcess() using named pipes and WSADuplicateSocket to pass socket to child. Add Windows branch in StartSpawned() reading socket from named pipe. Add Windows branch in WaitProcess() using WaitForSingleObject/GetExitCodeProcess. - proxy.cpp: Add SocketOutputStream class on Windows (analogous to FdOutputStream but using SOCKET/send()). Add Windows branch in EventLoop constructor to create m_post_writer using SocketOutputStream. Co-Authored-By: ViniciusCestarii <viniciuscestari01@gmail.com>
…g to named pipe SpawnProcess on Windows creates a named pipe with PIPE_WAIT and then calls ConnectNamedPipe synchronously. If the child exits or crashes before connecting, ConnectNamedPipe blocks forever with no recovery path. Fix by opening the pipe with FILE_FLAG_OVERLAPPED and using WaitForMultipleObjects on both the connect event and the child process handle. If the process handle signals first, the child died without connecting and SpawnProcess throws instead of hanging. Since the pipe is now in overlapped mode, WriteFile also requires an OVERLAPPED structure; use GetOverlappedResult with bWait=TRUE to handle both synchronous and asynchronous completion. Add a Windows-only test that spawns a child which exits immediately without opening the named pipe and asserts SpawnProcess does not block. (bitcoin-core#231 (comment)) Co-Authored-By: ViniciusCestarii <viniciuscestari01@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Allow code to compile without pthreads available, as required for MSVC compatibility. Avoid unconditional POSIX calls (fork, posix_spawn, pthread_getname_np) by moving them into #ifndef WIN32 or HAVE_PTHREAD_* guards. When pthreads is available on Windows (detected via cmake HAVE_PTHREAD_* checks), still use it for thread name reporting since it provides useful information at low cost. Also add Threads::Threads as an explicit dependency of the multiprocess library. proxy.cpp directly uses thread_local, std::this_thread, and std::thread, and the dependency was previously satisfied only through transitive linkage from CapnProto::kj-async. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add TCP (sockaddr_in) support to SocketListener and platform-conditional header guards so the class compiles and works on Windows: - Wrap POSIX headers (arpa/inet.h, netinet/in.h, sys/socket.h, sys/un.h) in #else of #ifdef WIN32, with Windows equivalents (afunix.h, ws2tcpip.h) in the #ifdef WIN32 branch. - Add a WsaInit static initializer on Windows that calls WSAStartup before any socket operations. Tests create sockets directly (not via StartSpawned), so Winsock must be initialized here. - Add Init(sockaddr_in&) and Connect(const sockaddr_in&) overloads for TCP loopback connections. - Use TCP on Windows to work around Wine's lack of the AcceptEx extension required by KJ's AF_UNIX listener; Unix platforms continue using AF_UNIX. - Expand std::variant<sockaddr_un> to std::variant<sockaddr_in, sockaddr_un>. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- shell.nix: add `windows` parameter that selects pkgs.pkgsCross.mingwW64
as the cross target; also change crossPkgs default from import<nixpkgs>{}
to null (cleaner API). When windows=true, add native pkgs.capnproto to
nativeBuildInputs so capnp/capnpc-c++ are in PATH for cmake code generation,
and add wine64Packages.staging so ctest can run mptest.exe via wine.
Change llvmBase to always use pkgs (native) instead of crossPkgs.
- ci/configs/windows.bash: new config that cross-compiles with mingw,
sets CMAKE_SYSTEM_NAME=Windows, CMAKE_FIND_ROOT_PATH_MODE_PROGRAM=NEVER
(so cmake finds native capnp from PATH), CMAKE_CROSSCOMPILING_EMULATOR=wine
(so ctest runs mptest.exe via wine), and sets MPGEN_PRE_BUILD=1.
- ci/scripts/ci.sh: add MPGEN_PRE_BUILD support: when set, build native mpgen
in $CI_DIR-native before the main cross build, then inject
-DMPGEN_EXECUTABLE into CMAKE_ARGS. This is needed because cmake's
add_custom_command does not use CMAKE_CROSSCOMPILING_EMULATOR, so the
cross-compiled mpgen.exe cannot be used as a code generator directly.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…g to named pipe SpawnProcess on Windows creates a named pipe with PIPE_WAIT and then calls ConnectNamedPipe synchronously. If the child exits or crashes before connecting, ConnectNamedPipe blocks forever with no recovery path. Fix by opening the pipe with FILE_FLAG_OVERLAPPED and using WaitForMultipleObjects on both the connect event and the child process handle. If the process handle signals first, the child died without connecting and SpawnProcess throws instead of hanging. Since the pipe is now in overlapped mode, WriteFile also requires an OVERLAPPED structure; use GetOverlappedResult with bWait=TRUE to handle both synchronous and asynchronous completion. Add a Windows-only test that spawns a child which exits immediately without opening the named pipe and asserts SpawnProcess does not block. (bitcoin-core#231 (comment)) Co-Authored-By: ViniciusCestarii <viniciuscestari01@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…g to named pipe SpawnProcess on Windows creates a named pipe with PIPE_WAIT and then calls ConnectNamedPipe synchronously. If the child exits or crashes before connecting, ConnectNamedPipe blocks forever with no recovery path. Fix by opening the pipe with FILE_FLAG_OVERLAPPED and using WaitForMultipleObjects on both the connect event and the child process handle. If the process handle signals first, the child died without connecting and SpawnProcess throws instead of hanging. Since the pipe is now in overlapped mode, WriteFile also requires an OVERLAPPED structure; use GetOverlappedResult with bWait=TRUE to handle both synchronous and asynchronous completion. Add a Windows-only test that spawns a child which exits immediately without opening the named pipe and asserts SpawnProcess does not block. (bitcoin-core/libmultiprocess#231 (comment)) Co-Authored-By: ViniciusCestarii <viniciuscestari01@gmail.com> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
d3d74e701f761f21d276959c15fb91eaca3c5607 ipc, refactor: Update mp::g_thread_context references (Ryan Ofsky) 2d3f72fd3fa45ab4e399094c39bfa93bb5a87323 ipc, refactor: Update mp::SpawnProcess call (Ryan Ofsky) e9f19815caa3e129020f116bef43d409cb50a475 ipc, refactor: Add Stream type alias and use it (Ryan Ofsky) 3859805f05e6fb642140c89e6d7378203e04df2c ipc, refactor: Add SocketId type alias and use it (Ryan Ofsky) 2ee9b69c7a1f27f4e5dbe94a55fb62cd636d205a ipc, refactor: Add ProcessId type alias and use it (Ryan Ofsky) 344979714189bb7cab20f77dd23256c08e235b84 ipc: Avoid 'unistd.h' error with MSVC (Ryan Ofsky) dbcc192dce6147f1397baa72d5dc98fde9528638 ipc, refactor: fix include order (Ryan Ofsky) 7c86d4834ed2767049b7c9a62456ad45dcbb572b ipc, refactor: use native path separators in test (Ryan Ofsky) 00287b9a34047a7ae75aa128d3b0f05feab06098 ipc, refactor: Change Protocol class field order (Ryan Ofsky) 33d37f3c35efaac136863253b91799bf2711fd46 ipc, refactor: Drop connect/listen/serve exe_name parameters (Ryan Ofsky) 794940469e77b82462db33011fa4d5e8f533e543 ipc, moveonly: combine ipc_test.cpp and ipc_tests.cpp (Ryan Ofsky) Pull request description: This PR makes Bitcoin Core changes needed to be compatible with bitcoin-core/libmultiprocess#274, which changes the libmultiprocess API to stop using unix-specific types so it is compatible with windows. (Windows support is added in followups: bitcoin-core/libmultiprocess#231 and bitcoin/bitcoin#32387.) The PR uses some [compatibility shims](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-wins/src/ipc/util.h) so it can be reviewed and merged without needing to merge bitcoin-core/libmultiprocess#274 first and bump the libmultiprocess subtree. These can be deleted when the subtree is updated. --- Review note: All the changes here are refactoring, and you don't really need to know anything about IPC or Windows to review this code. It is also a mostly move-only change (131 lines added, 96 removed, 215 moved) ACKs for top commit: xyzconstant: tACK d3d74e701f761f21d276959c15fb91eaca3c5607 enirox001: ACK d3d74e701f Sjors: ACK d3d74e701f761f21d276959c15fb91eaca3c5607 ViniciusCestarii: re-ACK d3d74e701f761f21d276959c15fb91eaca3c5607 tested locally on Linux Tree-SHA512: cd48708f9fd086ac8127dc75cfaf4bd8f8da81e07d11b2c9e65fd9061ffa33478bffc6fd6fa4b3505e86c6437752578fe6e5bd590c683c3bc9969093103a5608
Tearing down a TPTester (and with it the IPC EventLoop / per-thread state) intermittently deadlocks on Windows in std::thread::join during libmultiprocess thread-local cleanup. The fix lives upstream and rewrites the EventLoop wakeup primitive (raw fd -> KJ stream) and adds shutdownWrite() in ~Connection. Until that lands and is backported into our libmultiprocess subtree, paper over it in the test harness so the Windows CI job stays useful: - TPTesterHandle now heap-allocates and intentionally leaks the tester on Windows; the OS reclaims the remaining loop thread and IPC state at process exit. This only disables test cleanup, not the tests themselves. Non-Windows builds continue to own the tester by value. - sv2_tester_lifecycle_tests is gated off on _WIN32, since its whole purpose is to exercise repeated TPTester construction and destruction; leaking would defeat the test. - src/test/main.cpp installs a Windows-only Boost global fixture whose destructor runs at module teardown (after every test case has completed and Boost has tallied results). It flushes stdout/stderr and calls _exit() with 0 or 1 based on the Boost results, bypassing static destructors entirely so the leaked threads cannot fault and turn a green run into exit code 139. - lint-includes.py: allowlist boost/test/results_collector.hpp, required by the new fixture. References: bitcoin-core/libmultiprocess#231 bitcoin/bitcoin#32387 Assisted-by: GitHub Copilot Assisted-by: Anthropic Claude Opus 4 Assisted-by: Anthropic Claude Opus 4.7
Tearing down a TPTester (and with it the IPC EventLoop / per-thread state) intermittently deadlocks on Windows in std::thread::join during libmultiprocess thread-local cleanup. The fix lives upstream and rewrites the EventLoop wakeup primitive (raw fd -> KJ stream) and adds shutdownWrite() in ~Connection. Until that lands and is backported into our libmultiprocess subtree, paper over it in the test harness so the Windows CI job stays useful: - TPTesterHandle now heap-allocates and intentionally leaks the tester on Windows; the OS reclaims the remaining loop thread and IPC state at process exit. This only disables test cleanup, not the tests themselves. Non-Windows builds continue to own the tester by value. - sv2_tester_lifecycle_tests is gated off on _WIN32, since its whole purpose is to exercise repeated TPTester construction and destruction; leaking would defeat the test. - src/test/main.cpp installs a Windows-only Boost global fixture whose destructor runs at module teardown (after every test case has completed and Boost has tallied results). It flushes stdout/stderr and calls _exit() with 0 or 1 based on the Boost results, bypassing static destructors entirely so the leaked threads cannot fault and turn a green run into exit code 139. - lint-includes.py: allowlist boost/test/results_collector.hpp, required by the new fixture. References: bitcoin-core/libmultiprocess#231 bitcoin/bitcoin#32387 Assisted-by: GitHub Copilot Assisted-by: Anthropic Claude Opus 4 Assisted-by: Anthropic Claude Opus 4.7
Tearing down a TPTester (and with it the IPC EventLoop / per-thread state) intermittently deadlocks on Windows in std::thread::join during libmultiprocess thread-local cleanup. The fix lives upstream and rewrites the EventLoop wakeup primitive (raw fd -> KJ stream) and adds shutdownWrite() in ~Connection. Until that lands and is backported into our libmultiprocess subtree, paper over it in the test harness so the Windows CI job stays useful: - TPTesterHandle now heap-allocates and intentionally leaks the tester on Windows; the OS reclaims the remaining loop thread and IPC state at process exit. This only disables test cleanup, not the tests themselves. Non-Windows builds continue to own the tester by value. - sv2_tester_lifecycle_tests is gated off on _WIN32, since its whole purpose is to exercise repeated TPTester construction and destruction; leaking would defeat the test. - src/test/main.cpp installs a Windows-only Boost global fixture whose destructor runs at module teardown (after every test case has completed and Boost has tallied results). It flushes stdout/stderr and calls _exit() with 0 or 1 based on the Boost results, bypassing static destructors entirely so the leaked threads cannot fault and turn a green run into exit code 139. - lint-includes.py: allowlist boost/test/results_collector.hpp, required by the new fixture. References: bitcoin-core/libmultiprocess#231 bitcoin/bitcoin#32387 Assisted-by: GitHub Copilot Assisted-by: Anthropic Claude Opus 4 Assisted-by: Anthropic Claude Opus 4.7
Tearing down a TPTester (and with it the IPC EventLoop / per-thread state) intermittently deadlocks on Windows in std::thread::join during libmultiprocess thread-local cleanup. The fix lives upstream and rewrites the EventLoop wakeup primitive (raw fd -> KJ stream) and adds shutdownWrite() in ~Connection. Until that lands and is backported into our libmultiprocess subtree, paper over it in the test harness so the Windows CI job stays useful: - TPTesterHandle now heap-allocates and intentionally leaks the tester on Windows; the OS reclaims the remaining loop thread and IPC state at process exit. This only disables test cleanup, not the tests themselves. Non-Windows builds continue to own the tester by value. - sv2_tester_lifecycle_tests is gated off on _WIN32, since its whole purpose is to exercise repeated TPTester construction and destruction; leaking would defeat the test. - src/test/main.cpp installs a Windows-only Boost global fixture whose destructor runs at module teardown (after every test case has completed and Boost has tallied results). It flushes stdout/stderr and calls _exit() with 0 or 1 based on the Boost results, bypassing static destructors entirely so the leaked threads cannot fault and turn a green run into exit code 139. - lint-includes.py: allowlist boost/test/results_collector.hpp, required by the new fixture. References: bitcoin-core/libmultiprocess#231 bitcoin/bitcoin#32387 Assisted-by: GitHub Copilot Assisted-by: Anthropic Claude Opus 4 Assisted-by: Anthropic Claude Opus 4.7
Add support for running on windows. These changes make the libmultiprocess API more generic, using stream types instead of file descriptors. All features are supported, including spawning processes with socket connections to the parent process. These changes were originally made in bitcoin/bitcoin#32387