ipc: make ipc::disconnectIncoming wait for in-progress calls to complete #35932

3577d23f616 ci: Enable -Wundef in default and llvm jobs
debd42955ea proxy-types: Avoid -Wundef error when KJ_NO_EXCEPTIONS is not defined
a8c64c7faa5 ci: Enable -Wunused-const-variable in GCC jobs
701449c1657 util: Make SocketError inline constexpr
6fd2a9c90e1 Merge bitcoin-core/libmultiprocess#367: mpgen: Change Cap'n Proto link order to fix undefined references in static builds
f1042b1a60e Merge bitcoin-core/libmultiprocess#366: ci: Compile with minimum supported g++-10 in olddeps
a6ecdc113ff Merge bitcoin-core/libmultiprocess#364: util: Error out at compile time if KJ_NO_EXCEPTIONS is set
56cfdaa884e Change CapnProto library linkage for mpgen
fa5633467af ci: Compile with minimum supported g++-10 in olddeps
4a1f400f8b7 util: Error out at compile time if KJ_NO_EXCEPTIONS is set
2dba130478a Merge bitcoin-core/libmultiprocess#363: ci: use LLVM 23 in Bitcoin Core CI
161197a5c7b Merge bitcoin-core/libmultiprocess#352: ci: add cmake debug output
fb4ac7eb8c8 ci: use LLVM 23 in Bitcoin Core CI
7bac69de1fc Merge bitcoin-core/libmultiprocess#360: pull latest .clang-tidy from downstream
79ddc44eb32 Merge bitcoin-core/libmultiprocess#359: ci: bump cmake version to 4.3.4 in newdeps job
bf229bf82ae Merge bitcoin-core/libmultiprocess#351: ci: do not ignore NIXPKGS_CHANNEL in local ci runs
073ac4f1972 Merge bitcoin-core/libmultiprocess#347: refactor: Replace EventLoop::post() with sync() taking kj::FunctionParam
fa1db7a9e3e pull latest .clang-tidy from downstream
5c49666a1d8 refactor: rename EventLoop::m_post_fn to m_sync_fn
2330fbe81e2 refactor: replace EventLoop::post() with sync() taking kj::FunctionParam
4d454a81da6 Merge bitcoin-core/libmultiprocess#358: refactor: Enable readability-container-contains
dba99582b32 Merge bitcoin-core/libmultiprocess#357: doc: Update Cap'n Proto version to match minimum
00923922ac3 Merge bitcoin-core/libmultiprocess#356: ci: Remove hard-coded -j4 from sanitize config
766867fb326 ci: clarify CAPNP_CHECKOUT=master is the v1.x release branch
cc3675280bf ci: bump cmake version to 4.3.4 in newdeps job
fa101113b50 refactor: Enable readability-container-contains
81f824b025f doc: Update Cap'n Proto version to match minimum
fa30e2093c7 ci: Remove hard-coded -j4 from sanitize config
a7ff9d5da22 ci: add cmake debug output
f2e8df82ede Merge bitcoin-core/libmultiprocess#350: cmake: add type-unordered-set.h and version.h to public headers
0e146c0468d Merge bitcoin-core/libmultiprocess#349: type-context: fix async disconnect race condition found by antithesis
49f95e26326 Merge bitcoin-core/libmultiprocess#212: ci: add newdeps job testing newer versions of cmake and capnproto
7c73cceda9b ci: rename CI-internal variables to use CI_ prefix
fe1b8339f00 ci: do not ignore NIXPKGS_CHANNEL in local ci runs
914dc839f27 proxy: fix data race between server request threads and disconnect handling
275c8eefdfb Merge bitcoin-core/libmultiprocess#345: Remove trailing whitespace and Add -Wtrailing-whitespace to default ci config
cd7162fb832 Merge bitcoin-core/libmultiprocess#304: proxy: fix BuildList to use non-const iteration for interface types
9b136782af1 ci: Add -Wtrailing-whitespace to default config
2f4be9ec6ea refactor: Remove trailing whitespace
2448d282ccd cmake: add type-unordered-set.h and version.h to public headers
b3fc922ee4a ci: add newdeps job testing newest versions of cmake and capnproto
390b5f901f1 Merge bitcoin-core/libmultiprocess#344: test: listen_tests and connect_tests follow-ups
d6f8588d1ab proxy: fix BuildList to use non-const iteration for interface types
e18ca520f45 Merge bitcoin-core/libmultiprocess#343: test: fix race in connect_tests disconnect-deferred-failure test
c39c7850c66 doc: note construct() call in valid init interface test
b9c36c61751 test: close sockets unconditionally and check errors with KJ_SYSCALL
7eb741e6359 test: drop unnecessary KJ_EXPECT(true)
113f1d4d287 test: join server thread unconditionally in connect tests
44bc4630bc1 test: drop mp:: prefixes in connect tests
038d33eb31e test: share DefaultLogHandler between test files
b54a1633085 test: drop TestSetup socket members in connect tests
70467c5a727 test: add m_ prefix to TestSetup members in connect tests
cc260f2526f test: replace capnp fix link with upstream PR
137a6e4e039 test: fix race in connect_tests disconnect-deferred-failure test
8dab0d4bdeb Merge bitcoin-core/libmultiprocess#341: ci: add -Wextra-semi to llvm config
b3b134eed8b ci: add -Wextra-semi to llvm config
bdd0cd69418 Merge bitcoin-core/libmultiprocess#339: refactor: add `[[noreturn]]` attributes
a779a09764c ci: add -Wmissing-noreturn
636aaff576b refactor: add missing [[noreturn]] attributes
cc11c2b1b41 Merge bitcoin-core/libmultiprocess#338: test: check ReadList return value
2d6e863c77a Merge bitcoin-core/libmultiprocess#334: ci: Set CMAKE_BUILD_PARALLEL_LEVEL to enable parallelism by default
d4d10ff98ab Merge bitcoin-core/libmultiprocess#332: ci: add -Wextra-semi to default config
b540e70f25f Merge bitcoin-core/libmultiprocess#324: proxy: Name threads spawned by the event loop
e5e367e785c Merge bitcoin-core/libmultiprocess#312: util: report back child errors to parent and throw
2220df68c91 Merge bitcoin-core/libmultiprocess#298: Fix error handling when creating clients (`mp::ConnectStream`)
51defb79ef7 Merge bitcoin-core/libmultiprocess#340: ci: Update `capnproto` prerequisites on NetBSD
7e94790b08a ci: Update `capnproto` prerequisites on NetBSD
9f25ffca5b0 test: Cover OS thread names for worker, pool, and async threads
648a18589c4 proxy: Name threads spawned by the event loop
49834b2609e ci: add -Wextra-semi to default config
fae9a637e35 example: Remove unused kj/async.h include
bb473690c97 Fix error handling when creating clients
44d191420c6 Add test coverage for ConnectStream
231361ae5af Correct stale UnixListener doc comment
060c1a50d03 Extract `UnixListener` class to a dedicated file
62f25af06c3 test: check ReadList return value
ce51d737255 ci: Set CMAKE_BUILD_PARALLEL_LEVEL to enable parallism in build jobs by default
67302cd132a Merge bitcoin-core/libmultiprocess#331: Remove code for Cap'n Proto versions before 0.9
f13c64ab54e Merge bitcoin-core/libmultiprocess#330: ci: Compile with minimum supported g++ in olddeps
8e026f66252 Merge bitcoin-core/libmultiprocess#327: build: avoid unnecessary capnp-rpc dependency for mpgen
e5206e9eb5b Merge bitcoin-core/libmultiprocess#325: cmake: Remove `QUIET` option from `find_package(CapnProto ...)`
879efea2bc7 Merge bitcoin-core/libmultiprocess#321: ci: Roll NetBSD releases to 11.0, drop 9.4
abf127a3141 Merge bitcoin-core/libmultiprocess#317: ipc: Fix mpgen capnp tool path for vcpkg/Windows builds
c437d7f107e Merge bitcoin-core/libmultiprocess#310: test: cover immediate client disconnects for `ListenConnections`
31bff8a673f Merge bitcoin-core/libmultiprocess#307: refactor: memcpy -> std::ranges::copy
f355108b0a0 Merge bitcoin-core/libmultiprocess#303: type-chrono: Add CustomBuildField/CustomReadField overloads for std::chrono::time_point
2d678177c14 Merge bitcoin-core/libmultiprocess#296: ci: Bump channel to nixos-26.05
3f05b11624c util: kill and reap child on SpawnProcess error
4a56c1837a7 util: report back child error to parent and throw
a9e70dbe775 ci: Add NetBSD release 11.0
2d33b14fb0e ci: Switch to default compiler on NetBSD 9.4
36f74002775 ci: Drop NetBSD release 9.4
bd508311b56 refactor: Drop stray semicolons after function definitions
788f17a8509 Remove code for Cap'n Proto versions before 0.9
7402affd0ce ci: Pin oldeps config to older nixpkgs channel to compile older cmake with older gcc
edf63435624 ci: Compile with minimum supported g++-11 in olddeps
fa47449afe1 cmake: avoid unnecessary capnp-rpc dependency for mpgen
a494b764de5 cmake: Remove `QUIET` option from `find_package(CapnProto ...)`
26452e02d75 refactor: memcpy -> std::ranges::copy
e1dcc6eb182 Merge bitcoin-core/libmultiprocess#316: cmake: Fix stale codegen when mpgen binary changes
7a72df02e2d type-chrono: Add CustomBuildField/CustomReadField overloads for std::chrono::time_point
45b685c3f58 type-number, type-chrono: Fix static assert signed/unsigned comparisons
45f6255975d type-number: exclude bool from the integral overload
8d6d4649482 Merge bitcoin-core/libmultiprocess#315: Fix startup race in example
a6fc80d2547 Merge bitcoin-core/libmultiprocess#311: bugfix: clear FD_CLOEXEC in child instead of parent before fork
496fb84e69f test: cover immediate client disconnects for `ListenConnections`
36c6c63520e doc: Document reference-counted EventLoop lifetime
3a997e113cf Fix startup race in mpexample
f5c15ce33ff Merge bitcoin-core/libmultiprocess#323: refactor: access ThreadContext through CurrentThread(), ci: switch Bitcoin Core to master
66298c737f4 ci: Switch back to Bitcoin Core's master branch
86b48105018 refactor: access ThreadContext through CurrentThread()
eea9c64f6e1 cmake: Fix stale codegen when mpgen binary changes
a26a08496b8 cmake: Fix mpgen capnp tool path for vcpkg/Windows builds
140d9ba6ff7 test: allow custom log handler in `ListenSetup`
1e0c7ff9a51 util: Clear FD_CLOEXEC in child instead of parent before fork
8550ee6a317 util, refactor: Add ChildFail helper for post-fork child errors
17eab90b526 test: Fix typo in listen_tests.cpp
ce865a9ba86 refactor: Directly use value in CustomBuildField
3f221b5bfd7 Merge bitcoin-core/libmultiprocess#274: Add nonunix platform support
1b0f6056062 doc: Remove trailing whitespace
d8f8ca3119f ipc: Wrap mpgen main() in try-catch to print errors
fbe5a14ad46 ci: Check out bitcoin/bitcoin PR #35084 instead of master
39d3690d83d types: Replace SFINAE with requires clauses to avoid MSVC C2039 error
ba68520203c proxy, refactor: Fix C4305 truncation warning in Accessor on MSVC
1d81d47811e util, refactor: Fix PtrOrValue constructor for move-only types on MSVC
b883fe1e527 proxy: Fix shutdownWrite() exception handling on macOS with dynamic libraries
0012411ccc6 proxy: Call shutdownWrite() in Connection destructor
38312ad1912 proxy, refactor: Change ConnectStream and ServeStream to accept stream objects
e96d5d742ab proxy, refactor: Replace EventLoop wakeup fd integers with KJ stream objects
db4f9a3d739 cmake: Bump minimum required Cap'n Proto version to 0.9
652934fb793 util, refactor: Add SocketPair() and use it in SpawnProcess
1c6ef7a26c0 util, refactor: Do not fork() and exec() separately
1389cf3132f util, refactor: Add SpawnConnectInfo type alias and use it
c7ca1f00b62 util, refactor: Add SocketId type alias and use it
be46a35203c util, refactor: Add ProcessId type alias and use it
91a78db7808 doc: Bump version 13 > 14
fa2c56ec27f ci: Bump channel to nixos-26.05

