ci: add Windows job by Sjors · Pull Request #272 · bitcoin-core/libmultiprocess

Sjors · 2026-04-20T14:17:46Z

Based on #231

CI commits are entirely vibe coded, so will need some polishing if it actually works,

Add ProcessId = int type alias and apply it to WaitProcess, SpawnProcess (pid output argument), and callers.

Add SocketId = int and SocketError = -1 type aliases and apply SocketId to SpawnProcess (return type and callback parameter) and callers.

Add ConnectInfo type alias to pass socket handle from parent process to child process in more platform independent way.

gen.cpp used fork() directly via <unistd.h> to invoke the capnp compiler as a subprocess, but fork() is not available on Windows, so shouldn't be used in application code. Add an ExecProcess(const std::vector<std::string>& args) function to util.h/util.cpp that spawns a process and returns its ProcessId, leaving the caller responsible for WaitProcess. On POSIX it uses fork() (via KJ_SYSCALL) + execvp; on Windows it can use CreateProcess. Update gen.cpp to replace the inline fork/exec/wait with mp::WaitProcess(mp::ExecProcess(args)). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Extract socket pair creation from SpawnProcess into a standalone SocketPair() function, and use it to replace the inline socketpair() call. No behavior change.

Explicitly clear FD_CLOEXEC on the child's socket before calling exec, so the fd survives into the spawned process regardless of how the socket was created. Previously this relied on socketpair() not setting FD_CLOEXEC by default, which is not guaranteed if the caller creates sockets with SOCK_CLOEXEC or if the flag gets set by other means.

…objects Replace the m_wait_fd/m_post_fd raw int members with m_wait_stream/m_post_stream kj::Own<kj::AsyncIoStream> and m_post_writer kj::Own<kj::OutputStream>. The constructor uses provider->newTwoWayPipe() instead of calling socketpair() directly. The loop() and post() methods write through m_post_writer instead of calling write() with a raw fd, and EventLoopRef::reset does the same.

kj::AsyncIoStream::getFd() was added in capnproto 0.9 (commit d27bfb8a4175b32b783de68d93dd1dbafadddea5, first released in 0.9.0). The code now uses getFd() in proxy.cpp, so 0.7 is no longer a sufficient minimum. Set olddeps version to 0.9.2, which is the patched 0.9.x release for CVE-2022-46149. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…m objects Instead of accepting raw file descriptor integers and wrapping them internally, ConnectStream and ServeStream now accept kj::Own<kj::AsyncIoStream> directly. This removes the assumption that the transport is always a local unix fd, making the API easier to adapt to other I/O types (e.g. Windows handles). The Stream type alias (kj::Own<kj::AsyncIoStream>) is added as a convenience, along with StreamSocketId() to extract the underlying fd from a Stream when needed. Callers are updated to wrap their fd with wrapSocketFd() before calling.

Flush pending Cap'n Proto release messages before closing the stream. When one side of a socket pair closes, the other side does not receive an onDisconnect event, so it relies on receiving release messages from the closing side to free its ProxyServer objects and shut down cleanly. Without this, Server objects are not freed by Cap'n Proto on disconnection.

MSVC error when building multiprocess.vcxproj: mp/util.h(146,46): error C2280: 'std::variant<T *,T>::variant(const std::variant<T *,T> &)': attempting to reference a deleted function [with T=mp::Lock] The PtrOrValue constructor used a ternary expression to initialize data: data(ptr ? ptr : std::variant<T*, T>{std::in_place_type<T>, args...}) Both arms are prvalues of type std::variant<T*,T>, so under C++17's mandatory copy elision no copy/move constructor should be invoked. GCC and Clang apply this correctly. MSVC does not apply guaranteed copy elision to ternary expressions in this context: it materializes the temporary and then attempts to copy-construct data from it. Since std::variant<Lock*,Lock> has a deleted copy constructor (Lock holds a std::unique_lock which is move-only), MSVC fails. Fix by initializing data to hold T*=ptr in the member initializer list, then emplacing T in-place in the constructor body if ptr is null. This avoids the ternary entirely and requires only the in-place constructor of T, not any variant copy or move. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

DrahtBot · 2026-04-20T14:18:17Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#269 (proxy: add local connection limit to ListenConnections by enirox001)
#218 (Better error and log messages by ryanofsky)
#212 (ci: add newdeps job testing newer versions of cmake and capnproto by ryanofsky)
#209 (cmake: Increase cmake policy version by ryanofsky)
#175 (Set cmake_minimum_required(VERSION 3.22) by maflcko)

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:

nonunix -> non-Unix [standard term is hyphenated; nonunix reads as a spelling error]

