ipc: drain in-flight server calls before shutdown frees node state

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

Author: ryanofsky

src/ipc/capnp/protocol.cpp

@@ -26,6 +26,7 @@
 #include <sys/socket.h>
 #include <system_error>
 #include <thread>
+#include <vector>
 
 namespace ipc {
 namespace capnp {
@@ -108,12 +109,56 @@ class CapnpProtocol : public Protocol
     void disconnectIncoming() override
     {
         if (!m_loop) return;
-        // Delete incoming connections, except the connection to a parent
+        // Disconnect incoming connections, except the connection to a parent
         // process (if there is one), since a parent process should be able to
         // monitor and control this process, even during shutdown.
+        //
+        // Disconnecting cancels the KJ promise of any call in progress, but a
+        // server method body that was already dispatched to a worker thread is
+        // NOT interrupted and runs to completion. So before destroying the
+        // connections and returning, wait for in-flight call bodies to finish
+        // (mp::Connection::waitDrained). Without this wait, a still-running
+        // body (e.g. Mining.checkBlock) could dereference node state that the
+        // caller (Shutdown) frees right after this function returns, causing
+        // https://github.com/bitcoin/bitcoin/issues/35845.
+        //
+        // The wait happens off the event loop thread, between two sync()
+        // calls: call bodies need the event loop to deliver their results, so
+        // blocking the loop until they finish would deadlock.
+        //
+        // Limitations, intentional to keep this narrow (see #35845 discussion):
+        // - Only server call *bodies* are drained. ProxyServer m_impl
+        //   destructors scheduled via EventLoop::addAsyncCleanup() run later
+        //   on the async cleanup thread and are not waited for. (One of
+        //   several reasons IPC object implementations should not have
+        //   nontrivial destructors.)
+        // - Only the removed connections are drained; the kept-open parent
+        //   connection is not (that would require pausing new calls on it).
+        // - New incoming connections can still be accepted while this runs;
+        //   preventing that requires an API to stop the listener, proposed in
+        //   https://github.com/bitcoin-core/libmultiprocess/pull/269.
+        std::vector<mp::Connection::Tracker> disconnected;
         m_loop->sync([&] {
-            m_loop->m_incoming_connections.remove_if([this](mp::Connection& c) { return &c != m_parent_connection; });
+            for (mp::Connection& c : m_loop->incomingConnections()) {
+                if (&c != m_parent_connection) {
+                    disconnected.push_back(c.tracker());
+                    c.disconnect();
+                }
+            }
         });
+        // After disconnect(), the remaining counted server objects are exactly
+        // the ones kept alive by in-flight calls (idle ones are garbage
+        // collected synchronously by the disconnect), so a nonzero count means
+        // the wait below will actually block. Log when that happens so a
+        // shutdown hang here is diagnosable.
+        size_t pending{0};
+        for (const auto& connection : disconnected) pending += connection->pendingServerObjects();
+        if (pending > 0) {
+            LogInfo("ipc: Waiting for %d server object(s) in use by in-flight IPC call(s) on %d connection(s) to be freed",
+                    pending, disconnected.size());
+        }
+        for (const auto& tracker : disconnected) tracker->waitDrained();
+        if (pending > 0) LogInfo("ipc: Incoming IPC connections drained");
     }
     mp::Stream makeStream(mp::SocketId socket) override
     {