Merge bitcoin/bitcoin#32345: ipc: Handle unclean shutdowns better

2581258 ipc: Handle bitcoin-wallet disconnections (Ryan Ofsky)
2160995 ipc: Add Ctrl-C handler for spawned subprocesses (Ryan Ofsky)
0c28068 doc: Improve IPC interface comments (Ryan Ofsky)
7f65aac ipc: Avoid waiting for clients to disconnect when shutting down (Ryan Ofsky)
6eb09fd test: Add unit test coverage for Init and Shutdown code (Ryan Ofsky)
9a9fb19 ipc: Use EventLoopRef instead of addClient/removeClient (Ryan Ofsky)
e886c65 Squashed 'src/ipc/libmultiprocess/' changes from 27c7e8e5a581..b4120d34bad2 (Ryan Ofsky)

Pull request description:

  This PR fixes various problems when IPC connections are broken or hang which were reported in bitcoin-core/libmultiprocess#123, bitcoin-core/libmultiprocess#176, and bitcoin-core/libmultiprocess#182. The different fixes are described in commit messages.

  ---

  The first two commits of this PR update the libmultiprocess subtree including the following PRs:

  - bitcoin-core/libmultiprocess#181
  - bitcoin-core/libmultiprocess#179
  - bitcoin-core/libmultiprocess#160
  - bitcoin-core/libmultiprocess#184
  - bitcoin-core/libmultiprocess#187
  - bitcoin-core/libmultiprocess#186
  - bitcoin-core/libmultiprocess#192

  The subtree changes can be verified by running `test/lint/git-subtree-check.sh src/ipc/libmultiprocess` as described in [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees) and [lint instructions](https://github.com/bitcoin/bitcoin/tree/master/test/lint#git-subtree-checksh).

  The remaining commits are:

  - [`9a9fb19536fa` ipc: Use EventLoopRef instead of addClient/removeClient](bitcoin/bitcoin@9a9fb19)
  - [`6eb09fd6141f` test: Add unit test coverage for Init and Shutdown code](bitcoin/bitcoin@6eb09fd)
  - [`7f65aac78b95` ipc: Avoid waiting for clients to disconnect when shutting down](bitcoin/bitcoin@7f65aac)
  - [`0c28068ceb7b` doc: Improve IPC interface comments](bitcoin/bitcoin@0c28068)
  - [`216099591632` ipc: Add Ctrl-C handler for spawned subprocesses](bitcoin/bitcoin@2160995)
  - [`2581258ec200` ipc: Handle bitcoin-wallet disconnections](bitcoin/bitcoin@2581258)

  The new commits depend on the subtree update, and because the subtree update includes an incompatible API change, the "Use EventLoopRef" commit needs to be part of the same PR to avoid breaking the build. The other commits also make sense to merge at the same time because the bitcoin & libmultiprocess changes were written and tested together.

  ---

  This PR is part of the [process separation project](bitcoin/bitcoin#28722).

ACKs for top commit:
  Sjors:
    re-utACK 2581258
  josibake:
    code review ACK bitcoin/bitcoin@2581258
  pinheadmz:
    re-ACK 2581258

Tree-SHA512: 0095aa22d507803e2a2d46eff51fb6caf965cc0c97ccfa615bd97805d5d51e66a5b4b040640deb92896438b1fb9f6879847124c9d0e120283287bfce37b8d748

Author: fanquake

src/common/args.cpp

@@ -589,6 +589,14 @@ void ArgsManager::AddHiddenArgs(const std::vector<std::string>& names)
     }
 }
 