^{2026-04-23 13:40:15}

Sjors · 2026-04-20T14:25:07Z

This will need whitelisting of msys2/setup-msys2.

But I'm thinking of switching to cross-compilation, since that's how most users will use this library anyway.

Sjors · 2026-04-20T14:54:00Z

Ok, similar to Bitcoin Core we now cross-compile and then run the result on a Windows machine.

The agent made a few other adjustments to the code that I haven't looked at yet.

This needs the following whitelist:

actions/upload-artifact
actions/download-artifact

Sjors · 2026-04-21T16:39:52Z

@ryanofsky can you whitelist the upload / download actions I mentioned above?

ryanofsky · 2026-04-21T17:11:12Z

I think we will need some CI coverage for windows, and this approach looks ok, but it's also compilicated.

I wonder if it would be possible to implement a more minimal approach using nix's mingw compiler and wine (nativeBuildInputs = [ pkgs.pkgsCross.mingwW64.buildPackages.gcc pkgs.wine ];) to just build the tests and examples normally and run them with wine.

This would seem helpful for local development and debugging. If CI job fails just run it locally with CI_CONFIG=ci/configs/windows.bash ci/scripts/run.sh or similar.

Sjors · 2026-04-21T17:29:24Z

I'll first try to get the current attempt to run and then will look at some different approaches. Building with nix makes sense. Running in wine could also make sense, though I suspect a native run will catch other things, so maybe we should do both.

Sjors · 2026-04-21T19:01:31Z

I switched cross-compilation to nix. The (cold cache) build takes very long on CI, so I'll check again tomorrow and then see if I can clean things up.

Sjors · 2026-04-22T08:11:12Z

Alright, CI green!

@ryanofsky I'll wait for you to get #231 green again before rebasing and then I'll look into cleanup. You can cherry-pick things out of here if you think they're correct, but otherwise feel free to ignore. I need to study that PR first to better understand what does and doesn't make sense here.

ryanofsky · 2026-04-22T15:20:28Z

Thanks! I looked more at the nix/wine changes here than the native windows stuff but it looks like everything makes sense. My plan is to split #231 up and open a new PR that only makes non-windows changes. Then #231 can depend on that and this can depend on #231.

Alternately, this PR could be implemented independently of the other PRs by just having the windows &wine jobs build libmultiprocess and run mptest, adding a dummy test that can run on windows like

KJ_TEST("Hello world")
{
    KJ_EXPECT(true);
}

and skipping the other tests that don't compile on windows yet. But this is useful for testing in its current form too. Just suggesting in case you are impatient to get this merged.

MSVC warns (C4305, treated as error) about truncation from 'int' to 'const bool' when initializing static const bool members from integer bitwise-and expressions. Use constexpr bool with explicit != 0 to make the boolean conversion unambiguous. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ldField on MSVC MSVC cannot parse 'typename decltype(expr)::Member' syntax and fails with a hard error (C2039, C2146) instead of a SFINAE substitution failure. Use Decay<> wrapper to provide the extra template indirection that MSVC needs, consistent with the unique_ptr and shared_ptr overloads of CustomBuildField which already use this pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

This repo has introduced API changes to add Windows support to libmultiprocess (HANDLE-based IPC alongside the existing fd-based IPC). These changes require corresponding updates to Bitcoin Core, which are pending in bitcoin/bitcoin#35084. Until that PR merges, the Bitcoin Core CI jobs fail against master because Bitcoin Core has not yet been updated to use the new API. Switch the Bitcoin Core checkout in both jobs to use refs/pull/35084/merge so CI tests against the compatible version. A BITCOIN_CORE_REF env var is introduced at the top of the file; once (and keep the var in place for any future API compatibility cycles). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ibraries On macOS, when libcapnp is built as a dynamic library and Bitcoin Core REDUCE_EXPORT option is used the RTTI typeinfo for kj::Exception has a different address in libcapnp.dylib versus the calling binary. This means catch (const kj::Exception& e) in the calling binary silently fails to match exceptions thrown by capnp, so the DISCONNECTED exception from shutdownWrite() propagates as a fatal uncaught exception instead of being suppressed as intended. This causes the Bitcoin Core macOS native CI job to fail with: Fatal uncaught kj::Exception: kj/async-io-unix.c++:491: disconnected: shutdown(fd, SHUT_WR): Socket is not connected The fix is to use kj::runCatchingExceptions/kj::throwRecoverableException, which use KJ's own thread-level exception interception mechanism rather than C++ RTTI-based matching, and therefore work correctly across dynamic library boundaries. This is the same approach used elsewhere in the codebase (proxy.cpp EventLoop::post, type-context.h server request handler) for the same reason. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

