Update libmultiprocess subtree

src/ipc/libmultiprocess/CMakeLists.txt

@@ -90,13 +90,6 @@ if(MP_ENABLE_CLANG_TIDY)
     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()
 
 option(MP_ENABLE_IWYU "Run include-what-you-use with the compiler." OFF)
@@ -111,6 +104,15 @@ if(MP_ENABLE_IWYU)
   endif()
 endif()
 
+if(MP_ENABLE_CLANG_TIDY OR MP_ENABLE_IWYU)
+  # 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()
+
 include("cmake/compat_config.cmake")
 include("cmake/pthread_checks.cmake")
 include(GNUInstallDirs)
@@ -137,6 +139,10 @@ 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
+# Build-graph node for generated headers. This lets targets that include
+# the headers order themselves after generation without depending on the
+# library target that also uses them.
+add_custom_target(mp_headers DEPENDS ${MP_PROXY_HDRS})
 
 # util library
 add_library(mputil OBJECT src/mp/util.cpp)

src/ipc/libmultiprocess/cmake/TargetCapnpSources.cmake

@@ -27,10 +27,8 @@ Arguments:
 
       #include <schema.capnp.h>
 
-    The specified include_prefix should be ${CMAKE_SOURCE_DIR} or a
-    subdirectory of it to include files relative to the project root. It can
-    be ${CMAKE_CURRENT_SOURCE_DIR} to include files relative to the current
-    source directory.
+    Pass ${CMAKE_CURRENT_SOURCE_DIR} or a subdirectory of it to include files
+    relative to the current source directory (the typical usage).
 
 Additional Unnamed Arguments:
 
@@ -43,12 +41,19 @@ Optional Keyword Arguments:
   IMPORT_PATHS: Specifies additional directories to search for imported
     `.capnp` files.
 
+  ONLY_CAPNP: If specified, only the Cap'n Proto serialization files
+    (`.capnp.h`, `.capnp.c++`) are generated and compiled. The mpgen proxy
+    files (`.capnp.proxy-client.c++`, `.capnp.proxy-server.c++`,
+    `.capnp.proxy-types.c++`, etc.) are skipped. Useful when you need
+    Cap'n Proto serialization without the multiprocess RPC proxy
+    infrastructure.
+
 Example:
   # Assuming `my_library` is a target and `lib/` contains `.capnp` schema
   # files with imports from `include/`.
-  target_capnp_sources(my_library "${CMAKE_SOURCE_DIR}"
+  target_capnp_sources(my_library "${CMAKE_CURRENT_SOURCE_DIR}"
                        lib/schema1.capnp lib/schema2.capnp
-                       IMPORT_PATHS ${CMAKE_SOURCE_DIR}/include)
+                       IMPORT_PATHS ${CMAKE_CURRENT_SOURCE_DIR}/include)
 
 #]=]
 
@@ -98,8 +103,8 @@ function(target_capnp_sources target include_prefix)
 
   # Translate include_prefix from a source path to a binary path and add it as a
   # target include directory.
-  set(build_include_prefix ${CMAKE_BINARY_DIR})
-  file(RELATIVE_PATH relative_path ${CMAKE_SOURCE_DIR} ${include_prefix})
+  set(build_include_prefix ${CMAKE_CURRENT_BINARY_DIR})
+  file(RELATIVE_PATH relative_path ${CMAKE_CURRENT_SOURCE_DIR} ${include_prefix})
   if(relative_path)
     string(APPEND build_include_prefix "/" "${relative_path}")
   endif()

src/ipc/libmultiprocess/doc/design.md

@@ -15,7 +15,7 @@ Libmultiprocess acts as a pure wrapper or layer over the underlying protocol. Cl
 
 ## Core Architecture
 