+void ArgsManager::ClearArgs()
+{
+    LOCK(cs_args);
+    m_settings = {};
+    m_available_args.clear();
+    m_network_only_args.clear();
+}
+
 void ArgsManager::CheckMultipleCLIArgs() const
 {
     LOCK(cs_args);

src/common/args.h

@@ -359,11 +359,7 @@ class ArgsManager
     /**
      * Clear available arguments
      */
-    void ClearArgs() {
-        LOCK(cs_args);
-        m_available_args.clear();
-        m_network_only_args.clear();
-    }
+    void ClearArgs();
 
     /**
      * Check CLI command args

src/init.cpp

@@ -33,6 +33,7 @@
 #include <interfaces/ipc.h>
 #include <interfaces/mining.h>
 #include <interfaces/node.h>
+#include <ipc/exception.h>
 #include <kernel/caches.h>
 #include <kernel/context.h>
 #include <key.h>
@@ -298,6 +299,14 @@ void Shutdown(NodeContext& node)
     StopREST();
     StopRPC();
     StopHTTPServer();
+    for (auto& client : node.chain_clients) {
+        try {
+            client->stop();
+        } catch (const ipc::Exception& e) {
+            LogDebug(BCLog::IPC, "Chain client did not disconnect cleanly: %s", e.what());
+            client.reset();
+        }
+    }
     StopMapPort();
 
     // Because these depend on each-other, we make sure that neither can be
@@ -370,8 +379,11 @@ void Shutdown(NodeContext& node)
             }
         }
     }
-    for (const auto& client : node.chain_clients) {
-        client->stop();
+
+    // If any -ipcbind clients are still connected, disconnect them now so they
+    // do not block shutdown.
+    if (interfaces::Ipc* ipc = node.init->ipc()) {
+        ipc->disconnectIncoming();
     }
 
 #ifdef ENABLE_ZMQ

src/interfaces/ipc.h

@@ -59,17 +59,20 @@ class Ipc
     //! true. If this is not a spawned child process, return false.
     virtual bool startSpawnedProcess(int argc, char* argv[], int& exit_status) = 0;
 
-    //! Connect to a socket address and make a client interface proxy object
-    //! using provided callback. connectAddress returns an interface pointer if
-    //! the connection was established, returns null if address is empty ("") or
-    //! disabled ("0") or if a connection was refused but not required ("auto"),
-    //! and throws an exception if there was an unexpected error.
+    //! Connect to a socket address and return a pointer to its Init interface.
+    //! Returns a non-null pointer if the connection was established, returns
+    //! null if address is empty ("") or disabled ("0") or if a connection was
+    //! refused but not required ("auto"), and throws an exception if there was
+    //! an unexpected error.
     virtual std::unique_ptr<Init> connectAddress(std::string& address) = 0;
 
-    //! Connect to a socket address and make a client interface proxy object
-    //! using provided callback. Throws an exception if there was an error.
+    //! Listen on a socket address exposing this process's init interface to
+    //! clients. Throws an exception if there was an error.
     virtual void listenAddress(std::string& address) = 0;
 
+    //! Disconnect any incoming connections that are still connected.
+    virtual void disconnectIncoming() = 0;
+
     //! Add cleanup callback to remote interface that will run when the
     //! interface is deleted.
     template<typename Interface>

src/ipc/capnp/protocol.cpp

@@ -41,10 +41,7 @@ class CapnpProtocol : public Protocol
 public:
     ~CapnpProtocol() noexcept(true)
     {
-        if (m_loop) {
-            std::unique_lock<std::mutex> lock(m_loop->m_mutex);
-            m_loop->removeClient(lock);
-        }
+        m_loop_ref.reset();
         if (m_loop_thread.joinable()) m_loop_thread.join();
         assert(!m_loop);
     };
@@ -68,9 +65,20 @@ class CapnpProtocol : public Protocol
         m_loop.emplace(exe_name, &IpcLogFn, &m_context);
         if (ready_fn) ready_fn();
         mp::ServeStream<messages::Init>(*m_loop, fd, init);
+        m_parent_connection = &m_loop->m_incoming_connections.back();
         m_loop->loop();
         m_loop.reset();
     }
+    void disconnectIncoming() override
+    {
+        if (!m_loop) return;
+        // Delete 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.
+        m_loop->sync([&] {
+            m_loop->m_incoming_connections.remove_if([this](mp::Connection& c) { return &c != m_parent_connection; });
+        });
+    }
     void addCleanup(std::type_index type, void* iface, std::function<void()> cleanup) override
     {
         mp::ProxyTypeRegister::types().at(type)(iface).cleanup_fns.emplace_back(std::move(cleanup));
@@ -83,10 +91,7 @@ class CapnpProtocol : public Protocol
         m_loop_thread = std::thread([&] {
             util::ThreadRename("capnp-loop");
             m_loop.emplace(exe_name, &IpcLogFn, &m_context);
-            {
-                std::unique_lock<std::mutex> lock(m_loop->m_mutex);
-                m_loop->addClient(lock);
-            }
+            m_loop_ref.emplace(*m_loop);
             promise.set_value();
             m_loop->loop();
             m_loop.reset();
@@ -95,7 +100,14 @@ class CapnpProtocol : public Protocol
     }
     Context m_context;
     std::thread m_loop_thread;
+    //! EventLoop object which manages I/O events for all connections.
     std::optional<mp::EventLoop> m_loop;
+    //! Reference to the same EventLoop. Increments the loop’s refcount on
+    //! creation, decrements on destruction. The loop thread exits when the
+    //! refcount reaches 0. Other IPC objects also hold their own EventLoopRef.
+    std::optional<mp::EventLoopRef> m_loop_ref;
+    //! Connection to parent, if this is a child process spawned by a parent process.
+    mp::Connection* m_parent_connection{nullptr};
 };
 } // namespace
 

src/ipc/interfaces.cpp

@@ -13,6 +13,7 @@
 #include <tinyformat.h>
 #include <util/fs.h>
 
+#include <csignal>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
@@ -26,6 +27,28 @@
 
 namespace ipc {
 namespace {
+#ifndef WIN32
+std::string g_ignore_ctrl_c;
+
+void HandleCtrlC(int)
+{
+    // (void)! needed to suppress -Wunused-result warning from GCC
+    (void)!write(STDOUT_FILENO, g_ignore_ctrl_c.data(), g_ignore_ctrl_c.size());
+}
+#endif
+
+void IgnoreCtrlC(std::string message)
+{
+#ifndef WIN32
+    g_ignore_ctrl_c = std::move(message);
+    struct sigaction sa{};
+    sa.sa_handler = HandleCtrlC;
+    sigemptyset(&sa.sa_mask);
+    sa.sa_flags = SA_RESTART;
+    sigaction(SIGINT, &sa, nullptr);
+#endif
+}
+
 class IpcImpl : public interfaces::Ipc
 {
 public:
@@ -53,6 +76,7 @@ class IpcImpl : public interfaces::Ipc
         if (!m_process->checkSpawned(argc, argv, fd)) {
             return false;
         }
+        IgnoreCtrlC(strprintf("[%s] SIGINT received — waiting for parent to shut down.\n", m_exe_name));
         m_protocol->serve(fd, m_exe_name, m_init);
         exit_status = EXIT_SUCCESS;
         return true;
@@ -86,6 +110,10 @@ class IpcImpl : public interfaces::Ipc
         int fd = m_process->bind(gArgs.GetDataDirNet(), m_exe_name, address);
         m_protocol->listen(fd, m_exe_name, m_init);
     }
+    void disconnectIncoming() override
+    {
+        m_protocol->disconnectIncoming();
+    }
     void addCleanup(std::type_index type, void* iface, std::function<void()> cleanup) override
     {
         m_protocol->addCleanup(type, iface, std::move(cleanup));

src/ipc/libmultiprocess/.clang-tidy

@@ -1,41 +1,40 @@
 Checks: '
 -*,
-bugprone-*,
--bugprone-easily-swappable-parameters,
--bugprone-exception-escape,
--bugprone-move-forwarding-reference,
--bugprone-narrowing-conversions,
--bugprone-reserved-identifier,
-misc-*,
--misc-non-private-member-variables-in-classes,
--misc-no-recursion,
--misc-unconventional-assign-operator,
--misc-unused-parameters,
--misc-use-anonymous-namespace,
-modernize-*,
--modernize-avoid-c-arrays,
--modernize-concat-nested-namespaces,
--modernize-deprecated-headers,
--modernize-use-nodiscard,
--modernize-use-trailing-return-type,
--modernize-use-using,
+bugprone-argument-comment,
+bugprone-move-forwarding-reference,
+bugprone-string-constructor,
+bugprone-use-after-move,
+bugprone-lambda-function-name,
+bugprone-unhandled-self-assignment,
+misc-unused-using-decls,
+misc-no-recursion,
+modernize-deprecated-headers,
+modernize-use-default-member-init,
+modernize-use-emplace,
+modernize-use-equals-default,
+modernize-use-noexcept,
+modernize-use-nullptr,
+modernize-use-starts-ends-with,
 performance-*,
 -performance-avoid-endl,
+-performance-enum-size,
+-performance-inefficient-string-concatenation,
+-performance-no-int-to-ptr,
 -performance-noexcept-move-constructor,
-readability-*,
--readability-braces-around-statements,
--readability-convert-member-functions-to-static,
--readability-else-after-return,
--readability-function-cognitive-complexity,
--readability-identifier-length,
--readability-implicit-bool-conversion,
--readability-inconsistent-declaration-parameter-name,
--readability-magic-numbers,
--readability-named-parameter,
--readability-uppercase-literal-suffix,
--readability-use-anyofallof,
+-performance-unnecessary-value-param,
+readability-const-return-type,
+readability-redundant-declaration,
+readability-redundant-string-init,
+clang-analyzer-core.*,
+-clang-analyzer-core.UndefinedBinaryOperatorResult,
+clang-analyzer-optin.core.*,
 '
+HeaderFilterRegex: '.'
+WarningsAsErrors: '*'
 CheckOptions:
- - key: modernize-use-override.IgnoreDestructors
-   value: true
-HeaderFilterRegex: 'example/calculator.h|example/init.h|example/printer.h|include/mp/proxy-io.h|include/mp/proxy-types.h|include/mp/proxy.h|include/mp/util.h|test/mp/test/foo-types.h|test/mp/test/foo.h'
+ - key: modernize-deprecated-headers.CheckHeaderFile
+   value: false
+ - key: performance-move-const-arg.CheckTriviallyCopyableMove
+   value: false
+ - key: bugprone-unhandled-self-assignment.WarnOnlyIfThisHasSuspiciousField
+   value: false

src/ipc/libmultiprocess/.github/workflows/ci.yml

@@ -0,0 +1,29 @@
+name: CI
+
+on:
+  push:
+  pull_request:
+
+jobs:
+  build:
+    runs-on: ubuntu-latest
+
+    strategy:
+      fail-fast: false
+      matrix:
+        config: [default, llvm, gnu32, sanitize]
+
+    name: build • ${{ matrix.config }}
+
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install Nix
+        uses: cachix/install-nix-action@v31       # 2025-05-27, from https://github.com/cachix/install-nix-action/tags
+        with:
+          nix_path: nixpkgs=channel:nixos-25.05   # latest release
+
+      - name: Run CI script
+        env:
+          CI_CONFIG: ci/configs/${{ matrix.config }}.bash
+        run: ci/scripts/run.sh

src/ipc/libmultiprocess/CMakeLists.txt

@@ -1,4 +1,4 @@
-# Copyright (c) 2019 The Bitcoin Core developers
+# Copyright (c) The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -15,16 +15,35 @@ include("cmake/compat_find.cmake")
 find_package(CapnProto REQUIRED)
 find_package(Threads REQUIRED)
 
-option(Libmultiprocess_ENABLE_CLANG_TIDY "Run clang-tidy with the compiler." OFF)
-if(Libmultiprocess_ENABLE_CLANG_TIDY)
+set(MPGEN_EXECUTABLE "" CACHE FILEPATH "If specified, should be full path to an external mpgen binary to use rather than the one built internally.")
+
+option(MP_ENABLE_CLANG_TIDY "Run clang-tidy with the compiler." OFF)
+if(MP_ENABLE_CLANG_TIDY)
   find_program(CLANG_TIDY_EXECUTABLE NAMES clang-tidy)
   if(NOT CLANG_TIDY_EXECUTABLE)
-    message(FATAL_ERROR "Libmultiprocess_ENABLE_CLANG_TIDY is ON but clang-tidy is not found.")
+    message(FATAL_ERROR "MP_ENABLE_CLANG_TIDY is ON but clang-tidy is not found.")
   endif()
   set(CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_EXECUTABLE}")
+
+  # Workaround for nix from https://gitlab.kitware.com/cmake/cmake/-/issues/20912#note_793338
+  # Nix injects header paths via $NIX_CFLAGS_COMPILE; CMake tags these as
+  # CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES and omits them from the compile
+  # database, so clang-tidy, which ignores $NIX_CFLAGS_COMPILE, can't find capnp
+  # headers. Setting them as standard passes them to clang-tidy.
+  set(CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES ${CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES})
 endif()
 
-set(MPGEN_EXECUTABLE "" CACHE FILEPATH "If specified, should be full path to an external mpgen binary to use rather than the one built internally.")
+option(MP_ENABLE_IWYU "Run include-what-you-use with the compiler." OFF)
+if(MP_ENABLE_IWYU)
+  find_program(IWYU_EXECUTABLE NAMES include-what-you-use iwyu)
+  if(NOT IWYU_EXECUTABLE)
+    message(FATAL_ERROR "MP_ENABLE_IWYU is ON but include-what-you-use was not found.")
+  endif()
+  set(CMAKE_CXX_INCLUDE_WHAT_YOU_USE "${IWYU_EXECUTABLE};-Xiwyu;--error")
+  if(DEFINED ENV{IWYU_MAPPING_FILE})
+    list(APPEND CMAKE_CXX_INCLUDE_WHAT_YOU_USE "-Xiwyu" "--mapping_file=$ENV{IWYU_MAPPING_FILE}")
+  endif()
+endif()
 
 include("cmake/compat_config.cmake")
 include("cmake/pthread_checks.cmake")
@@ -51,6 +70,7 @@ configure_file(include/mp/config.h.in "${CMAKE_CURRENT_BINARY_DIR}/include/mp/co
 
 # Generated C++ Capn'Proto schema files
 capnp_generate_cpp(MP_PROXY_SRCS MP_PROXY_HDRS include/mp/proxy.capnp)
+set_source_files_properties("${MP_PROXY_SRCS}" PROPERTIES SKIP_LINTING TRUE) # Ignored before cmake 3.27
 
 # util library
 add_library(mputil OBJECT src/mp/util.cpp)

src/ipc/libmultiprocess/ci/README.md

@@ -0,0 +1,25 @@
+### CI quick-reference
+
+All CI is just bash and nix.
+
+* **Workflow**:
+  -  `.github/workflows/ci.yml` – lists the jobs (`default`, `llvm`, …).
+* **Scripts**:
+  - `ci/scripts/run.sh` – spins up the Nix shell then calls…
+  - `ci/scripts/ci.sh` – …to configure, build, and test.
+* **Configuration**:
+  - `ci/configs/*.sh` – defines flags for each job.
+  - `shell.nix` – defines build environment (compilers, tools, libraries).
+* **Build directories**:
+  - `build-*/` – separate build directories (like `build-default`, `build-llvm`) will be created for each job.
+
+To run jobs locally:
+
+```bash
+CI_CONFIG=ci/configs/default.bash  ci/scripts/run.sh
+CI_CONFIG=ci/configs/llvm.bash     ci/scripts/run.sh
+CI_CONFIG=ci/configs/gnu32.bash    ci/scripts/run.sh
+CI_CONFIG=ci/configs/sanitize.bash ci/scripts/run.sh
+```
+
+By default CI jobs will reuse their build directories. `CI_CLEAN=1` can be specified to delete them before running instead.