On MSVC, std::terminate() does not print the exception message before calling abort()/fastfail, so exceptions thrown during mpgen execution appear as a bare 0xC0000409 exit code with no diagnostic output. Wrap main() in a try-catch to explicitly print the error to stderr and return 1 instead of crashing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Bitcoin Core linter rejects it: https://github.com/bitcoin/bitcoin/actions/runs/24568789956/job/71835997334?pr=32387

…bols Use target_compile_definitions on mpgen to expose CAPNP_EXECUTABLE, CAPNPC_CXX_EXECUTABLE (via $<TARGET_FILE:...> generator expressions on the CapnProto::capnp_tool and CapnProto::capnpc_cpp imported targets), and CAPNP_INCLUDE_DIRS (from the CAPNP_INCLUDE_DIRS variable set by find_package). gen.cpp uses these directly instead of constructing paths from capnp_PREFIX. Remove capnp_PREFIX from config.h.in as it is no longer needed there. Add compat fallbacks in compat_config.cmake to synthesize the tool imported targets and CAPNP_INCLUDE_DIRS from older variables when using an older CapnProto package. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Ubuntu Noble's libcapnp-dev 1.0.1 cmake config file is installed under /usr/lib/x86_64-linux-gnu/cmake/CapnProto/ but its _IMPORT_PREFIX calculation goes up only 3 directory levels to /usr/lib instead of 4 levels to /usr, so IMPORTED_LOCATION for CapnProto::capnp_tool is set to /usr/lib/bin/capnp (non-existent) rather than /usr/bin/capnp. The previous compat_config.cmake fallback only fired when the target didn't exist at all (NOT TARGET), so it didn't catch this case where the target exists but has a wrong path. Add a validation pass that iterates over both tool targets after they are created (either by the package or by our own fallback). For each target, check whether any IMPORTED_LOCATION (config-specific or generic) resolves to an existing file. If none do, use find_program (with capnp_PREFIX/bin as a hint) to locate the actual binary and override all stored locations on that target. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

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-io.h: Add Windows branch in StreamSocketId() using getWin32Handle(). - 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.

Remove POSIX and pthread calls from util.cpp to avoid relying on MinGW's POSIX compatibility layer. This lets code be compiled with MSVC.

Sjors · 2026-04-23T11:40:44Z

I'm happy to stay based on #231 so there's some useful coverage.

Adds two CI jobs and the supporting nix/shell plumbing to build libmultiprocess for Windows and run mptest: - windows-cross: nix-based mingw-w64 (UCRT) cross-build on Linux, followed by mptest.exe under wine-wow. - windows-native: native MSVC build + ctest on a Windows runner, driven by ci/scripts/windows_native_test.ps1. The cross job pins cap'n proto to v1.4.0 (v1.3.0+ includes the upstream fix moving cidr.c++ into kj-async, so the previously-required local patch is dropped) and uses a matching native capnpc helper (capnprotoNative) so build-time generated headers match the cross library version. A small wine-invalid-function patch is applied to capnp on the cross build only: capnp's DiskHandle::stat() calls GetFileInformationByHandleEx(FileCompressionInfo), which Wine answers with ERROR_INVALID_FUNCTION (its NTSTATUS->DOS mapping for the unsupported info class). capnp's existing fallback only tolerated ERROR_CALL_NOT_IMPLEMENTED -- a guess that has sat unverified in the filesystem-disk-win32 backend since 2017 and that no project appears to have actually exercised under Wine before -- so without the patch every mp::Connection setup that touches a temp file throws. See the patch header for details.

<winsock2.h> transitively pulls in <windows.h> and <commdlg.h>, which #define INTERFACE, interface, and ERROR. These collide with capnp's Kind::INTERFACE enumerator, the gen.cpp parameter named 'interface', and KJ_LOG(ERROR, ...) in proxy.cpp. Undef the macros right after the winsock2.h include in util.h, and rename gen.cpp's 'interface' locals to 'schema' / 'node_interface'.

Sjors · 2026-04-23T13:40:24Z

Rebased. The agent ended up dropping some commits so that's a good sign.

@ryanofsky can you check if 07a7f71 has something useful for #231?

I bumped the capnproto version for Windows builds to 1.4.0. That gets rid of one patch: 67e5833#diff-022254a27cb514a71c153b75204b316472ba0a5bdf9e20041d2fc17cc2007806

Although it's a small patch, I don't see a good reason to support older versions when we're adding a new platform like this. Especially because I expect most mining users to use our binaries.

The other patch I just upstreamed: capnproto/capnproto#2633

ryanofsky and others added 12 commits April 15, 2026 09:41