-The `ProxyClient` and `ProxyServer` generated classes are not directly exposed to the user, as described in [usage.md](usage.md). Instead, they wrap C++ interfaces and appear to the user as pointers to an interface. They are first instantiated when calling `ConnectStream` and `ServeStream` respectively for creating the `InitInterface`. These methods establish connections through sockets, internally creating `Connection` objects wrapping a `capnp::RpcSystem` configured for client and server mode respectively.
+The `ProxyServer` generated class is not directly exposed to the user. The `ProxyClient` generated class inherits from the C++ interface class, so user code interacts with it through the abstract interface type, and the `ProxyClient` type itself is generally not visible or accessible without a cast. `ConnectStream` returns a `unique_ptr<ProxyClient<InitInterface>>` as an exception for the root Init interface, but even there users typically treat it as a pointer to the abstract `InitInterface`. For all interfaces returned by Init methods (e.g., `Printer`, `Calculator`), the return type is the abstract class pointer, hiding the underlying `ProxyClient` entirely. They are first instantiated when calling `ConnectStream` and `ServeStream` respectively for creating the `InitInterface`. These methods establish connections through sockets, internally creating `Connection` objects wrapping a `capnp::RpcSystem` configured for client and server mode respectively.
 
 The `InitInterface` interface will typically have methods which return other interfaces, giving the connecting process the ability to call other functions in the serving process. Interfaces can also have methods accepting other interfaces as parameters, giving serving processes the ability to call back and invoke functions in connecting processes. Creating new interfaces does not create new connections, and typically many interface objects will share the same connection.
 
