ipc: Add nonunix platform support #35084

pull ryanofsky wants to merge 11 commits into bitcoin:master from ryanofsky:pr/ipc-wins changing 13 files +352 −282
  1. ryanofsky commented at 4:25 PM on April 15, 2026: contributor

    This PR makes bitcoin core changes needed to be compatible with https://github.com/bitcoin-core/libmultiprocess/pull/274, which changes the libmultiprocess API to stop using unix-specific types so it can be compatible with windows. (Windows support is added in followups: https://github.com/bitcoin-core/libmultiprocess/pull/231 and #32387.)

    The PR uses some compatibility shims so it can be reviewed and potentially merged without needing to merge https://github.com/bitcoin-core/libmultiprocess/pull/274 and bump the libmultiprocess subtree. These can be deleted when subtree is updated.

  2. DrahtBot commented at 4:25 PM on April 15, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35084.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK Sjors
    Concept ACK stickies-v

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #35511 (RFC: consensus: Make CAmount a class by hodlinator)
    • #34020 (mining: add getTransactions(ByWitnessID) IPC methods by Sjors)
    • #19461 (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
    • #19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
    • #10102 (Multiprocess bitcoin by ryanofsky)

    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-->

    LLM Linter (✨ experimental)

    Possible places where comparison-specific test macros should replace generic comparisons:

    • [src/ipc/test/ipc_tests.cpp] BOOST_CHECK_THROW(process->bind(datadir, "test_bitcoin", invalid_bind), std::invalid_argument); -> Prefer BOOST_CHECK_EXCEPTION(...) with a matcher that verifies the exception message/details, instead of only checking the exception type.
    • [src/ipc/test/ipc_tests.cpp] BOOST_CHECK_THROW(process->connect(datadir, "test_bitcoin", invalid_bind), std::invalid_argument); -> Prefer BOOST_CHECK_EXCEPTION(...) with a matcher that verifies the exception message/details, instead of only checking the exception type.

    <sup>2026-06-22 18:59:34</sup>

  3. DrahtBot added the label CI failed on Apr 15, 2026
  4. DrahtBot commented at 5:15 PM on April 15, 2026: contributor

    <!--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>

  5. stickies-v commented at 5:45 PM on April 15, 2026: contributor

    Concept ACK

  6. ryanofsky force-pushed on Apr 16, 2026
  7. ryanofsky commented at 11:29 PM on April 16, 2026: contributor

    <!-- 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

    <!-- begin push-5 -->

    Updated 236762802185b1c8219e6f567964be1ddc97b839 -> 1a97f072f887ab95e2ac89b5cf1bc4a75b7255c6 (pr/ipc-wins.4 -> pr/ipc-wins.5, compare)<!-- end --> with MakeStream cleanup from #32387 pr/ipc-win.23

    <!-- begin push-6 -->

    Rebased 1a97f072f887ab95e2ac89b5cf1bc4a75b7255c6 -> 0a16f106c1862d33b64b683089e7aac84446ac69 (pr/ipc-wins.5 -> pr/ipc-wins.6, compare)<!-- end --> due to conflict with #34806

  8. ryanofsky referenced this in commit e563c968b4 on Apr 16, 2026
  9. ryanofsky referenced this in commit b42ec11a5c on Apr 16, 2026
  10. ryanofsky force-pushed on Apr 17, 2026
  11. ryanofsky force-pushed on Apr 17, 2026
  12. ryanofsky referenced this in commit d9fcac6e30 on Apr 17, 2026
  13. ryanofsky referenced this in commit c537b41d09 on Apr 17, 2026
  14. ryanofsky referenced this in commit f813f9f959 on Apr 17, 2026
  15. DrahtBot removed the label CI failed on Apr 17, 2026
  16. ryanofsky referenced this in commit 74fc8ac8c3 on Apr 17, 2026
  17. ryanofsky referenced this in commit 6d87dd7990 on Apr 17, 2026
  18. ryanofsky referenced this in commit 68e367a0aa on Apr 17, 2026
  19. ryanofsky referenced this in commit 97b3bc9e31 on Apr 17, 2026
  20. ryanofsky referenced this in commit eeac89b481 on Apr 17, 2026
  21. ryanofsky referenced this in commit e451de95b3 on Apr 17, 2026
  22. SirMentos-apt commented at 1:14 PM on April 20, 2026: none

    can run bitcoin core on WSL ?

  23. Sjors commented at 3:04 PM on April 20, 2026: member

    @SirMentos-apt: @hebasto might know.

  24. ryanofsky referenced this in commit 323eb3a169 on Apr 20, 2026
  25. ryanofsky force-pushed on Apr 21, 2026
  26. ryanofsky referenced this in commit e2872e28cb on Apr 21, 2026
  27. Sjors referenced this in commit b1199a5604 on Apr 22, 2026
  28. ryanofsky referenced this in commit 926ae3562e on Apr 22, 2026
  29. DrahtBot added the label Needs rebase on May 22, 2026
  30. ipc, moveonly: combine ipc_test.cpp and ipc_tests.cpp
    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
    5e2b81301c
  31. ipc, refactor: Drop connect/listen/serve exe_name parameters
    Pass exe_name parameter 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.
    0146b59ae4
  32. ipc, refactor: Change Protocol class field order
    This just changes Protocol class field order to make sure class members are not
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.
    7220a142f8
  33. ipc, refactor: use native path separators in test
    Avoid hardcoded forward slashes is ParseAddress test, use native path
