This PR is a draft. I wrote most of the code that should be needed to support windows, but am still debugging various issues.
This is meant to resolve https://github.com/bitcoin-core/libmultiprocess/issues/53 and https://github.com/bitcoin-core/libmultiprocess/issues/114
This PR is part of the process separation project.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32387.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | hebasto |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
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.
Possible typos and grammar issues:
analagous → analogous
pr/ipc-win.1 -> pr/ipc-win.2, compare) fixing accidentally disabled tests
🚧 At least one of the CI tasks failed.
Task multiprocess, i686, DEBUG: https://github.com/bitcoin/bitcoin/runs/41420704031
LLM reason (✨ experimental): The CI failure is caused by an assertion failure in the IPC test suite due to a nullptr dereference.
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.
Updated 87432b6a4325e09a13c912d77b386daa3832b34d -> 0a35e21103b7fed6e46c809e9029446277b6f6ee (pr/ipc-win.2 -> pr/ipc-win.3, compare) to fix test CI failure https://cirrus-ci.com/task/5583224107171840 due to bad std::optional access
Updated 0a35e21103b7fed6e46c809e9029446277b6f6ee -> 27328195136754babe8d8f5d99f4f0b6a5ab2e55 (pr/ipc-win.3 -> pr/ipc-win.4, compare) to fix CI test failure https://cirrus-ci.com/task/5668412652781568 in rpc_misc.py echoipc call due to CLOEXEC flag
re: #32387 (comment)
Agree changes in #31756 should make this easier to test (though I might be able to make a minimal change here to turn on multiprocess windows build).
Having #30437 or #32297 merged first would also make test coverage more meaningful because they both add functional tests for -ipcbind and -ipcconect options.
In general there is more work to do here and I hope other PRs can be merged before this one.
Updated 27328195136754babe8d8f5d99f4f0b6a5ab2e55 -> f215e742045401ab386f0cd67d06b358558ecf89 (pr/ipc-win.4 -> pr/ipc-win.5, compare) fixing many windows bugs.
With this update, IPC code is mostly working on windows: mpexample and mptest programs work, test_bitcoin IPC calls over socketpairs work, bitcoin-node echoipc command is able to create a subprocess and successfully make calls to it, and bitcoin-node -ipcbind option creates a unix socket file. Some problems remain:
bitcoin-mine -ipcconnect seems unable to find the unix socket, failing with “Error: The system cannot find the file specified” even though the socket exists. Two test_bitcoin unix socket tests also give similar errors.test_bitcoin parse address tests fails because some checks are looking for / not \ separators.