@@ -134,6 +134,8 @@ sequenceDiagram
     SF1->>serverInvoke: return
 ```
 
+Destroy methods are a special case: a capnp interface can declare a `destroy` method that is handled by `ServerDestroy` instead of `ServerCall`. Rather than dispatching through the `ServerField`/`ServerRet`/`ServerCall` chain to a C++ interface method, `ServerDestroy` calls `invokeDestroy()` on the `ProxyServer`, which resets `m_impl` and runs any registered cleanup functions, giving the client a way to synchronously destroy the wrapped object on the server side.
+
 ## Advanced Features
 
 ### Callbacks

src/ipc/libmultiprocess/doc/versions.md

@@ -7,9 +7,14 @@ Library versions are tracked with simple
 Versioning policy is described in the [version.h](../include/mp/version.h)
 include.
 
-## v10
+## v11
 - Current unstable version.
 
+## [v10.0](https://github.com/bitcoin-core/libmultiprocess/commits/v10.0)
+- Increases spawn test timeout to avoid spurious failures.
+- Uses `throwRecoverableException` instead of raw `throw` to improve runtime error messages in macOS builds.
+- Used in Bitcoin Core master branch, pulled in by [#34977](https://github.com/bitcoin/bitcoin/pull/34977). Also pulled into Bitcoin Core 31.x stable branch by [#35028](https://github.com/bitcoin/bitcoin/pull/35028).
+
 ## [v9.0](https://github.com/bitcoin-core/libmultiprocess/commits/v9.0)
 - Fixes race conditions where worker thread could be used after destruction, where getParams() could be called after request cancel, and where m_on_cancel could be called after request finishes.
 - Adds `CustomHasField` hook to map Cap'n Proto null values to C++ null values.

src/ipc/libmultiprocess/include/mp/proxy-io.h

@@ -384,7 +384,7 @@ struct Waiter
     }
 
     template <class Predicate>
-    void wait(Lock& lock, Predicate pred)
+    void wait(Lock& lock, Predicate pred) MP_REQUIRES(m_mutex)
     {
         m_cv.wait(lock.m_lock, [&]() MP_REQUIRES(m_mutex) {
             // Important for this to be "while (m_fn)", not "if (m_fn)" to avoid
@@ -410,7 +410,7 @@ struct Waiter
     //! EventLoop::m_mutex as long as Waiter::mutex is locked first and
     //! EventLoop::m_mutex is locked second.
     Mutex m_mutex;
-    std::condition_variable m_cv;
+    std::condition_variable m_cv MP_GUARDED_BY(m_mutex);
     std::optional<kj::Function<void()>> m_fn MP_GUARDED_BY(m_mutex);
 };
 
@@ -511,6 +511,7 @@ ProxyClientBase<Interface, Impl>::ProxyClientBase(typename Interface::Client cli
     : m_client(std::move(client)), m_context(connection)
 
 {
+    MP_LOG(*m_context.loop, Log::Debug) << "Creating " << CxxTypeName(*this) << " " << this;
     // Handler for the connection getting destroyed before this client object.
     auto disconnect_cb = m_context.connection->addSyncCleanup([this]() {
         // Release client capability by move-assigning to temporary.
@@ -538,7 +539,12 @@ ProxyClientBase<Interface, Impl>::ProxyClientBase(typename Interface::Client cli
         // the remote object, waiting for it to be deleted server side. If the
         // capnp interface does not define a destroy method, this will just call
         // an empty stub defined in the ProxyClientBase class and do nothing.
-        Sub::destroy(*this);
+        // Exceptions are caught and logged rather than propagated because
+        // ~ProxyClientBase is noexcept and the peer may be gone by the time
+        // this runs.
+        if (kj::runCatchingExceptions([&]{ Sub::destroy(*this); }) != nullptr) {
+            MP_LOG(*m_context.loop, Log::Warning) << "Remote destroy call failed during cleanup. Continuing.";
+        }
 
         // FIXME: Could just invoke removed addCleanup fn here instead of duplicating code
         m_context.loop->sync([&]() {
@@ -567,13 +573,16 @@ ProxyClientBase<Interface, Impl>::ProxyClientBase(typename Interface::Client cli
 template <typename Interface, typename Impl>
 ProxyClientBase<Interface, Impl>::~ProxyClientBase() noexcept
 {
+    MP_LOG(*m_context.loop, Log::Debug) << "Cleaning up " << CxxTypeName(*this) << " " << this;
     CleanupRun(m_context.cleanup_fns);
+    MP_LOG(*m_context.loop, Log::Debug) << "Destroying " << CxxTypeName(*this) << " " << this;
 }
 
 template <typename Interface, typename Impl>
 ProxyServerBase<Interface, Impl>::ProxyServerBase(std::shared_ptr<Impl> impl, Connection& connection)
     : m_impl(std::move(impl)), m_context(&connection)
 {
+    MP_LOG(*m_context.loop, Log::Debug) << "Creating " << CxxTypeName(*this) << " " << this;
     assert(m_impl);
 }
 
@@ -592,6 +601,7 @@ ProxyServerBase<Interface, Impl>::ProxyServerBase(std::shared_ptr<Impl> impl, Co
 template <typename Interface, typename Impl>
 ProxyServerBase<Interface, Impl>::~ProxyServerBase()
 {
+    MP_LOG(*m_context.loop, Log::Debug) << "Cleaning up " << CxxTypeName(*this) << " " << this;
     if (m_impl) {
         // If impl is non-null at this point, it means no client is waiting for
         // the m_impl server object to be destroyed synchronously. This can
@@ -618,6 +628,7 @@ ProxyServerBase<Interface, Impl>::~ProxyServerBase()
         });
     }
     assert(m_context.cleanup_fns.empty());
+    MP_LOG(*m_context.loop, Log::Debug) << "Destroying " << CxxTypeName(*this) << " " << this;
 }
 
 //! If the capnp interface defined a special "destroy" method, as described the

src/ipc/libmultiprocess/include/mp/proxy-types.h

@@ -280,6 +280,17 @@ struct ListOutput<::capnp::List<T, kind>>
     // clang-format on
 };
 
+template <typename LocalType, typename Value, typename Output>
+void BuildList(TypeList<LocalType>, InvokeContext& invoke_context, Output&& output, Value&& value)
+{
+    auto list = output.init(value.size());
+    size_t i = 0;
+    for (const auto& elem : value) {
+        BuildField(TypeList<LocalType>(), invoke_context, ListOutput<typename decltype(list)::Builds>(list, i), elem);
+        ++i;
+    }
+}
+
 template <typename LocalType, typename Value, typename Output>
 void CustomBuildField(TypeList<LocalType>, Priority<0>, InvokeContext& invoke_context, Value&& value, Output&& output)
 {
@@ -644,17 +655,13 @@ struct CapRequestTraits<::capnp::Request<_Params, _Results>>
 template <typename Client>
 void clientDestroy(Client& client)
 {
-    if (client.m_context.connection) {
-        MP_LOG(*client.m_context.loop, Log::Debug) << "IPC client destroy " << typeid(client).name();
-    } else {
-        KJ_LOG(INFO, "IPC interrupted client destroy", typeid(client).name());
-    }
+    MP_LOG(*client.m_context.loop, Log::Debug) << "IPC client destroy " << CxxTypeName(client);
 }
 
 template <typename Server>
 void serverDestroy(Server& server)
 {
-    MP_LOG(*server.m_context.loop, Log::Debug) << "IPC server destroy " << typeid(server).name();
+    MP_LOG(*server.m_context.loop, Log::Debug) << "IPC server destroy " << CxxTypeName(server);
 }
 
 //! Entry point called by generated client code that looks like:
@@ -780,10 +787,11 @@ kj::Promise<void> serverInvoke(Server& server, CallContext& call_context, Fn fn)
     using Params = decltype(params);
     using Results = typename decltype(call_context.getResults())::Builds;
 
+    EventLoop& loop = *server.m_context.loop;
     int req = ++server_reqs;
-    MP_LOG(*server.m_context.loop, Log::Debug) << "IPC server recv request  #" << req << " "
-                                     << TypeName<typename Params::Reads>();
-    MP_LOG(*server.m_context.loop, Log::Trace) << "request data: "
+    MP_LOG(loop, Log::Debug) << "IPC server recv request  #" << req << " "
+        << TypeName<typename Params::Reads>();
+    MP_LOG(loop, Log::Trace) << "request data: "
         << LogEscape(params.toString(), server.m_context.loop->m_log_opts.max_chars);
 
     try {
@@ -799,16 +807,23 @@ kj::Promise<void> serverInvoke(Server& server, CallContext& call_context, Fn fn)
         // and waiting for it to complete.
         return ReplaceVoid([&]() { return fn.invoke(server_context, ArgList()); },
             [&]() { return kj::Promise<CallContext>(kj::mv(call_context)); })
-            .then([&server, req](CallContext call_context) {
-                MP_LOG(*server.m_context.loop, Log::Debug) << "IPC server send response #" << req << " " << TypeName<Results>();
-                MP_LOG(*server.m_context.loop, Log::Trace) << "response data: "
-                    << LogEscape(call_context.getResults().toString(), server.m_context.loop->m_log_opts.max_chars);
+            .then([&loop, req](CallContext call_context) {
+                MP_LOG(loop, Log::Debug) << "IPC server send response #" << req << " " << TypeName<Results>();
+                MP_LOG(loop, Log::Trace) << "response data: "
+                    << LogEscape(call_context.getResults().toString(), loop.m_log_opts.max_chars);
+            }).catch_([&loop, req](::kj::Exception&& e) -> kj::Promise<void> {
+                // Call failed for some reason. Cap'n Proto will try to send
+                // this error to the client as well, but it is good to log the
+                // failure early here and include the request number.
+                MP_LOG(loop, Log::Error) << "IPC server error request #" << req << " " << TypeName<Results>()
+                    << " " << kj::str("kj::Exception: ", e.getDescription()).cStr();
+                return kj::mv(e);
             });
     } catch (const std::exception& e) {
-        MP_LOG(*server.m_context.loop, Log::Error) << "IPC server unhandled exception: " << e.what();
+        MP_LOG(loop, Log::Error) << "IPC server unhandled exception: " << e.what();
         throw;
     } catch (...) {
-        MP_LOG(*server.m_context.loop, Log::Error) << "IPC server unhandled exception";
+        MP_LOG(loop, Log::Error) << "IPC server unhandled exception";
         throw;
     }
 }

src/ipc/libmultiprocess/include/mp/proxy.h

@@ -304,20 +304,31 @@ struct ProxyServerMethodTraits : public ProxyMethodTraits<MethodParams>
 {
 };
 
-static constexpr int FIELD_IN = 1;
-static constexpr int FIELD_OUT = 2;
-static constexpr int FIELD_OPTIONAL = 4;
-static constexpr int FIELD_REQUESTED = 8;
-static constexpr int FIELD_BOXED = 16;
+static constexpr int FIELD_IN = 1;        //!< See Accessor::in.
+static constexpr int FIELD_OUT = 2;       //!< See Accessor::out.
+static constexpr int FIELD_OPTIONAL = 4;  //!< See Accessor::optional.
+static constexpr int FIELD_REQUESTED = 8; //!< See Accessor::requested.
+static constexpr int FIELD_BOXED = 16;    //!< See Accessor::boxed.
 
 //! Accessor type holding flags that determine how to access a message field.
 template <typename Field, int flags>
 struct Accessor : public Field
 {
+    //! Field is present from the Cap'n Proto Params struct (client -> server).
     static const bool in = flags & FIELD_IN;
+    //! Field is present from the Cap'n Proto Results struct (server -> client).
     static const bool out = flags & FIELD_OUT;
+    //! Field has a companion has{Name} boolean field in the Cap'n Proto struct.
+    //! This is used to represent optional primitive values (e.g. C++
+    //! std::optional<int>) because Cap'n Proto doesn't allow primitive fields to
+    //! be unset.
     static const bool optional = flags & FIELD_OPTIONAL;
+    //! Results field has a companion want{Name} boolean field in the Params
+    //! struct. Used for optional output parameters (e.g. C++ int*) and set to
+    //! true if the caller passed a non-null pointer and wants the result.
     static const bool requested = flags & FIELD_REQUESTED;
+    //! Field is a Cap'n Proto pointer type (struct, list, text, data,
+    //! interface) as opposed to a primitive type (bool, int, float, enum).
     static const bool boxed = flags & FIELD_BOXED;
 };
 

src/ipc/libmultiprocess/include/mp/type-context.h

@@ -216,6 +216,40 @@ auto PassField(Priority<1>, TypeList<>, ServerContext& server_context, const Fn&
                     << "IPC server error request #" << req << ", missing thread to execute request";
                 throw std::runtime_error("invalid thread handle");
             }
+        }, [&loop, req](::kj::Exception&& e) -> kj::Promise<typename ServerContext::CallContext> {
+            // If you see the error "(remote):0: failed: remote exception:
+            // Called null capability" here, it probably means your Init class
+            // is missing a declaration like:
+            //
+            //   construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :Proxy.ThreadMap);
+            //
+            // which passes a ThreadMap reference from the client to the server,
+            // allowing the server to create threads to run IPC calls on the
+            // client, and also returns a ThreadMap reference from the server to
+            // the client, allowing the client to create threads on the server.
+            // (Typically the latter ThreadMap is used more often because there
+            // are more client-to-server calls.)
+            //
+            // If the other side of the connection did not previously get a
+            // ThreadMap reference from this side of the connection, when the
+            // other side calls `m_thread_map.makeThreadRequest()` in
+            // `BuildField` above, `m_thread_map` will be null, but that call
+            // will not fail immediately due to Cap'n Proto's request pipelining
+            // and delayed execution. Instead that call will return an invalid
+            // Thread reference, and when that reference is passed to this side
+            // of the connection as `thread_client` above, the
+            // `getLocalServer(thread_client)` call there will be the first
+            // thing to overtly fail, leading to an error here.
+            //
+            // Potentially there are also other things that could cause errors
+            // here, but this is the most likely cause.
+            //
+            // The log statement here is not strictly necessary since the same
+            // exception will also be logged in serverInvoke, but this logging
+            // may provide extra context that could be helpful for debugging.
+            MP_LOG(loop, Log::Info)
+                << "IPC server error request #" << req << " CapabilityServerSet<Thread>::getLocalServer call failed, did you forget to provide a ThreadMap to the client prior to this IPC call?";
+            return kj::mv(e);
         });
     // Use connection m_canceler object to cancel the result promise if the
     // connection is destroyed. (By default Cap'n Proto does not cancel requests