separators instead.
    4715190406
  34. ipc, refactor: fix include order
    Keep standard headers separate from posix headers
    19414bceea
  35. ryanofsky force-pushed on May 29, 2026
  36. DrahtBot removed the label Needs rebase on May 29, 2026
  37. DrahtBot added the label CI failed on May 29, 2026
  38. DrahtBot removed the label CI failed on Jun 1, 2026
  39. Sjors commented at 2:43 PM on June 17, 2026: member

    IIUC this can (must?) be merged before updating the subtree to https://github.com/bitcoin-core/libmultiprocess/pull/231?

  40. ryanofsky commented at 2:56 PM on June 17, 2026: contributor

    IIUC this can (must?) be merged before updating the subtree to bitcoin-core/libmultiprocess#231?

    Yes this is reviewable and meant to be merged before https://github.com/bitcoin-core/libmultiprocess/pull/231. Technically these changes could be made in the same PR updating the subtree, but seems a little better to keep bitcoin changes and libmultiprocess ones separate for better review. Most of the changes here are pretty small though. The biggest change is merging previously separated test files.

  41. Sjors commented at 3:15 PM on June 17, 2026: member

    Ok, maybe a better PR title would be "ipc: prepare for windows support"?

  42. in src/ipc/util.h:23 in 0a16f106c1 outdated 
      18 | +#include <sys/socket.h>
  19 | +
  20 | +namespace mp {
  21 | +// Definitions that can be deleted when libmultiprocess subtree is updated to
  22 | +// v12. Having these allows Bitcoin Core changes to be decoupled from
  23 | +// libmultiprocess changes so they don't have to be reviewed in a single PR.
    Sjors commented at 6:32 PM on June 18, 2026:

    This is smart. Would be good to point out in the PR description that this can merged before ~or after~ https://github.com/bitcoin-core/libmultiprocess/pull/274 because of this neat little trick :-)

    ryanofsky commented at 6:44 PM on June 22, 2026:

    re: #35084 (review)

    This is smart. Would be good to point out in the PR description that this can merged before ~or after~ bitcoin-core/libmultiprocess#274 because of this neat little trick :-)

    Thanks, updated PR description to mention https://github.com/bitcoin-core/libmultiprocess/pull/274 and explain this.

  43. in src/ipc/test/ipc_tests.cpp:155 in 3cd19ce33a 
     150 | @@ -151,15 +151,15 @@ void IpcSocketTest(const fs::path& datadir)
 151 |  
 152 |      auto bind_and_listen{[&](const std::string& bind_address) {
 153 |          std::string address{bind_address};
 154 | -        int serve_fd = process->bind(datadir, "test_bitcoin", address);
 155 | +        mp::SocketId serve_fd = process->bind(datadir, "test_bitcoin", address);
 156 |          BOOST_CHECK_GE(serve_fd, 0);
    Sjors commented at 8:15 AM on June 19, 2026:

    In 3cd19ce33a4d147674af637ab92c7df30176fe0b ipc, refactor: Add SocketId type alias and use it nit:

    BOOST_CHECK_NE(serve_fd, mp::SocketError);
    ryanofsky commented at 6:45 PM on June 22, 2026:

    re: #35084 (review)

    BOOST_CHECK_NE(serve_fd, mp::SocketError);

    Thanks, added

  44. Sjors commented at 11:30 AM on June 19, 2026: member

    ACK 0a16f106c1862d33b64b683089e7aac84446ac69

    I also checked that the tests still pass if I pull the subtree from #274 and drop the <v12 shim. Ideally we do that in a single PR.

  45. DrahtBot requested review from stickies-v on Jun 19, 2026
  46. ipc: Avoid 'unistd.h' error with MSVC
    Avoid compile error from MSVC:

D:\a\bitcoin\bitcoin\src\ipc\interfaces.cpp(24,1): error C1083: Cannot open include file: 'unistd.h': No such file or directory

MinGW provides this header but MSVC does not. Header is unneeded on windows
because HandleCtrlC code that uses it is not compiled on windows.
    4ef1eba4a3
  47. ipc: fix MSVC build error C3861 in ipc_tests.cpp
    MSVC does not provide mkstemp (a POSIX function available in MinGW but
not the MSVC runtime), causing:

  error C3861: 'mkstemp': identifier not found
  error C4996: 'close': The POSIX name for this item is deprecated

Fix by using _mktemp_s on WIN32, which fills in the XXXXXX suffix
in-place without creating a file (so no fd to close or file to remove).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
    587fcedb90
  48. ipc, refactor: Add ProcessId type alias and use it
    Use ProcessId type instead of int to represent process ids to be
compatible with an upcoming version of libmultiprocess which adds
windows support.
    adeacc7291
  49. ipc, refactor: Add SocketId type alias and use it
    Use SocketId type instead of int to represent socket ids to be
compatible with an upcoming version of libmultiprocess which adds
windows support.
    68ca5fb776
  50. ipc, refactor: Add ConnectInfo type alias and use it
    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.
    b8b3753f26
  51. ipc, refactor: Add Stream type alias and use it
    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.
    c8dca662d1
  52. ryanofsky referenced this in commit 9d2dd067fc on Jun 22, 2026
  53. ryanofsky force-pushed on Jun 22, 2026
  54. ryanofsky renamed this:
    ipc: Support for windows support
    ipc: Add nonunix platform support
    on Jun 22, 2026
  55. ryanofsky commented at 7:08 PM on June 22, 2026: contributor

    re: #35084 (comment)

    Ok, maybe a better PR title would be "ipc: prepare for windows support"?

    Yeah "support for windows support" was meant to be a little jokey. Renamed to be consistent with https://github.com/bitcoin-core/libmultiprocess/pull/274.

    <!-- begin push-7 -->

    Updated 0a16f106c1862d33b64b683089e7aac84446ac69 -> c8dca662d1a4e2e99035e2f367d865a7b047b53c (pr/ipc-wins.6 -> pr/ipc-wins.7, compare)<!-- end --> adding MSVC compatibility fixes and implementing review suggestions

  56. ryanofsky referenced this in commit 10ca1d46a3 on Jun 22, 2026
  57. ryanofsky referenced this in commit da3767fb40 on Jun 22, 2026
  58. Sjors commented at 5:43 PM on June 23, 2026: member

    utACK c8dca662d1a4e2e99035e2f367d865a7b047b53c

    I only briefly glanced at the MSVC change, didn't test that.

Contributors
ryanofskyDrahtBotstickies-vSirMentos-aptSjors


stickies-v

Linked (view graph)
#28722 Multiprocess tracking issue#32387 ipc: add windows support#35142 RFC: IPC based tracing interface (alternative to eBPF/USDT)
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-06-24 12:51 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me