This PR makes bitcoin core changes needed to be compatible with https://github.com/bitcoin-core/libmultiprocess/pull/231, which adds windows to support libmultiprocess.
The two PRs are meant to be reviewed in parallel and there is a draft PR #32387 combining both of these with more changes to enable IPC by default on windows.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | stickies-v |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, 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.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
Possible places where comparison-specific test macros should replace generic comparisons:
BOOST_CHECK_THROW(process->bind(datadir, "test_bitcoin", invalid_bind), std::invalid_argument); -> Consider using BOOST_CHECK_EXCEPTION with HasReason(...) to validate the thrown message (e.g., Unrecognized address 'invalid:'), not just the exception type.BOOST_CHECK_THROW(process->connect(datadir, "test_bitcoin", invalid_bind), std::invalid_argument); -> Consider using BOOST_CHECK_EXCEPTION with HasReason(...) to validate the thrown message (e.g., Unrecognized address 'invalid:'), not just the exception type.<sup>2026-04-17 00:47:10</sup>
<!--85328a0da195eb286784d51f73fa0af9-->
🚧 At least one of the CI tasks failed.
<sub>Task FreeBSD Cross: https://github.com/bitcoin/bitcoin/actions/runs/24465865499/job/71492637060</sub>
<sub>LLM reason (✨ experimental): CI failed due to a linker error: duplicate symbol mp::SocketPair() in libbitcoin_ipc.a (defined in multiple objects).</sub>
<details><summary>Hints</summary>
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
</details>
Concept ACK
Previously ipc_test.cpp contained tests which depended on
libmultiprocess and ipc_tests.cpp contained tests which didn't.
Separation was needed because libmultiprocess tests need to be built
with additional include and link paths, and cmake only has good support
for setting these on libraries, not source files. The separation also
allowed the add_boost_test custom cmake function to work with no changes,
because it could find the boost test registration in ipc_tests.cpp, and
then ipc_tests.cpp would run the tests in ipc_test.cpp without them
needing to be registered in boost.
But with windows support being added, the parse address test can't
easily avoid a dependecy on libmultiprocess, because it depends on the
ipc/process.h header, and ipc/process.h header will now need
platform-specific ProcessId and SocketId types defined by
libmultiprocess, rather than plain ints.
With all ipc tests depending on libmultiprocess, there is not really a
rationale for having separate test files anymore, so this change
combines them, and move the cmake add_boost_test function definition so
it can be used instead of target_sources to register ipc_tests.cpp with
ctest.
The change prevents CI errors from including ipc/process.h in
ipc_tests.cpp:
In file included from /Users/runner/work/bitcoin/bitcoin/repo_archive/src/ipc/test/ipc_tests.cpp:5:
In file included from /Users/runner/work/bitcoin/bitcoin/repo_archive/src/ipc/process.h:11:
/Users/runner/work/bitcoin/bitcoin/repo_archive/src/ipc/util.h:14:10: fatal error: 'kj/debug.h' file not found
14 | #include <kj/debug.h>
https://github.com/bitcoin/bitcoin/actions/runs/24465865499/job/71492617687?pr=35084
Pass to ipc::Protocol class constructor instead. It never really made
sense to have exe parameters as part of the protocol interface and
removing them makes adding new features like windows support easier.
The exe name values are only used for logging and debuggging purposes to
distinguish log messages from different processes.
This just changes Protocol class field order to make no class members
are destroyed before the event loop thread exits. There is no change in
behavior. The change is just being made to clarify intent and avoid
potential bugs.
Avoid hardcoded forward slashes is ParseAddress test, use native path
separators instead.
Keep standard headers separate from posix headers
Use ProcessId type instead of int to represent process ids to be
compatible with an upcoming version of libmultiprocess which adds
windows support.
Use SocketId type instead of int to represent socket ids to be
compatible with an upcoming version of libmultiprocess which adds
windows support.
Use ConnectInfo type instead of int to represent socket ids that are
passed between processes, to be compatible with an upcoming version of
libmultiprocess which adds windows support.
Use Stream type to abstract socket ids and be compatible with updated
mp::ConnectStream() and mp::ServeStream() functions that use streams
instead of socket ids in an upcoming version of libmultiprocess which
adds windows support.
Since creating Stream objects from socket ids can require the event loop
to be running, the ipc::Protocol::serve() method is also updated to
accept the server stream though a callback parameter instead of a normal
parameter.
<!-- begin push-2 -->
Updated 9c39299c7589cb650178f4a411956f2818d1b5c5 -> 012f428e7fe52707ea7421201def84a847328341 (pr/ipc-wins.1 -> pr/ipc-wins.2, compare)<!-- end --> to fix CI failures https://github.com/bitcoin/bitcoin/actions/runs/24465865499: ipc_tests cmake include directory bug and multiple definition link error
<!-- begin push-3 -->
Updated 012f428e7fe52707ea7421201def84a847328341 -> 129f48be88f738e6af9960fe81c0a9d05bb8b5f9 (pr/ipc-wins.2 -> pr/ipc-wins.3, compare)<!-- end --> to fix CI failures https://github.com/bitcoin/bitcoin/actions/runs/24539375845: HasReason include error and lint error about ipc_tests filename
<!-- begin push-4 -->
Updated 129f48be88f738e6af9960fe81c0a9d05bb8b5f9 -> 236762802185b1c8219e6f567964be1ddc97b839 (pr/ipc-wins.3 -> pr/ipc-wins.4, compare)<!-- end --> adding an extra version check to be able to build against https://github.com/bitcoin-core/libmultiprocess/pull/231 without code changes
can run bitcoin core on WSL ?
@SirMentos-apt: @hebasto might know.