git-subtree-dir: src/ipc/libmultiprocess
git-subtree-split: 3577d23f616ec6e8d601d14452a5793f9d386708

Lambda [[noreturn]] attributes before the parameter list are a C++23
extension intentionally used in C++20 builds (needed for compatibility
with -Wmissing-noreturn). Suppress the extension warning rather than
removing the attributes or working around them.

Uses the existing IF_CHECK_PASSED idiom so GCC, which silently ignores
unknown -Wno-* flags, is unaffected.

This change was written with Claude Sonnet 4.6.

This is a documentation-only change meant to make upcoming commits easier to
understand.

This change was written with Claude Opus 4.8 (1M context).

Correct the ThreadContext "Synchronization note", which said
Waiter::m_mutex must not be locked before EventLoop::m_mutex. That is
the reverse of the documented and actual lock order (Waiter::m_mutex
first, as ~ProxyServer<Thread> does). The constraint it was reaching for
is the EventLoop blocking rule now documented on Waiter::m_mutex.

This change was written with Claude Fable 5.1.

Currently, a ListenConnections listener that reaches its max-connection limit
stops accepting new connections permanently if one of its connections is closed
locally instead of by a remote disconnect. Closing a connection locally (e.g.
erasing it from m_incoming_connections) leaves the listener's active-connection
count stuck at the limit, so it never resumes accepting.