doc: Bump version 11 > 12

b15d63e

util, refactor: Add ProcessId type alias and use it

36c91a0

Add ProcessId = int type alias and apply it to WaitProcess, SpawnProcess (pid output argument), and callers.

util, refactor: Add SocketId type alias and use it

94af41b

Add SocketId = int and SocketError = -1 type aliases and apply SocketId to SpawnProcess (return type and callback parameter) and callers.

util, refactor: Add ConnectInfo type alias and use it

beaa50a

Add ConnectInfo type alias to pass socket handle from parent process to child process in more platform independent way.

util, refactor: Add SocketPair() and use it in SpawnProcess

022b29b

Extract socket pair creation from SpawnProcess into a standalone SocketPair() function, and use it to replace the inline socketpair() call. No behavior change.

Sjors force-pushed the 2026/04/ci-win branch from 93ea85c to 2379484 Compare April 20, 2026 14:53

Sjors force-pushed the 2026/04/ci-win branch from 2379484 to 12c3193 Compare April 20, 2026 14:59

This was referenced Apr 21, 2026

proxy: add local connection limit to ListenConnections #269

Draft

cmake: Increase cmake policy version #209

Open

Set cmake_minimum_required(VERSION 3.22) #175

Open

ryanofsky closed this Apr 21, 2026

ryanofsky reopened this Apr 21, 2026

ryanofsky mentioned this pull request Apr 21, 2026

Add windows support #231

Open

Sjors force-pushed the 2026/04/ci-win branch 3 times, most recently from bc01b38 to bbe64cf Compare April 21, 2026 18:59

Sjors force-pushed the 2026/04/ci-win branch from bbe64cf to 58be7b8 Compare April 22, 2026 06:20

ryanofsky and others added 10 commits April 22, 2026 16:35

doc: Remove trailing whitespace

7f513a4

Bitcoin Core linter rejects it: https://github.com/bitcoin/bitcoin/actions/runs/24568789956/job/71835997334?pr=32387

util: drop POSIX/pthread dependencies to enable MSVC builds

7fd5ec4

Remove POSIX and pthread calls from util.cpp to avoid relying on MinGW's POSIX compatibility layer. This lets code be compiled with MSVC.

This was referenced Apr 23, 2026

Add nonunix platform support #274

Open

ci: add newdeps job testing newer versions of cmake and capnproto #212

Open

Sjors force-pushed the 2026/04/ci-win branch from b1199a5 to 4e2e149 Compare April 23, 2026 12:34

Sjors mentioned this pull request Apr 23, 2026

kj: tolerate ERROR_INVALID_FUNCTION from GetFileInformationByHandleEx on Wine capnproto/capnproto#2633

Open

Sjors added 2 commits April 23, 2026 15:38

Sjors force-pushed the 2026/04/ci-win branch from 4e2e149 to 07a7f71 Compare April 23, 2026 13:39

DrahtBot mentioned this pull request Apr 24, 2026

Better error and log messages #218

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci: add Windows job#272

ci: add Windows job#272
Sjors wants to merge 24 commits intobitcoin-core:masterfrom
Sjors:2026/04/ci-win

Sjors commented Apr 20, 2026

Uh oh!

DrahtBot commented Apr 20, 2026 •

edited

Loading

Uh oh!

Sjors commented Apr 20, 2026

Uh oh!

Sjors commented Apr 20, 2026 •

edited

Loading

Uh oh!

Sjors commented Apr 21, 2026

Uh oh!

ryanofsky commented Apr 21, 2026

Uh oh!

Sjors commented Apr 21, 2026

Uh oh!

Sjors commented Apr 21, 2026 •

edited

Loading

Uh oh!

Sjors commented Apr 22, 2026 •

edited

Loading

Uh oh!

ryanofsky commented Apr 22, 2026

Uh oh!

Sjors commented Apr 23, 2026

Uh oh!

Sjors commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Sjors commented Apr 20, 2026

Uh oh!

DrahtBot commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

Sjors commented Apr 20, 2026

Uh oh!

Sjors commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented Apr 21, 2026

Uh oh!

ryanofsky commented Apr 21, 2026

Uh oh!

Sjors commented Apr 21, 2026

Uh oh!

Sjors commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky commented Apr 22, 2026

Uh oh!

Sjors commented Apr 23, 2026

Uh oh!

Sjors commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DrahtBot commented Apr 20, 2026 •

edited

Loading

Sjors commented Apr 20, 2026 •

edited

Loading

Sjors commented Apr 21, 2026 •

edited

Loading

Sjors commented Apr 22, 2026 •

edited

Loading