#35932 ipc: make ipc::disconnectIncoming wait for in-progress calls to complete
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #35932 ipc: make ipc::disconnectIncoming wait for in-progress calls to complete | Needs rebase | 0 | Crickets | 2,169 + 3 tests | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Separates connection teardown from destruction in libmultiprocess and adds server object tracking so callers can wait for in-flight calls to drain. Updates Bitcoin Core's `ipc::disconnectIncoming` to wait off the event loop thread until active server call bodies finish executing before freeing node state during shutdown. Also brings in libmultiprocess fixes for races and use-after-free bugs during connection disconnects.
Problem: During node shutdown, `Ipc::disconnectIncoming` drops incoming client connections and cancels pending promises, but does not wait for asynchronous C++ call bodies already executing on worker threads (such as `Mining.checkBlock`) to complete. A still-running call body can dereference node components after `Shutdown()` frees them, triggering assertion failures such as `Assert(m_node.chainman)`.
Evidence of importance:
- Antithesis bug report in #35845 and #33387: 'if asynchronous IPC mining calls are made when the node is shutting down it's possible for assert(m_node.chainman) to trigger'
- Commit 6023bbb113: 'A still-running body (an in-flight Mining.checkBlock) could then dereference m_node.chainman after chainman.reset() nulled it, aborting on Assert(m_node.chainman)'
Scope: Contains a subtree bump for `src/ipc/libmultiprocess` along with direct commits modifying the subtree before upstream merge; includes a temporary lint workaround disabling subtree link checks that must be removed before merging.
Risk of a mistake: medium (other). A flaw in connection draining or reference counting could cause node shutdown to hang indefinitely or introduce race conditions in multiprocess IPC.
Changed since the previous assessment: Incorporated upstream libmultiprocess fixes for double-free races and thread map teardown use-after-free on disconnect, added compiler warning suppressions for C++23 lambda attributes, and temporarily bypassed the subtree lint check.
Reviewability
Ready · Needs rebase
Conflicts with master; review the logic now, the rebase is mechanical.
Waiting on (from the thread read):
- author: upstream libmultiprocess changes and taking the PR out of draft
Agreement
Crickets
no substantive comment either way
Thread notes: Initial record. Draft PR with no reviewers or comments from anyone other than the author.
Objections, suggestions, and questions: none recorded.
Files
4 lines under test/bench/ci.
- src/ipc/libmultiprocess/include/mp/proxy-io.h +343/-92
- src/ipc/libmultiprocess/test/mp/test/test.cpp +402/-27
- src/ipc/libmultiprocess/src/mp/proxy.cpp +222/-78
- src/ipc/libmultiprocess/src/mp/util.cpp +216/-31
- src/ipc/libmultiprocess/test/mp/test/connect_tests.cpp +212/-0
- src/ipc/libmultiprocess/test/mp/test/listen_tests.cpp +77/-77
- src/ipc/libmultiprocess/test/mp/test/unixlistener.h +84/-0
- src/ipc/libmultiprocess/src/mp/gen.cpp +39/-41
- src/ipc/libmultiprocess/include/mp/util.h +63/-15
- src/ipc/capnp/protocol.cpp +48/-3
- src/ipc/libmultiprocess/include/mp/proxy-types.h +37/-12
- src/ipc/libmultiprocess/cmake/compat_config.cmake +0/-42
- src/ipc/libmultiprocess/include/mp/type-number.h +24/-15
- src/ipc/libmultiprocess/ci/scripts/ci.sh +34/-1
- src/ipc/libmultiprocess/.github/workflows/ci.yml +19/-14
- src/ipc/libmultiprocess/test/mp/test/spawn_tests.cpp +28/-5
- src/ipc/libmultiprocess/test/mp/test/foo.h +31/-1
- src/ipc/libmultiprocess/include/mp/type-chrono.h +25/-2
- src/ipc/libmultiprocess/test/mp/test/common.h +27/-0
- src/ipc/libmultiprocess/CMakeLists.txt +18/-8
- src/ipc/libmultiprocess/include/mp/type-context.h +19/-7
- src/ipc/libmultiprocess/include/mp/type-struct.h +12/-12
- src/ipc/libmultiprocess/include/mp/proxy.h +15/-6
- src/ipc/test/fuzz/ipc.cpp +2/-19
- src/ipc/libmultiprocess/include/mp/type-interface.h +10/-10
- src/ipc/libmultiprocess/shell.nix +13/-6
- CMakeLists.txt +14/-0
- src/ipc/libmultiprocess/.clang-tidy +12/-2
- src/ipc/libmultiprocess/example/calculator.cpp +4/-10
- src/ipc/libmultiprocess/example/printer.cpp +4/-10
- src/ipc/test/ipc_tests.cpp +2/-12
- src/ipc/libmultiprocess/.github/workflows/bitcoin-core-ci.yml +8/-5
- src/ipc/libmultiprocess/example/example.cpp +7/-5
- src/ipc/libmultiprocess/include/mp/type-char.h +9/-3
- src/ipc/libmultiprocess/test/mp/test/foo.capnp +12/-0
- src/ipc/libmultiprocess/ci/scripts/run.sh +6/-4
- src/ipc/libmultiprocess/doc/versions.md +8/-2
- src/ipc/libmultiprocess/ci/configs/netbsd.bash +1/-7
- src/ipc/libmultiprocess/cmake/pthread_checks.cmake +8/-0
- src/ipc/libmultiprocess/include/mp/type-threadmap.h +4/-4
- src/ipc/libmultiprocess/ci/configs/olddeps.bash +5/-2
- src/ipc/libmultiprocess/ci/configs/newdeps.bash +6/-0
- src/ipc/libmultiprocess/doc/usage.md +6/-0
- src/ipc/libmultiprocess/test/mp/test/foo-types.h +6/-0
- src/ipc/libmultiprocess/include/mp/type-data.h +2/-3
- src/ipc/libmultiprocess/ci/configs/sanitize.bash +2/-2
- src/ipc/libmultiprocess/ci/scripts/config.sh +2/-2
- src/ipc/libmultiprocess/doc/design.md +2/-2
- src/ipc/libmultiprocess/include/mp/type-string.h +3/-1
- test/lint/test_runner/src/util.rs +3/-1
- src/ipc/libmultiprocess/ci/configs/default.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/freebsd.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/gnu32.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/llvm.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/macos.bash +1/-1
- src/ipc/libmultiprocess/ci/configs/openbsd.bash +1/-1
- src/ipc/libmultiprocess/cmake/TargetCapnpSources.cmake +1/-1
- src/ipc/libmultiprocess/doc/install.md +1/-1
- src/ipc/libmultiprocess/include/mp/type-function.h +1/-1
- src/ipc/libmultiprocess/include/mp/type-map.h +2/-0
- src/ipc/libmultiprocess/include/mp/type-unordered-set.h +1/-1
- src/ipc/libmultiprocess/include/mp/version.h +1/-1
- src/ipc/libmultiprocess/ci/README.md +1/-0
- src/ipc/libmultiprocess/include/mp/config.h.in +1/-0
- src/ipc/libmultiprocess/test/CMakeLists.txt +1/-0
Card
The short text the ranking pass compares across PRs.
This PR updates IPC disconnect handling to wait for active asynchronous call bodies executing on worker threads to drain before node shutdown continues. This resolves an assertion failure and potential crash reported by Antithesis in #35845 and #33387, where in-flight mining calls access node state after it has been cleared during shutdown. It also includes several fixes for disconnect-time race conditions in libmultiprocess. It is currently a draft dependent on upstream changes in bitcoin-core/libmultiprocess#335.
Data
thread read through 2026-08-12 (1 statements), head 228ca73a64
labels: Needs rebase, IPC · draft
Categories
IPC / multiprocess
P1 · bug fix
Fixes a shutdown assertion failure and use-after-free where active IPC calls dereference node state after teardown. During shutdown, `Ipc::disconnectIncoming` fails to wait for running asynchronous worker threads such as `Mining.checkBlock`, triggering `Assert(m_node.chainman)` as reported by Antithesis in #35845.
Membership: Has the IPC label and modifies the libmultiprocess subtree along with `ipc::disconnectIncoming` and Cap'n Proto interface connection management.
Factors: security/stability 2, bug 2, performance 0, user value 0, leverage 1