This happens because the count is decremented by a callback which only fires on
a remote disconnects, not local disconnects. Fix by moving the decrement to
callback which fires on both local and remote disconnects.

Add a regression test that closes a connection locally and checks the listener
resumes accepting; it fails before this change (the listener never accepts the
waiting client) and passes after.

Co-Authored-By: Enoch Azariah <enirox001@gmail.com>
This change was written with Claude Opus 4.8 (1M context).

Fix a use-after-free, possible since the destroy_connection option was added
in 2019 (c685fa9): a Connection's disconnect handler could run after the
Connection had already been destroyed, deleting it a second time and
crashing. Reported by enirox001 in
https://github.com/bitcoin-core/libmultiprocess/pull/335#discussion_r3821831654

Give each Connection a shared_ptr "alive" token that disconnect handlers hold
a weak_ptr to and check before running, so a handler is skipped once its
Connection is gone. Having this check also enables the simplifications
described below.

Previously each Connection kept its disconnect handlers in its own
kj::TaskSet, and when the network disconnected it moved a handler onto the
shared event loop TaskSet with kj::evalLater. Destroying the Connection
destroyed that per-connection TaskSet, canceling a still-pending handler --
but a handler already moved onto the shared TaskSet was no longer canceled
and could run after the Connection was gone. (The evalLater step existed only
to avoid a "promise callback destroyed itself" error when a handler deletes
its own Connection, which the per-connection TaskSet made possible.)

With the token doing the cancellation, neither the per-connection TaskSet nor
the evalLater step is needed, and both are removed.

Co-Authored-By: Enoch Azariah <enirox001@gmail.com>
This change was written with Claude Fable 5.

Fix a race between a thread exiting after making IPC calls and its
connection being destroyed on the event loop thread, which could destroy
the same ProxyClient<Thread> object twice. ~ThreadContext destroyed the
thread-local request_threads/callback_threads maps with no locking while
the SetThread cleanup callback run by ~Connection erased entries from the
same maps. When both ran at once, each side destroyed the entry's
ProxyClient<Thread>, and ~Connection then ran the ProxyClientBase
disconnect callback on the freed map node (heap-use-after-free, then a
glibc "double free or corruption" abort).

Fix by making map entry removal decide which side destroys an entry:
~ThreadContext and the SetThread callback each remove entries under
Waiter::m_mutex before destroying them, and a side that finds an entry
already gone leaves it to the other. See the code comments for why the
entries are destroyed with the mutex released.

Add a regression test, "Thread exiting while its connection is
destroyed", which uses a new testing_hook_thread_client_destroy hook to
interleave the two sides deterministically and fails on every run
without the fix.

The race is long-standing and reachable on master via connections
created by ConnectStream, whose onDisconnect handler deletes the client
Connection on the event loop thread when the peer disconnects.

This change was written with Claude Fable 5.1.

Split connection teardown out of ~Connection into an idempotent disconnect()
method, with the destructor delegating to it. For existing callers, this is a
behavior-neutral refactor: the same steps run in the same order on destruction.

Having a separate disconnect() method allows severing a connection while
keeping the Connection object alive, which the next commits use to let
shutdown code wait for in-flight server call bodies to finish after a
disconnect (bitcoin/bitcoin#35845). Two details are new in the disconnect()
method which were not present in the destructor method:

- disconnect() expires the m_alive token explicitly, where previously it was
  expired implicitly by member destruction. This keeps onRemoteDisconnect able
  to distinguish a local disconnect from a remote one when a connection is
  severed without destroying the object (see the disconnect() code comment).

- disconnect() explicitly releases m_thread_pool and m_thread_map so worker
  thread teardown happens at disconnect time whether or not the object is
  destroyed right away. Previously this happened implicitly during member
  destruction.

This change was written with Claude Fable 5.

Add a per-connection ServerObjectTracker counting live ProxyServer objects,
incremented in the ProxyServerBase constructor and decremented in its
destructor, with Connection::waitDrained() blocking until the count reaches
zero and Connection::pendingServerObjects() exposing it for logging.

Disconnecting a connection cancels the KJ promise of an in-flight call, but a
C++ server method body already dispatched to a worker thread runs to
completion. Counting live server objects turns Cap'n Proto's object lifetime
rules into a usable quiescence signal: a ProxyServer object is not destroyed
until its outstanding calls finish (the target capability is kept alive for
the duration of a call and pinned by post()/PassField via thisCap()), so
after disconnect() the count drains to zero exactly when no server call body
is still executing. Waiting for that lets shutdown code avoid freeing
application state that a still-running call body dereferences
(bitcoin/bitcoin#35845).

The tracker is held via shared_ptr by the Connection and by every
ProxyServer object because objects kept alive by in-flight calls can outlive
the Connection on some teardown paths (see ~ProxyServerBase), and their
destructors must decrement state that is still valid. It must be declared
before m_rpc_system, whose construction creates the bootstrap server object
that registers itself with the tracker.

This change was written with Claude Fable 5.

Add a deterministic mptest regression test for bitcoin/bitcoin#35845: hold a
server method body in flight on a worker thread, call
Connection::disconnect(), and assert that Connection::waitDrained() blocks
until the body finishes and its server object is destroyed. Also covers
destroying an already-disconnected connection (~Connection noticing
disconnect() has run).

This change was written with Claude Fable 5.
Co-Authored-By: Enoch Azariah <enirox001@gmail.com>

Add an accessor and a Connections type alias for the EventLoop's
list of incoming connections, so future code can be simplified to
locate a specific connection without directly accessing the private
list or embedding a Connection object itself.

This change was written with Claude Sonnet 5.

Add a _Serve/ServeStream overload accepting the init object as a
shared_ptr, so callers can transfer ownership instead of always
passing a reference to an object they keep alive themselves. Existing
reference-taking callers keep working through a thin overload that
wraps the reference in a shared_ptr with an empty deleter.

Also return the constructed ProxyServer along with an iterator to its
Connection in loop.m_incoming_connections, so callers can look up or
erase the connection later without embedding a Connection object
themselves.

This change was written with Claude Sonnet 5.

Give ServeStream and ConnectStream a destroy_connection parameter,
defaulting to true, so callers can opt out of automatic connection
teardown and manage the Connection's lifetime themselves instead.
ServeStream gates the internal disconnect handler's list erase on the
parameter; ConnectStream just forwards it to the existing
ProxyClientBase parameter of the same name.

This lets callers that need to keep a connection alive past a
disconnect notification (e.g. to let in-flight server calls finish)
use these helpers instead of constructing a Connection manually.

This change was written with Claude Sonnet 5.

Replace TestSetup's manual Connection construction with
ServeStream/ConnectStream, following the same pattern already used in
Bitcoin Core's own IPC test and fuzz code. This drops server_on_disconnect
entirely: ServeStream's destroy_connection parameter now controls whether
a remote disconnect erases the server Connection, so the only test that
needed to suppress that (the mp#348 getResults race test) just constructs
TestSetup with server_owns_connection=false instead of overriding a
callback afterward.

This change was written with Claude Sonnet 5.

Replace manual Connection construction in protocol.cpp, ipc_tests.cpp,
and fuzz/ipc.cpp with the ServeStream/ConnectStream/incomingConnections
helpers added in preceding commits. None of these call sites need
anything the helpers don't already provide: destroy_connection defaults
to true and the constructed Connection/ProxyServer are not otherwise
inspected.

This change was written with Claude Sonnet 5.

Fix bitcoin/bitcoin#35845, an assertion failure in MinerImpl::chainman()
during shutdown of an IPC-mining node.

Shutdown() calls disconnectIncoming() before node.chainman.reset().
Disconnecting cancels the KJ promise of an in-flight IPC server call, but a
C++ server method body already dispatched to a libmultiprocess worker thread
is not interrupted and runs to completion. 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).

Make disconnectIncoming() disconnect the non-parent incoming connections,
wait off the event loop thread for their in-flight server call bodies to
finish (Connection::waitDrained), and only then destroy them and return, so
Shutdown() frees node state only once no server code is running. Log when
the wait actually blocks so a shutdown hang here is diagnosable.

No wait is needed for calls parked in waitTipChanged()/waitNext():
Interrupt() runs before Shutdown() and notifies m_tip_block_cv after setting
the shutdown signal, so those return before disconnectIncoming() runs.

Intentional limitations, to keep the fix narrow: m_impl destructors
scheduled on the async cleanup thread are not waited for, the kept-open
parent connection is not drained, and new incoming connections can still be
accepted during shutdown (preventing that needs a listener API, proposed in
bitcoin-core/libmultiprocess#269).

This change was written with Claude Fable 5.

WARNING: DO NOT MERGE — this draft PR modifies the libmultiprocess
subtree directly (intentionally, for testing purposes) which causes
the subtree link check to fail. Disabling the check here to avoid
masking unrelated lint failures while the PR is open for testing.

Restore the omitted entry before any merge attempt.

Note: removing src/ipc/libmultiprocess from get_subtrees() also removes
it from get_pathspecs_default_excludes(), so all other lint checks
(trailing newline, whitespace, etc.) now run on files in that directory
as well. This is why src/ipc/libmultiprocess/include/mp/type-unordered-set.h
needs a trailing newline added here even though it is not otherwise
modified by this PR: it has always been missing the newline, but was
previously hidden from the lint check by the subtree exclusion.

This change was written with Claude Sonnet 4.6.