From c1aecbcad73ced9496b6d986f281c6635715bb01 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 15 Apr 2026 09:41:50 -0400 Subject: [PATCH 01/19] doc: Bump version 11 > 12 --- doc/versions.md | 6 +++++- include/mp/version.h | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/versions.md b/doc/versions.md index 3cfa28e3..14bd8ad8 100644 --- a/doc/versions.md +++ b/doc/versions.md @@ -7,8 +7,12 @@ Library versions are tracked with simple Versioning policy is described in the [version.h](../include/mp/version.h) include. -## v11 +## v12 - Current unstable version. +- Adds support for nonunix platforms, making API changes that are not backwards compatible. + +## [v11.0](https://github.com/bitcoin-core/libmultiprocess/commits/v11.0) +- Improves debug output if EventLoop::post callback fails. ## [v10.0](https://github.com/bitcoin-core/libmultiprocess/commits/v10.0) - Increases spawn test timeout to avoid spurious failures. diff --git a/include/mp/version.h b/include/mp/version.h index 423ed460..4587a288 100644 --- a/include/mp/version.h +++ b/include/mp/version.h @@ -24,7 +24,7 @@ //! pointing at the prior merge commit. The /doc/versions.md file should also be //! updated, noting any significant or incompatible changes made since the //! previous version. -#define MP_MAJOR_VERSION 11 +#define MP_MAJOR_VERSION 12 //! Minor version number. Should be incremented in stable branches after //! backporting changes. The /doc/versions.md file should also be updated to From 5f578bbce2cc0b68a20f9657027d79bd4dad89df Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 30 Apr 2025 08:39:29 -0400 Subject: [PATCH 02/19] util, refactor: Add ProcessId type alias and use it Add ProcessId = int type alias and apply it to WaitProcess, SpawnProcess (pid output argument), and callers. ProcessId type will be different on windows so this provides more portability. --- example/example.cpp | 2 +- include/mp/util.h | 6 ++++-- src/mp/util.cpp | 2 +- test/mp/test/spawn_tests.cpp | 10 +++++++--- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/example/example.cpp b/example/example.cpp index 38313977..af727516 100644 --- a/example/example.cpp +++ b/example/example.cpp @@ -25,7 +25,7 @@ namespace fs = std::filesystem; static auto Spawn(mp::EventLoop& loop, const std::string& process_argv0, const std::string& new_exe_name) { - int pid; + mp::ProcessId pid; const int fd = mp::SpawnProcess(pid, [&](int fd) -> std::vector { fs::path path = process_argv0; path.remove_filename(); diff --git a/include/mp/util.h b/include/mp/util.h index 380a142f..8fc19160 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -255,6 +255,8 @@ std::string ThreadName(const char* exe_name); //! errors in python unit tests. std::string LogEscape(const kj::StringTree& string, size_t max_size); +using ProcessId = int; + //! Callback type used by SpawnProcess below. using FdToArgsFn = std::function(int fd)>; @@ -265,7 +267,7 @@ using FdToArgsFn = std::function(int fd)>; //! It must not rely on child pid/state, and must return the command line //! arguments that should be used to execute the process. Embed the remote file //! descriptor number in whatever format the child process expects. -int SpawnProcess(int& pid, FdToArgsFn&& fd_to_args); +int SpawnProcess(ProcessId& pid, FdToArgsFn&& fd_to_args); //! Call execvp with vector args. //! Not safe to call in a post-fork child of a multi-threaded process. @@ -273,7 +275,7 @@ int SpawnProcess(int& pid, FdToArgsFn&& fd_to_args); void ExecProcess(const std::vector& args); //! Wait for a process to exit and return its exit code. -int WaitProcess(int pid); +int WaitProcess(ProcessId pid); inline char* CharCast(char* c) { return c; } inline char* CharCast(unsigned char* c) { return (char*)c; } diff --git a/src/mp/util.cpp b/src/mp/util.cpp index 463947b9..d02aa0e1 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -183,7 +183,7 @@ void ExecProcess(const std::vector& args) } } -int WaitProcess(int pid) +int WaitProcess(ProcessId pid) { int status; if (::waitpid(pid, &status, /*options=*/0) != pid) { diff --git a/test/mp/test/spawn_tests.cpp b/test/mp/test/spawn_tests.cpp index a14e50e2..e3027552 100644 --- a/test/mp/test/spawn_tests.cpp +++ b/test/mp/test/spawn_tests.cpp @@ -18,6 +18,8 @@ #include #include +namespace mp { +namespace test { namespace { constexpr auto FAILURE_TIMEOUT = std::chrono::seconds{30}; @@ -25,7 +27,7 @@ constexpr auto FAILURE_TIMEOUT = std::chrono::seconds{30}; // Poll for child process exit using waitpid(..., WNOHANG) until the child exits // or timeout expires. Returns true if the child exited and status_out was set. // Returns false on timeout or error. -static bool WaitPidWithTimeout(int pid, std::chrono::milliseconds timeout, int& status_out) +static bool WaitPidWithTimeout(ProcessId pid, std::chrono::milliseconds timeout, int& status_out) { const auto deadline = std::chrono::steady_clock::now() + timeout; while (std::chrono::steady_clock::now() < deadline) { @@ -86,8 +88,8 @@ KJ_TEST("SpawnProcess does not run callback in child") control_cv.notify_one(); }); - int pid{-1}; - const int fd{mp::SpawnProcess(pid, [&](int child_fd) -> std::vector { + ProcessId pid{-1}; + const int fd{SpawnProcess(pid, [&](int child_fd) -> std::vector { // If this callback runs in the post-fork child, target_mutex appears // locked forever (the owning thread does not exist), so this deadlocks. std::lock_guard g(target_mutex); @@ -110,3 +112,5 @@ KJ_TEST("SpawnProcess does not run callback in child") KJ_EXPECT(exited, "Timeout waiting for child process to exit"); KJ_EXPECT(WIFEXITED(status) && WEXITSTATUS(status) == 0); } +} // namespace test +} // namespace mp From 0ec153445ac06aaf010c40da700759f83c674ec2 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 30 Apr 2025 08:39:29 -0400 Subject: [PATCH 03/19] util, refactor: Add SocketId type alias and use it Add SocketId = int and SocketError = -1 type aliases and apply SocketId to SpawnProcess (return type and callback parameter) and callers. SocketId type will be different on Windows, so this provides more portability. Co-authored-by: Sjors Provoost --- example/calculator.cpp | 2 +- example/example.cpp | 2 +- example/printer.cpp | 2 +- include/mp/proxy-io.h | 4 ++-- include/mp/util.h | 6 ++++-- src/mp/proxy.cpp | 10 +++++----- src/mp/util.cpp | 4 ++-- test/mp/test/spawn_tests.cpp | 2 +- 8 files changed, 17 insertions(+), 15 deletions(-) diff --git a/example/calculator.cpp b/example/calculator.cpp index 86ce388b..cfaf785c 100644 --- a/example/calculator.cpp +++ b/example/calculator.cpp @@ -51,7 +51,7 @@ int main(int argc, char** argv) std::cout << "Usage: mpcalculator \n"; return 1; } - int fd; + mp::SocketId fd; if (std::from_chars(argv[1], argv[1] + strlen(argv[1]), fd).ec != std::errc{}) { std::cerr << argv[1] << " is not a number or is larger than an int\n"; return 1; diff --git a/example/example.cpp b/example/example.cpp index af727516..6a5bdc93 100644 --- a/example/example.cpp +++ b/example/example.cpp @@ -26,7 +26,7 @@ namespace fs = std::filesystem; static auto Spawn(mp::EventLoop& loop, const std::string& process_argv0, const std::string& new_exe_name) { mp::ProcessId pid; - const int fd = mp::SpawnProcess(pid, [&](int fd) -> std::vector { + const mp::SocketId fd = mp::SpawnProcess(pid, [&](mp::SocketId fd) -> std::vector { fs::path path = process_argv0; path.remove_filename(); path.append(new_exe_name); diff --git a/example/printer.cpp b/example/printer.cpp index 9150d59b..ec749d83 100644 --- a/example/printer.cpp +++ b/example/printer.cpp @@ -44,7 +44,7 @@ int main(int argc, char** argv) std::cout << "Usage: mpprinter \n"; return 1; } - int fd; + mp::SocketId fd; if (std::from_chars(argv[1], argv[1] + strlen(argv[1]), fd).ec != std::errc{}) { std::cerr << argv[1] << " is not a number or is larger than an int\n"; return 1; diff --git a/include/mp/proxy-io.h b/include/mp/proxy-io.h index 092ea42e..a1c4cd4f 100644 --- a/include/mp/proxy-io.h +++ b/include/mp/proxy-io.h @@ -309,10 +309,10 @@ class EventLoop std::optional m_async_fns MP_GUARDED_BY(m_mutex); //! Pipe read handle used to wake up the event loop thread. - int m_wait_fd = -1; + SocketId m_wait_fd = SocketError; //! Pipe write handle used to wake up the event loop thread. - int m_post_fd = -1; + SocketId m_post_fd = SocketError; //! Number of clients holding references to ProxyServerBase objects that //! reference this event loop. diff --git a/include/mp/util.h b/include/mp/util.h index 8fc19160..56667bb4 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -256,9 +256,11 @@ std::string ThreadName(const char* exe_name); std::string LogEscape(const kj::StringTree& string, size_t max_size); using ProcessId = int; +using SocketId = int; +constexpr SocketId SocketError{-1}; //! Callback type used by SpawnProcess below. -using FdToArgsFn = std::function(int fd)>; +using FdToArgsFn = std::function(SocketId fd)>; //! Spawn a new process that communicates with the current process over a socket //! pair. Returns pid through an output argument, and file descriptor for the @@ -267,7 +269,7 @@ using FdToArgsFn = std::function(int fd)>; //! It must not rely on child pid/state, and must return the command line //! arguments that should be used to execute the process. Embed the remote file //! descriptor number in whatever format the child process expects. -int SpawnProcess(ProcessId& pid, FdToArgsFn&& fd_to_args); +SocketId SpawnProcess(ProcessId& pid, FdToArgsFn&& fd_to_args); //! Call execvp with vector args. //! Not safe to call in a post-fork child of a multi-threaded process. diff --git a/src/mp/proxy.cpp b/src/mp/proxy.cpp index 963050c3..4783e1f1 100644 --- a/src/mp/proxy.cpp +++ b/src/mp/proxy.cpp @@ -204,7 +204,7 @@ EventLoop::EventLoop(const char* exe_name, LogOptions log_opts, void* context) m_log_opts(std::move(log_opts)), m_context(context) { - int fds[2]; + SocketId fds[2]; KJ_SYSCALL(socketpair(AF_UNIX, SOCK_STREAM, 0, fds)); m_wait_fd = fds[0]; m_post_fd = fds[1]; @@ -216,8 +216,8 @@ EventLoop::~EventLoop() const Lock lock(m_mutex); KJ_ASSERT(m_post_fn == nullptr); KJ_ASSERT(!m_async_fns); - KJ_ASSERT(m_wait_fd == -1); - KJ_ASSERT(m_post_fd == -1); + KJ_ASSERT(m_wait_fd == SocketError); + KJ_ASSERT(m_post_fd == SocketError); KJ_ASSERT(m_num_clients == 0); // Spin event loop. wait for any promises triggered by RPC shutdown. @@ -268,8 +268,8 @@ void EventLoop::loop() wait_stream = nullptr; KJ_SYSCALL(::close(post_fd)); const Lock lock(m_mutex); - m_wait_fd = -1; - m_post_fd = -1; + m_wait_fd = SocketError; + m_post_fd = SocketError; m_async_fns.reset(); m_cv.notify_all(); } diff --git a/src/mp/util.cpp b/src/mp/util.cpp index d02aa0e1..414e4f31 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -116,9 +116,9 @@ std::string LogEscape(const kj::StringTree& string, size_t max_size) return result; } -int SpawnProcess(int& pid, FdToArgsFn&& fd_to_args) +SocketId SpawnProcess(ProcessId& pid, FdToArgsFn&& fd_to_args) { - int fds[2]; + SocketId fds[2]; if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) != 0) { throw std::system_error(errno, std::system_category(), "socketpair"); } diff --git a/test/mp/test/spawn_tests.cpp b/test/mp/test/spawn_tests.cpp index e3027552..b2c3f061 100644 --- a/test/mp/test/spawn_tests.cpp +++ b/test/mp/test/spawn_tests.cpp @@ -89,7 +89,7 @@ KJ_TEST("SpawnProcess does not run callback in child") }); ProcessId pid{-1}; - const int fd{SpawnProcess(pid, [&](int child_fd) -> std::vector { + const SocketId fd{SpawnProcess(pid, [&](SocketId child_fd) -> std::vector { // If this callback runs in the post-fork child, target_mutex appears // locked forever (the owning thread does not exist), so this deadlocks. std::lock_guard g(target_mutex); From c7a939bdd882ccfb60f17b6c8aea0de698984805 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 30 Apr 2025 08:39:29 -0400 Subject: [PATCH 04/19] util, refactor: Add SpawnConnectInfo type alias and use it Add SpawnConnectInfo type alias to pass socket handle from parent process to child process in more platform independent way. --- example/calculator.cpp | 13 ++++--------- example/example.cpp | 8 ++++---- example/printer.cpp | 13 ++++--------- include/mp/util.h | 21 +++++++++++++-------- src/mp/util.cpp | 13 +++++++++---- test/mp/test/spawn_tests.cpp | 9 +++++---- 6 files changed, 39 insertions(+), 38 deletions(-) diff --git a/example/calculator.cpp b/example/calculator.cpp index cfaf785c..db9f2907 100644 --- a/example/calculator.cpp +++ b/example/calculator.cpp @@ -6,8 +6,7 @@ #include #include // NOLINT(misc-include-cleaner) // IWYU pragma: keep -#include -#include +#include // IWYU pragma: keep #include #include #include @@ -16,9 +15,9 @@ #include #include #include +#include #include #include -#include #include class CalculatorImpl : public Calculator @@ -51,14 +50,10 @@ int main(int argc, char** argv) std::cout << "Usage: mpcalculator \n"; return 1; } - mp::SocketId fd; - if (std::from_chars(argv[1], argv[1] + strlen(argv[1]), fd).ec != std::errc{}) { - std::cerr << argv[1] << " is not a number or is larger than an int\n"; - return 1; - } + mp::SocketId socket{mp::StartSpawned(argv[1])}; mp::EventLoop loop("mpcalculator", LogPrint); std::unique_ptr init = std::make_unique(); - mp::ServeStream(loop, fd, *init); + mp::ServeStream(loop, socket, *init); loop.loop(); return 0; } diff --git a/example/example.cpp b/example/example.cpp index 6a5bdc93..843af85a 100644 --- a/example/example.cpp +++ b/example/example.cpp @@ -19,20 +19,20 @@ #include #include #include +#include #include namespace fs = std::filesystem; static auto Spawn(mp::EventLoop& loop, const std::string& process_argv0, const std::string& new_exe_name) { - mp::ProcessId pid; - const mp::SocketId fd = mp::SpawnProcess(pid, [&](mp::SocketId fd) -> std::vector { + const auto [pid, socket] = mp::SpawnProcess([&](mp::SpawnConnectInfo info) -> std::vector { fs::path path = process_argv0; path.remove_filename(); path.append(new_exe_name); - return {path.string(), std::to_string(fd)}; + return {path.string(), std::move(info)}; }); - return std::make_tuple(mp::ConnectStream(loop, fd), pid); + return std::make_tuple(mp::ConnectStream(loop, socket), pid); } static void LogPrint(mp::LogMessage log_data) diff --git a/example/printer.cpp b/example/printer.cpp index ec749d83..76a040d0 100644 --- a/example/printer.cpp +++ b/example/printer.cpp @@ -7,8 +7,7 @@ #include #include // NOLINT(misc-include-cleaner) // IWYU pragma: keep -#include -#include +#include // IWYU pragma: keep #include #include #include @@ -16,9 +15,9 @@ #include #include #include +#include #include #include -#include class PrinterImpl : public Printer { @@ -44,14 +43,10 @@ int main(int argc, char** argv) std::cout << "Usage: mpprinter \n"; return 1; } - mp::SocketId fd; - if (std::from_chars(argv[1], argv[1] + strlen(argv[1]), fd).ec != std::errc{}) { - std::cerr << argv[1] << " is not a number or is larger than an int\n"; - return 1; - } + mp::SocketId socket{mp::StartSpawned(argv[1])}; mp::EventLoop loop("mpprinter", LogPrint); std::unique_ptr init = std::make_unique(); - mp::ServeStream(loop, fd, *init); + mp::ServeStream(loop, socket, *init); loop.loop(); return 0; } diff --git a/include/mp/util.h b/include/mp/util.h index 56667bb4..ecb11f5d 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -259,17 +259,22 @@ using ProcessId = int; using SocketId = int; constexpr SocketId SocketError{-1}; +//! Information about parent process passed to child process as a command-line +//! argument. On unix this is the child socket fd number formatted as a string. +using SpawnConnectInfo = std::string; + //! Callback type used by SpawnProcess below. -using FdToArgsFn = std::function(SocketId fd)>; +using SpawnConnectInfoToArgsFn = std::function(const SpawnConnectInfo&)>; //! Spawn a new process that communicates with the current process over a socket -//! pair. Returns pid through an output argument, and file descriptor for the -//! local side of the socket. -//! The fd_to_args callback is invoked in the parent process before fork(). -//! It must not rely on child pid/state, and must return the command line -//! arguments that should be used to execute the process. Embed the remote file -//! descriptor number in whatever format the child process expects. -SocketId SpawnProcess(ProcessId& pid, FdToArgsFn&& fd_to_args); +//! pair. Calls connect_info_to_args callback with a connection string that +//! needs to be passed to the child process, and executes the argv command line +//! it returns. Returns child process id and socket id. +std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_info_to_args); + +//! Initialize spawned child process using the SpawnConnectInfo string passed to it, +//! returning a socket id for communicating with the parent process. +SocketId StartSpawned(const SpawnConnectInfo& connect_info); //! Call execvp with vector args. //! Not safe to call in a post-fork child of a multi-threaded process. diff --git a/src/mp/util.cpp b/src/mp/util.cpp index 414e4f31..37011ab3 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -116,7 +116,7 @@ std::string LogEscape(const kj::StringTree& string, size_t max_size) return result; } -SocketId SpawnProcess(ProcessId& pid, FdToArgsFn&& fd_to_args) +std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_info_to_args) { SocketId fds[2]; if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) != 0) { @@ -129,10 +129,10 @@ SocketId SpawnProcess(ProcessId& pid, FdToArgsFn&& fd_to_args) // locks at fork time. In that case, running code that allocates memory or // takes locks in the child between fork() and exec() can deadlock // indefinitely. Precomputing arguments in the parent avoids this. - const std::vector args{fd_to_args(fds[0])}; + const std::vector args{connect_info_to_args(std::to_string(fds[0]))}; const std::vector argv{MakeArgv(args)}; - pid = fork(); + ProcessId pid = fork(); if (pid == -1) { throw std::system_error(errno, std::system_category(), "fork"); } @@ -168,7 +168,12 @@ SocketId SpawnProcess(ProcessId& pid, FdToArgsFn&& fd_to_args) perror("execvp failed"); _exit(127); } - return fds[1]; + return {pid, fds[1]}; +} + +SocketId StartSpawned(const SpawnConnectInfo& connect_info) +{ + return std::stoi(connect_info); } void ExecProcess(const std::vector& args) diff --git a/test/mp/test/spawn_tests.cpp b/test/mp/test/spawn_tests.cpp index b2c3f061..bdc9ef54 100644 --- a/test/mp/test/spawn_tests.cpp +++ b/test/mp/test/spawn_tests.cpp @@ -15,7 +15,9 @@ #include #include #include +#include #include +#include #include namespace mp { @@ -88,14 +90,13 @@ KJ_TEST("SpawnProcess does not run callback in child") control_cv.notify_one(); }); - ProcessId pid{-1}; - const SocketId fd{SpawnProcess(pid, [&](SocketId child_fd) -> std::vector { + const auto [pid, socket]{SpawnProcess([&](SpawnConnectInfo connect_info) -> std::vector { // If this callback runs in the post-fork child, target_mutex appears // locked forever (the owning thread does not exist), so this deadlocks. std::lock_guard g(target_mutex); - return {"true", std::to_string(child_fd)}; + return {"true", std::move(connect_info)}; })}; - ::close(fd); + ::close(socket); int status{0}; // Give the child some time to exit. If it does not, terminate it and From ddd3945be422d3b2addd2b25778eaff01ba41923 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 17 Apr 2026 16:57:13 -0400 Subject: [PATCH 05/19] util, refactor: Do not fork() and exec() separately gen.cpp used fork() directly via to invoke the capnp compiler as a subprocess, but fork() is not available on Windows, so shouldn't be used in application code. Add a StartProcess(const std::vector& args) function to util.h/util.cpp that spawns a process and returns its ProcessId, leaving the caller responsible for WaitProcess. On POSIX it uses posix_spawn; on Windows it can use CreateProcess. Update gen.cpp to replace the inline fork/exec/wait with mp::WaitProcess(mp::StartProcess(args)). There a change in behavior if starting the process fails, because KJ_FAIL_SYSCALL is now used for error reporting and the fs::weakly_canonical call is dropped (it is probably more useful to see literal executable name passed). Also previously, the child process could deadlock if exec() failed because threads in the parent may have held allocator or stdio mutex locks that are now leaked in the child. posix_spawn() avoids this entirely by never running arbitrary code in the child process. Co-Authored-By: Claude Sonnet 4.6 --- include/mp/util.h | 7 +++---- src/mp/gen.cpp | 12 +----------- src/mp/util.cpp | 18 ++++++++---------- 3 files changed, 12 insertions(+), 25 deletions(-) diff --git a/include/mp/util.h b/include/mp/util.h index ecb11f5d..ea4801a8 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -276,10 +276,9 @@ std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_ //! returning a socket id for communicating with the parent process. SocketId StartSpawned(const SpawnConnectInfo& connect_info); -//! Call execvp with vector args. -//! Not safe to call in a post-fork child of a multi-threaded process. -//! Currently only used by mpgen at build time. -void ExecProcess(const std::vector& args); +//! Start a process and return its process id. Caller should call WaitProcess +//! on the returned id. +ProcessId StartProcess(const std::vector& args); //! Wait for a process to exit and return its exit code. int WaitProcess(ProcessId pid); diff --git a/src/mp/gen.cpp b/src/mp/gen.cpp index c8db469d..227c736f 100644 --- a/src/mp/gen.cpp +++ b/src/mp/gen.cpp @@ -9,7 +9,6 @@ #include // IWYU pragma: keep #include #include -#include #include #include #include @@ -27,8 +26,6 @@ #include #include #include -#include -#include #include #include @@ -284,14 +281,7 @@ static void Generate(kj::StringPtr src_prefix, } args.emplace_back("--output=" capnp_PREFIX "/bin/capnpc-c++"); args.emplace_back(src_file); - const int pid = fork(); - if (pid == -1) { - throw std::system_error(errno, std::system_category(), "fork"); - } - if (!pid) { - mp::ExecProcess(args); - } - const int status = mp::WaitProcess(pid); + const int status = mp::WaitProcess(mp::StartProcess(args)); if (status) { throw std::runtime_error("Invoking " capnp_PREFIX "/bin/capnp failed"); } diff --git a/src/mp/util.cpp b/src/mp/util.cpp index 37011ab3..c450e5ba 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -7,14 +7,14 @@ #include #include -#include -#include #include +#include #include #include #include #include #include +#include #include #include #include @@ -32,7 +32,7 @@ #include #endif // HAVE_PTHREAD_GETTHREADID_NP -namespace fs = std::filesystem; +extern "C" char **environ; // NOLINT(readability-redundant-declaration) namespace mp { namespace { @@ -176,16 +176,14 @@ SocketId StartSpawned(const SpawnConnectInfo& connect_info) return std::stoi(connect_info); } -void ExecProcess(const std::vector& args) +ProcessId StartProcess(const std::vector& args) { const std::vector argv{MakeArgv(args)}; - if (execvp(argv[0], argv.data()) != 0) { - perror("execvp failed"); - if (errno == ENOENT && !args.empty()) { - std::cerr << "Missing executable: " << fs::weakly_canonical(args.front()) << '\n'; - } - _exit(1); + ProcessId pid; + if (int err = posix_spawn(&pid, argv[0], nullptr, nullptr, argv.data(), ::environ)) { + KJ_FAIL_SYSCALL("posix_spawn", err, args.front()); } + return pid; } int WaitProcess(ProcessId pid) From d9a5a68872167dd1bb0412d1174dcee694182527 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 30 Apr 2025 08:39:29 -0400 Subject: [PATCH 06/19] util, refactor: Add SocketPair() and use it in SpawnProcess Extract socket pair creation from SpawnProcess into a standalone SocketPair() function, and use it to replace the inline socketpair() call. This is pretty much a refactoring but technically there are two small behavior changes: - FD_CLOEXEC is now used to ensure sockets are not leaked if processes are spawned. This has no impact on SpawnProcess, but is better hygiene and could affect future callers of SocketPair(). - KJ_SYSCALL is now used to simplify error-handling. Co-Authored-By: Claude Sonnet 4.6 --- include/mp/util.h | 5 +++++ src/mp/util.cpp | 18 ++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/include/mp/util.h b/include/mp/util.h index ea4801a8..08811dee 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -5,6 +5,7 @@ #ifndef MP_UTIL_H #define MP_UTIL_H +#include #include #include #include @@ -276,6 +277,10 @@ std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_ //! returning a socket id for communicating with the parent process. SocketId StartSpawned(const SpawnConnectInfo& connect_info); +//! Create a socket pair that can be used to communicate within a process or +//! between parent and child processes. +std::array SocketPair(); + //! Start a process and return its process id. Caller should call WaitProcess //! on the returned id. ProcessId StartProcess(const std::vector& args); diff --git a/src/mp/util.cpp b/src/mp/util.cpp index c450e5ba..1721e12d 100644 --- a/src/mp/util.cpp +++ b/src/mp/util.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -118,10 +119,7 @@ std::string LogEscape(const kj::StringTree& string, size_t max_size) std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_info_to_args) { - SocketId fds[2]; - if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) != 0) { - throw std::system_error(errno, std::system_category(), "socketpair"); - } + auto fds{SocketPair()}; // Evaluate the callback and build the argv array before forking. // @@ -132,6 +130,9 @@ std::tuple SpawnProcess(SpawnConnectInfoToArgsFn&& connect_ const std::vector args{connect_info_to_args(std::to_string(fds[0]))}; const std::vector argv{MakeArgv(args)}; + // Clear FD_CLOEXEC on fds[0] before forking so it survives exec in the child. + KJ_SYSCALL(fcntl(fds[0], F_SETFD, 0)); + ProcessId pid = fork(); if (pid == -1) { throw std::system_error(errno, std::system_category(), "fork"); @@ -176,6 +177,15 @@ SocketId StartSpawned(const SpawnConnectInfo& connect_info) return std::stoi(connect_info); } +std::array SocketPair() +{ + int pair[2]; + KJ_SYSCALL(socketpair(AF_UNIX, SOCK_STREAM, 0, pair)); + KJ_SYSCALL(fcntl(pair[0], F_SETFD, FD_CLOEXEC)); + KJ_SYSCALL(fcntl(pair[1], F_SETFD, FD_CLOEXEC)); + return {pair[0], pair[1]}; +} + ProcessId StartProcess(const std::vector& args) { const std::vector argv{MakeArgv(args)}; From 85e13a6151ba8b665439cde6224ab40abff2d13a Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 30 Apr 2025 08:39:29 -0400 Subject: [PATCH 07/19] proxy, refactor: Replace EventLoop wakeup fd integers with KJ stream objects Replace the m_wait_fd/m_post_fd raw int members with m_wait_stream/m_post_stream kj::Own and m_post_writer kj::Own. The constructor uses provider->newTwoWayPipe() instead of calling socketpair() directly. The loop() and post() methods write through m_post_writer instead of calling write() with a raw fd, and EventLoopRef::reset does the same. --- include/mp/proxy-io.h | 10 ++++++---- src/mp/proxy.cpp | 36 +++++++++++++++++------------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/include/mp/proxy-io.h b/include/mp/proxy-io.h index a1c4cd4f..f8e0d059 100644 --- a/include/mp/proxy-io.h +++ b/include/mp/proxy-io.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -308,11 +309,12 @@ class EventLoop //! Callback functions to run on async thread. std::optional m_async_fns MP_GUARDED_BY(m_mutex); - //! Pipe read handle used to wake up the event loop thread. - SocketId m_wait_fd = SocketError; + //! Socket pair used to post and wait for wakeups to the event loop thread. + kj::Own m_wait_stream; + kj::Own m_post_stream; - //! Pipe write handle used to wake up the event loop thread. - SocketId m_post_fd = SocketError; + //! Synchronous writer used to write to m_post_stream. + kj::Own m_post_writer; //! Number of clients holding references to ProxyServerBase objects that //! reference this event loop. diff --git a/src/mp/proxy.cpp b/src/mp/proxy.cpp index 4783e1f1..7418807f 100644 --- a/src/mp/proxy.cpp +++ b/src/mp/proxy.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -31,10 +32,8 @@ #include #include #include -#include #include #include -#include #include namespace mp { @@ -67,10 +66,9 @@ void EventLoopRef::reset(bool relock) MP_NO_TSA loop->m_num_clients -= 1; if (loop->done()) { loop->m_cv.notify_all(); - int post_fd{loop->m_post_fd}; loop_lock->unlock(); char buffer = 0; - KJ_SYSCALL(write(post_fd, &buffer, 1)); // NOLINT(bugprone-suspicious-semicolon) + loop->m_post_writer->write(&buffer, 1); // By default, do not try to relock `loop_lock` after writing, // because the event loop could wake up and destroy itself and the // mutex might no longer exist. @@ -204,10 +202,14 @@ EventLoop::EventLoop(const char* exe_name, LogOptions log_opts, void* context) m_log_opts(std::move(log_opts)), m_context(context) { - SocketId fds[2]; - KJ_SYSCALL(socketpair(AF_UNIX, SOCK_STREAM, 0, fds)); - m_wait_fd = fds[0]; - m_post_fd = fds[1]; + auto pipe = m_io_context.provider->newTwoWayPipe(); + m_wait_stream = kj::mv(pipe.ends[0]); + m_post_stream = kj::mv(pipe.ends[1]); + KJ_IF_MAYBE(fd, m_post_stream->getFd()) { + m_post_writer = kj::heap(*fd); + } else { + throw std::logic_error("Could not get file descriptor for new pipe."); + } } EventLoop::~EventLoop() @@ -216,8 +218,8 @@ EventLoop::~EventLoop() const Lock lock(m_mutex); KJ_ASSERT(m_post_fn == nullptr); KJ_ASSERT(!m_async_fns); - KJ_ASSERT(m_wait_fd == SocketError); - KJ_ASSERT(m_post_fd == SocketError); + KJ_ASSERT(!m_wait_stream); + KJ_ASSERT(!m_post_stream); KJ_ASSERT(m_num_clients == 0); // Spin event loop. wait for any promises triggered by RPC shutdown. @@ -237,9 +239,7 @@ void EventLoop::loop() m_async_fns.emplace(); } - kj::Own wait_stream{ - m_io_context.lowLevelProvider->wrapSocketFd(m_wait_fd, kj::LowLevelAsyncIoProvider::TAKE_OWNERSHIP)}; - int post_fd{m_post_fd}; + kj::Own& wait_stream{m_wait_stream}; char buffer = 0; for (;;) { const size_t read_bytes = wait_stream->read(&buffer, 0, 1).wait(m_io_context.waitScope); @@ -256,7 +256,7 @@ void EventLoop::loop() m_cv.notify_all(); } else if (done()) { // Intentionally do not break if m_post_fn was set, even if done() - // would return true, to ensure that the EventLoopRef write(post_fd) + // would return true, to ensure that the post() m_post_writer->write() // call always succeeds and the loop does not exit between the time // that the done condition is set and the write call is made. break; @@ -266,10 +266,9 @@ void EventLoop::loop() m_task_set.reset(); MP_LOG(*this, Log::Info) << "EventLoop::loop bye."; wait_stream = nullptr; - KJ_SYSCALL(::close(post_fd)); const Lock lock(m_mutex); - m_wait_fd = SocketError; - m_post_fd = SocketError; + m_wait_stream = nullptr; + m_post_stream = nullptr; m_async_fns.reset(); m_cv.notify_all(); } @@ -284,10 +283,9 @@ void EventLoop::post(kj::Function fn) EventLoopRef ref(*this, &lock); m_cv.wait(lock.m_lock, [this]() MP_REQUIRES(m_mutex) { return m_post_fn == nullptr; }); m_post_fn = &fn; - int post_fd{m_post_fd}; Unlock(lock, [&] { char buffer = 0; - KJ_SYSCALL(write(post_fd, &buffer, 1)); + m_post_writer->write(&buffer, 1); }); m_cv.wait(lock.m_lock, [this, &fn]() MP_REQUIRES(m_mutex) { return m_post_fn != &fn; }); } From 57350eac6faad5ec65b4bcf40cc37e610ef7c150 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 16 Apr 2026 18:33:22 -0400 Subject: [PATCH 08/19] cmake: Bump minimum required Cap'n Proto version to 0.9 kj::AsyncIoStream::getFd() was added in capnproto 0.9 (commit d27bfb8a4175b32b783de68d93dd1dbafadddea5, first released in 0.9.0). The code now uses getFd() in proxy.cpp, so 0.7 is no longer a sufficient minimum. Set olddeps version to 0.9.2, which is the patched 0.9.x release for CVE-2022-46149. Co-Authored-By: Claude Sonnet 4.6 --- CMakeLists.txt | 2 +- ci/configs/olddeps.bash | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ea91f98f..6354b0ee 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,7 +13,7 @@ endif() include("cmake/compat_find.cmake") find_package(Threads REQUIRED) -find_package(CapnProto 0.7 QUIET NO_MODULE) +find_package(CapnProto 0.9 QUIET NO_MODULE) if(NOT CapnProto_FOUND) message(FATAL_ERROR "Cap'n Proto is required but was not found.\n" diff --git a/ci/configs/olddeps.bash b/ci/configs/olddeps.bash index 95f44128..1a363b1b 100644 --- a/ci/configs/olddeps.bash +++ b/ci/configs/olddeps.bash @@ -1,5 +1,5 @@ CI_DESC="CI job using old Cap'n Proto and cmake versions" CI_DIR=build-olddeps export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wno-unused-parameter -Wno-error=array-bounds" -NIX_ARGS=(--argstr capnprotoVersion "0.7.1" --argstr cmakeVersion "3.12.4") +NIX_ARGS=(--argstr capnprotoVersion "0.9.2" --argstr cmakeVersion "3.12.4") BUILD_ARGS=(-k) From 174604aca41a35b3cd5200870e05dd04d5891fd2 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 30 Apr 2025 08:39:29 -0400 Subject: [PATCH 09/19] proxy, refactor: Change ConnectStream and ServeStream to accept stream objects Instead of accepting raw file descriptor integers and wrapping them internally, ConnectStream and ServeStream now accept kj::Own directly. This removes the assumption that the transport is always a local unix fd, making the API easier to adapt to other I/O types (e.g. Windows handles). The Stream type alias (kj::Own) is added as a convenience. Callers are updated to wrap their fd with wrapSocketFd() before calling. --- example/calculator.cpp | 2 +- example/example.cpp | 2 +- example/printer.cpp | 2 +- include/mp/proxy-io.h | 29 ++++++++++++++++------------- include/mp/util.h | 4 ++++ 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/example/calculator.cpp b/example/calculator.cpp index db9f2907..6ed2df5f 100644 --- a/example/calculator.cpp +++ b/example/calculator.cpp @@ -53,7 +53,7 @@ int main(int argc, char** argv) mp::SocketId socket{mp::StartSpawned(argv[1])}; mp::EventLoop loop("mpcalculator", LogPrint); std::unique_ptr init = std::make_unique(); - mp::ServeStream(loop, socket, *init); + mp::ServeStream(loop, mp::MakeStream(loop.m_io_context, socket), *init); loop.loop(); return 0; } diff --git a/example/example.cpp b/example/example.cpp index 843af85a..e6f69e3f 100644 --- a/example/example.cpp +++ b/example/example.cpp @@ -32,7 +32,7 @@ static auto Spawn(mp::EventLoop& loop, const std::string& process_argv0, const s path.append(new_exe_name); return {path.string(), std::move(info)}; }); - return std::make_tuple(mp::ConnectStream(loop, socket), pid); + return std::make_tuple(mp::ConnectStream(loop, mp::MakeStream(loop.m_io_context, socket)), pid); } static void LogPrint(mp::LogMessage log_data) diff --git a/example/printer.cpp b/example/printer.cpp index 76a040d0..9b456d9c 100644 --- a/example/printer.cpp +++ b/example/printer.cpp @@ -46,7 +46,7 @@ int main(int argc, char** argv) mp::SocketId socket{mp::StartSpawned(argv[1])}; mp::EventLoop loop("mpprinter", LogPrint); std::unique_ptr init = std::make_unique(); - mp::ServeStream(loop, socket, *init); + mp::ServeStream(loop, mp::MakeStream(loop.m_io_context, socket), *init); loop.loop(); return 0; } diff --git a/include/mp/proxy-io.h b/include/mp/proxy-io.h index f8e0d059..91d02ccd 100644 --- a/include/mp/proxy-io.h +++ b/include/mp/proxy-io.h @@ -211,6 +211,12 @@ class Logger std::string LongThreadName(const char* exe_name); +//! Wrap a socket file descriptor as an async stream, taking ownership of the fd. +inline Stream MakeStream(kj::AsyncIoContext& io_context, SocketId socket) +{ + return io_context.lowLevelProvider->wrapSocketFd(socket, kj::LowLevelAsyncIoProvider::TAKE_OWNERSHIP); +} + //! Event loop implementation. //! //! Cap'n Proto threading model is very simple: all I/O operations are @@ -806,17 +812,15 @@ kj::Promise ProxyServer::post(Fn&& fn) return ret; } -//! Given stream file descriptor, make a new ProxyClient object to send requests -//! over the stream. Also create a new Connection object embedded in the -//! client that is freed when the client is closed. +//! Given a stream, make a new ProxyClient object to send requests over it. +//! Also create a new Connection object embedded in the client that is freed +//! when the client is closed. template -std::unique_ptr> ConnectStream(EventLoop& loop, int fd) +std::unique_ptr> ConnectStream(EventLoop& loop, Stream stream) { typename InitInterface::Client init_client(nullptr); std::unique_ptr connection; loop.sync([&] { - auto stream = - loop.m_io_context.lowLevelProvider->wrapSocketFd(fd, kj::LowLevelAsyncIoProvider::TAKE_OWNERSHIP); connection = std::make_unique(loop, kj::mv(stream)); init_client = connection->m_rpc_system->bootstrap(ServerVatId().vat_id).castAs(); Connection* connection_ptr = connection.get(); @@ -834,7 +838,7 @@ std::unique_ptr> ConnectStream(EventLoop& loop, int f //! ProxyServer in a Connection object that is stored and erased if //! disconnected. This should be called from the event loop thread. template -void _Serve(EventLoop& loop, kj::Own&& stream, InitImpl& init) +void _Serve(EventLoop& loop, Stream&& stream, InitImpl& init) { loop.m_incoming_connections.emplace_front(loop, kj::mv(stream), [&](Connection& connection) { // Disable deleter so proxy server object doesn't attempt to delete the @@ -858,19 +862,18 @@ void _Listen(EventLoop& loop, kj::Own&& listener, InitIm { auto* ptr = listener.get(); loop.m_task_set->add(ptr->accept().then( - [&loop, &init, listener = kj::mv(listener)](kj::Own&& stream) mutable { + [&loop, &init, listener = kj::mv(listener)](Stream&& stream) mutable { _Serve(loop, kj::mv(stream), init); _Listen(loop, kj::mv(listener), init); })); } -//! Given stream file descriptor and an init object, handle requests on the -//! stream by calling methods on the Init object. +//! Given a stream and an init object, handle requests on the stream by calling +//! methods on the Init object. template -void ServeStream(EventLoop& loop, int fd, InitImpl& init) +void ServeStream(EventLoop& loop, Stream stream, InitImpl& init) { - _Serve( - loop, loop.m_io_context.lowLevelProvider->wrapSocketFd(fd, kj::LowLevelAsyncIoProvider::TAKE_OWNERSHIP), init); + _Serve(loop, kj::mv(stream), init); } //! Given listening socket file descriptor and an init object, handle incoming diff --git a/include/mp/util.h b/include/mp/util.h index 08811dee..c9409b83 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include #include #include @@ -256,6 +258,8 @@ std::string ThreadName(const char* exe_name); //! errors in python unit tests. std::string LogEscape(const kj::StringTree& string, size_t max_size); +using Stream = kj::Own; + using ProcessId = int; using SocketId = int; constexpr SocketId SocketError{-1}; From c5faf68d371d3d7d84e5b89743073924022716ac Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 30 Apr 2025 08:39:29 -0400 Subject: [PATCH 10/19] proxy: Call shutdownWrite() in Connection destructor Flush pending Cap'n Proto release messages before closing the stream. When one side of a socket pair closes, the other side does not receive an onDisconnect event, so it relies on receiving release messages from the closing side to free its ProxyServer objects and shut down cleanly. Without this, Server objects are not freed by Cap'n Proto on disconnection. --- src/mp/proxy.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/mp/proxy.cpp b/src/mp/proxy.cpp index 7418807f..f957893d 100644 --- a/src/mp/proxy.cpp +++ b/src/mp/proxy.cpp @@ -99,6 +99,28 @@ Connection::~Connection() // after the calls finish. m_rpc_system.reset(); + // shutdownWrite is needed on Windows so pending data in the m_stream socket + // will be sent instead of discarded when m_stream is destroyed. On unix, + // this doesn't seem to be needed because data is sent more reliably. + // + // Sending pending data is important if the connection is a socketpair + // because when one side of the socketpair is closed, the other side doesn't + // seem to receive any onDisconnect event. So it is important for the other + // side to instead receive Cap'n Proto "release" messages (see `struct + // Release` in capnp/rpc.capnp) from local Client objects being destroyed so + // the remote side can free resources and shut down cleanly. Without this, + // when one side of a socket pair is closed the other side may not receive + // these messages, preventing the remote side from freeing ProxyServer + // resources and shutting down cleanly. + try { + m_stream->shutdownWrite(); + } catch (const kj::Exception& e) { + // Ignore ENOTCONN: on macOS/FreeBSD (unlike Linux), shutdown(SHUT_WR) + // returns ENOTCONN if the peer already closed the connection. This is + // expected when the destructor is triggered by a remote disconnect. + if (e.getType() != kj::Exception::Type::DISCONNECTED) throw; + } + // ProxyClient cleanup handlers are in sync list, and ProxyServer cleanup // handlers are in the async list. // From 727be3de924d11ba4246dd90fb9877c57c325b76 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 16 Apr 2026 21:28:52 -0400 Subject: [PATCH 11/19] proxy: Fix shutdownWrite() exception handling on macOS with dynamic libraries On macOS, when libcapnp is built as a dynamic library and Bitcoin Core REDUCE_EXPORT option is used the RTTI typeinfo for kj::Exception has a different address in libcapnp.dylib versus the calling binary. This means catch (const kj::Exception& e) in the calling binary silently fails to match exceptions thrown by capnp, so the DISCONNECTED exception from shutdownWrite() propagates as a fatal uncaught exception instead of being suppressed as intended. This causes the Bitcoin Core macOS native CI job to fail with: Fatal uncaught kj::Exception: kj/async-io-unix.c++:491: disconnected: shutdown(fd, SHUT_WR): Socket is not connected The fix is to use kj::runCatchingExceptions/kj::throwRecoverableException, which use KJ's own thread-level exception interception mechanism rather than C++ RTTI-based matching, and therefore work correctly across dynamic library boundaries. This is the same approach used elsewhere in the codebase (proxy.cpp EventLoop::post, type-context.h server request handler) for the same reason. Co-Authored-By: Claude Sonnet 4.6 --- src/mp/proxy.cpp | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/mp/proxy.cpp b/src/mp/proxy.cpp index f957893d..c7b602e0 100644 --- a/src/mp/proxy.cpp +++ b/src/mp/proxy.cpp @@ -112,13 +112,20 @@ Connection::~Connection() // when one side of a socket pair is closed the other side may not receive // these messages, preventing the remote side from freeing ProxyServer // resources and shutting down cleanly. - try { + // Use kj::runCatchingExceptions instead of try/catch because on macOS with + // dynamic libraries, kj::Exception typeinfo differs between libcapnp and + // the calling binary, so catch (const kj::Exception&) silently fails to + // match. kj::runCatchingExceptions uses KJ's own interception mechanism + // which works correctly across dynamic library boundaries. + KJ_IF_MAYBE(exception, kj::runCatchingExceptions([&]() { m_stream->shutdownWrite(); - } catch (const kj::Exception& e) { + })) { // Ignore ENOTCONN: on macOS/FreeBSD (unlike Linux), shutdown(SHUT_WR) // returns ENOTCONN if the peer already closed the connection. This is // expected when the destructor is triggered by a remote disconnect. - if (e.getType() != kj::Exception::Type::DISCONNECTED) throw; + if (exception->getType() != kj::Exception::Type::DISCONNECTED) { + kj::throwRecoverableException(kj::mv(*exception)); + } } // ProxyClient cleanup handlers are in sync list, and ProxyServer cleanup From a5e23ec316b7a3302af5ed611099c0b85356bad8 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 17 Apr 2026 16:57:05 -0400 Subject: [PATCH 12/19] util, refactor: Fix PtrOrValue constructor for move-only types on MSVC MSVC error when building multiprocess.vcxproj: mp/util.h(146,46): error C2280: 'std::variant::variant(const std::variant &)': attempting to reference a deleted function [with T=mp::Lock] The PtrOrValue constructor used a ternary expression to initialize data: data(ptr ? ptr : std::variant{std::in_place_type, args...}) Both arms are prvalues of type std::variant, so under C++17's mandatory copy elision no copy/move constructor should be invoked. GCC and Clang apply this correctly. MSVC does not apply guaranteed copy elision to ternary expressions in this context: it materializes the temporary and then attempts to copy-construct data from it. Since std::variant has a deleted copy constructor (Lock holds a std::unique_lock which is move-only), MSVC fails. Fix by initializing data to hold T*=ptr in the member initializer list, then emplacing T in-place in the constructor body if ptr is null. This avoids the ternary entirely and requires only the in-place constructor of T, not any variant copy or move. Co-Authored-By: Claude Sonnet 4.6 --- include/mp/util.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/mp/util.h b/include/mp/util.h index c9409b83..54b087f7 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -145,7 +145,10 @@ struct PtrOrValue { std::variant data; template - PtrOrValue(T* ptr, Args&&... args) : data(ptr ? ptr : std::variant{std::in_place_type, std::forward(args)...}) {} + PtrOrValue(T* ptr, Args&&... args) : data(std::in_place_type, ptr) + { + if (!ptr) data.template emplace(std::forward(args)...); + } T& operator*() { return data.index() ? std::get(data) : *std::get(data); } T* operator->() { return &**this; } From 236a30b7c7d0d55336e36515aa9f9b3bbeb31051 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 22 Apr 2026 16:35:35 -0400 Subject: [PATCH 13/19] proxy, refactor: Fix C4305 truncation warning in Accessor on MSVC MSVC warns (C4305, treated as error) about truncation from 'int' to 'const bool' when initializing static const bool members from integer bitwise-and expressions. Use constexpr bool with explicit != 0 to make the boolean conversion unambiguous. Co-Authored-By: Claude Sonnet 4.6 --- include/mp/proxy.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/mp/proxy.h b/include/mp/proxy.h index 4458ee9e..f9d3fb33 100644 --- a/include/mp/proxy.h +++ b/include/mp/proxy.h @@ -315,21 +315,21 @@ template struct Accessor : public Field { //! Field is present from the Cap'n Proto Params struct (client -> server). - static const bool in = flags & FIELD_IN; + static constexpr bool in = (flags & FIELD_IN) != 0; //! Field is present from the Cap'n Proto Results struct (server -> client). - static const bool out = flags & FIELD_OUT; + static constexpr bool out = (flags & FIELD_OUT) != 0; //! 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) because Cap'n Proto doesn't allow primitive fields to //! be unset. - static const bool optional = flags & FIELD_OPTIONAL; + static constexpr bool optional = (flags & FIELD_OPTIONAL) != 0; //! 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; + static constexpr bool requested = (flags & FIELD_REQUESTED) != 0; //! 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; + static constexpr bool boxed = (flags & FIELD_BOXED) != 0; }; //! Wrapper around std::function for passing std::function objects between client and servers. From 79fba82b9e361ef93c651765119aa846717752f5 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Sun, 21 Jun 2026 20:23:30 -0400 Subject: [PATCH 14/19] types: Replace SFINAE with requires clauses to avoid MSVC C2039 error MSVC does not correctly apply SFINAE when the substitution failure occurs in a default function argument that uses decltype of another function parameter (MSVC error C2039). Instead of silently excluding the overload, MSVC instantiates the function body and reports a hard error, e.g.: type-interface.h(62,56): error C2039: 'Calls': is not a member of 'capnp::List, capnp::Kind::STRUCT>::Builder' The root cause is that MSVC evaluates default argument expressions outside the SFINAE immediate context when they reference function parameters via decltype. GCC and Clang treat this as a substitution failure and silently exclude the overload, as the standard intends. Replace all uses of the `* enable = nullptr` default-argument SFINAE pattern with C++20 requires clauses, which are well-supported on all three compilers and give cleaner constraint-violation diagnostics. Add two concepts to util.h to reduce repetition across the type-*.h headers: FieldTypeIs - T's .get() returns exactly type U InterfaceField - T's .get() returns a capnp interface type (a type that exposes a nested ::Calls member) Co-Authored-By: Claude Sonnet 4.6 --- include/mp/type-context.h | 4 ++-- include/mp/type-interface.h | 20 ++++++++++---------- include/mp/type-number.h | 24 ++++++++++++------------ include/mp/type-struct.h | 24 ++++++++++++------------ include/mp/type-threadmap.h | 8 ++++---- include/mp/util.h | 10 ++++++++++ 6 files changed, 50 insertions(+), 40 deletions(-) diff --git a/include/mp/type-context.h b/include/mp/type-context.h index 3ab3d4b0..84e5f32e 100644 --- a/include/mp/type-context.h +++ b/include/mp/type-context.h @@ -12,11 +12,11 @@ namespace mp { template + requires FieldTypeIs void CustomBuildField(TypeList<>, Priority<1>, ClientInvokeContext& invoke_context, - Output&& output, - typename std::enable_if::value>::type* enable = nullptr) + Output&& output) { auto& connection = invoke_context.connection; auto& thread_context = invoke_context.thread_context; diff --git a/include/mp/type-interface.h b/include/mp/type-interface.h index a32c53d2..39bb5ce6 100644 --- a/include/mp/type-interface.h +++ b/include/mp/type-interface.h @@ -21,12 +21,12 @@ kj::Own CustomMakeProxyServer(InvokeContext& context } template + requires InterfaceField void CustomBuildField(TypeList>, Priority<1>, InvokeContext& invoke_context, Value&& value, - Output&& output, - typename Decay::Calls* enable = nullptr) + Output&& output) { if (value) { using Interface = typename decltype(output.get())::Calls; @@ -35,12 +35,12 @@ void CustomBuildField(TypeList>, } template + requires InterfaceField void CustomBuildField(TypeList>, Priority<2>, InvokeContext& invoke_context, Value&& value, - Output&& output, - typename Decay::Calls* enable = nullptr) + Output&& output) { if (value) { using Interface = typename decltype(output.get())::Calls; @@ -49,12 +49,12 @@ void CustomBuildField(TypeList>, } template + requires InterfaceField void CustomBuildField(TypeList, Priority<1>, InvokeContext& invoke_context, Impl& value, - Output&& output, - typename decltype(output.get())::Calls* enable = nullptr) + Output&& output) { // Disable deleter so proxy server object doesn't attempt to delete the // wrapped implementation when the proxy client is destroyed or @@ -77,12 +77,12 @@ std::unique_ptr CustomMakeProxyClient(InvokeContext& context, typename Int } template + requires InterfaceField decltype(auto) CustomReadField(TypeList>, Priority<1>, InvokeContext& invoke_context, Input&& input, - ReadDest&& read_dest, - typename Decay::Calls* enable = nullptr) + ReadDest&& read_dest) { using Interface = typename Decay::Calls; if (CustomHasField(TypeList(), invoke_context, input)) { @@ -93,12 +93,12 @@ decltype(auto) CustomReadField(TypeList>, } template + requires InterfaceField decltype(auto) CustomReadField(TypeList>, Priority<1>, InvokeContext& invoke_context, Input&& input, - ReadDest&& read_dest, - typename Decay::Calls* enable = nullptr) + ReadDest&& read_dest) { using Interface = typename Decay::Calls; if (CustomHasField(TypeList(), invoke_context, input)) { diff --git a/include/mp/type-number.h b/include/mp/type-number.h index ddff5cdd..380d70c9 100644 --- a/include/mp/type-number.h +++ b/include/mp/type-number.h @@ -9,10 +9,10 @@ namespace mp { template + requires std::is_enum_v LocalType BuildPrimitive(InvokeContext& invoke_context, const Value& value, - TypeList, - typename std::enable_if::value>::type* enable = nullptr) + TypeList) { using E = std::make_unsigned_t>; using T = std::make_unsigned_t; @@ -21,10 +21,10 @@ LocalType BuildPrimitive(InvokeContext& invoke_context, } template + requires std::is_integral_v LocalType BuildPrimitive(InvokeContext& invoke_context, const Value& value, - TypeList, - typename std::enable_if::value, int>::type* enable = nullptr) + TypeList) { static_assert( std::numeric_limits::lowest() <= std::numeric_limits::lowest(), "mismatched integral types"); @@ -34,10 +34,10 @@ LocalType BuildPrimitive(InvokeContext& invoke_context, } template + requires std::is_floating_point_v LocalType BuildPrimitive(InvokeContext& invoke_context, const Value& value, - TypeList, - typename std::enable_if::value>::type* enable = nullptr) + TypeList) { static_assert(std::is_same::value, "mismatched floating point types. please fix message.capnp type declaration to match wrapped interface"); @@ -45,12 +45,12 @@ LocalType BuildPrimitive(InvokeContext& invoke_context, } template + requires std::is_enum_v decltype(auto) CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, Input&& input, - ReadDest&& read_dest, - typename std::enable_if::value>::type* enable = nullptr) + ReadDest&& read_dest) { // Disable clang-tidy out-of-range enum value check which triggers when // using an enum type that does not have a 0 value. The check correctly @@ -62,12 +62,12 @@ decltype(auto) CustomReadField(TypeList, } template + requires std::is_integral_v decltype(auto) CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, Input&& input, - ReadDest&& read_dest, - typename std::enable_if::value>::type* enable = nullptr) + ReadDest&& read_dest) { auto value = input.get(); if (value < std::numeric_limits::min() || value > std::numeric_limits::max()) { @@ -77,12 +77,12 @@ decltype(auto) CustomReadField(TypeList, } template + requires std::is_floating_point_v decltype(auto) CustomReadField(TypeList, Priority<1>, InvokeContext& invoke_context, Input&& input, - ReadDest&& read_dest, - typename std::enable_if::value>::type* enable = nullptr) + ReadDest&& read_dest) { auto value = input.get(); static_assert(std::is_same::value, "floating point type mismatch"); diff --git a/include/mp/type-struct.h b/include/mp/type-struct.h index 6d396387..10d7dbd2 100644 --- a/include/mp/type-struct.h +++ b/include/mp/type-struct.h @@ -9,11 +9,11 @@ namespace mp { template + requires (index < ProxyType::fields) void BuildOne(TypeList param, InvokeContext& invoke_context, Output&& output, - Value&& value, - typename std::enable_if < index::fields>::type * enable = nullptr) + Value&& value) { using Index = std::integral_constant; using Struct = typename ProxyType::Struct; @@ -25,31 +25,31 @@ void BuildOne(TypeList param, } template + requires (index == ProxyType::fields) void BuildOne(TypeList param, InvokeContext& invoke_context, Output&& output, - Value&& value, - typename std::enable_if::fields>::type* enable = nullptr) + Value&& value) { } template + requires requires { typename ProxyType::Struct; } void CustomBuildField(TypeList local_type, Priority<1>, InvokeContext& invoke_context, Value&& value, - Output&& output, - typename ProxyType::Struct* enable = nullptr) + Output&& output) { BuildOne<0>(local_type, invoke_context, output.init(), value); } template + requires (index != ProxyType::fields) void ReadOne(TypeList param, InvokeContext& invoke_context, Input&& input, - Value&& value, - typename std::enable_if::fields>::type* enable = nullptr) + Value&& value) { using Index = std::integral_constant; using Struct = typename ProxyType::Struct; @@ -62,21 +62,21 @@ void ReadOne(TypeList param, } template + requires (index == ProxyType::fields) void ReadOne(TypeList param, InvokeContext& invoke_context, Input& input, - Value& value, - typename std::enable_if::fields>::type* enable = nullptr) + Value& value) { } template + requires requires { typename ProxyType::Struct; } decltype(auto) CustomReadField(TypeList param, Priority<1>, InvokeContext& invoke_context, Input&& input, - ReadDest&& read_dest, - typename ProxyType::Struct* enable = nullptr) + ReadDest&& read_dest) { return read_dest.update([&](auto& value) { ReadOne<0>(param, invoke_context, input, value); }); } diff --git a/include/mp/type-threadmap.h b/include/mp/type-threadmap.h index 3005d9de..99d1aaf6 100644 --- a/include/mp/type-threadmap.h +++ b/include/mp/type-threadmap.h @@ -18,21 +18,21 @@ struct ProxyServer final : public virtual ThreadMap::Server }; template + requires FieldTypeIs void CustomBuildField(TypeList<>, Priority<1>, InvokeContext& invoke_context, - Output&& output, - typename std::enable_if::value>::type* enable = nullptr) + Output&& output) { output.set(kj::heap>(invoke_context.connection)); } template + requires FieldTypeIs decltype(auto) CustomReadField(TypeList<>, Priority<1>, InvokeContext& invoke_context, - Input&& input, - typename std::enable_if::value>::type* enable = nullptr) + Input&& input) { invoke_context.connection.m_thread_map = input.get(); } diff --git a/include/mp/util.h b/include/mp/util.h index 54b087f7..f647d5d7 100644 --- a/include/mp/util.h +++ b/include/mp/util.h @@ -94,6 +94,16 @@ using RemoveCvRef = std::remove_cv_t>; template using Decay = std::decay_t; +//! Concept satisfied when T's .get() method returns exactly type U. +//! Used to constrain overloads that handle a specific capnp field type. +template +concept FieldTypeIs = std::is_same_v().get()), U>; + +//! Concept satisfied when T's .get() method returns a capnp interface type +//! (i.e., a type that exposes a nested ::Calls type in generated code). +template +concept InterfaceField = requires { typename Decay().get())>::Calls; }; + //! SFINAE helper, see using Require below. template struct _Require From da3767fb40a8a6a03018878e7357f8200555e25e Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Thu, 16 Apr 2026 18:03:24 -0400 Subject: [PATCH 15/19] ci: Check out bitcoin/bitcoin PR #35084 instead of master This repo has introduced API changes to add Windows support to libmultiprocess (HANDLE-based IPC alongside the existing fd-based IPC). These changes require corresponding updates to Bitcoin Core, which are pending in bitcoin/bitcoin#35084. Until that PR merges, the Bitcoin Core CI jobs fail against master because Bitcoin Core has not yet been updated to use the new API. Switch the Bitcoin Core checkout in both jobs to use refs/pull/35084/merge so CI tests against the compatible version. A BITCOIN_CORE_REF env var is introduced at the top of the file; once (and keep the var in place for any future API compatibility cycles). Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/bitcoin-core-ci.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/bitcoin-core-ci.yml b/.github/workflows/bitcoin-core-ci.yml index e6ac83f0..89380ac4 100644 --- a/.github/workflows/bitcoin-core-ci.yml +++ b/.github/workflows/bitcoin-core-ci.yml @@ -18,6 +18,8 @@ concurrency: env: BITCOIN_REPO: bitcoin/bitcoin + # Temporary: use PR #35084 until it merges; revert to refs/heads/master after + BITCOIN_CORE_REF: refs/pull/35084/merge LLVM_VERSION: 22 LIBCXX_DIR: /tmp/libcxx-build/ @@ -79,6 +81,7 @@ jobs: uses: actions/checkout@v4 with: repository: ${{ env.BITCOIN_REPO }} + ref: ${{ env.BITCOIN_CORE_REF }} fetch-depth: 1 - name: Checkout libmultiprocess @@ -195,6 +198,7 @@ jobs: uses: actions/checkout@v4 with: repository: ${{ env.BITCOIN_REPO }} + ref: ${{ env.BITCOIN_CORE_REF }} fetch-depth: 1 - name: Checkout libmultiprocess From 0b919217689d45858d48fb788c4136f195c16129 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Mon, 20 Apr 2026 17:12:03 -0400 Subject: [PATCH 16/19] ipc: Wrap mpgen main() in try-catch to print errors On MSVC, std::terminate() does not print the exception message before calling abort()/fastfail, so exceptions thrown during mpgen execution appear as a bare 0xC0000409 exit code with no diagnostic output. Wrap main() in a try-catch to explicitly print the error to stderr and return 1 instead of crashing. Co-Authored-By: Claude Sonnet 4.6 --- src/mp/gen.cpp | 61 ++++++++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/src/mp/gen.cpp b/src/mp/gen.cpp index 227c736f..435ba441 100644 --- a/src/mp/gen.cpp +++ b/src/mp/gen.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -691,35 +692,41 @@ static void Generate(kj::StringPtr src_prefix, int main(int argc, char** argv) { - if (argc < 3) { - std::cerr << "Usage: " << PROXY_BIN << " SRC_PREFIX INCLUDE_PREFIX SRC_FILE [IMPORT_PATH...]\n"; - exit(1); - } - std::vector import_paths; - std::vector> import_dirs; - auto fs = kj::newDiskFilesystem(); - auto cwd = fs->getCurrentPath(); - kj::Own src_dir; - KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(argv[1]))) { - src_dir = kj::mv(*dir); - } else { - throw std::runtime_error(std::string("Failed to open src_prefix prefix directory: ") + argv[1]); - } - for (int i = 4; i < argc; ++i) { - KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(argv[i]))) { - import_paths.emplace_back(argv[i]); - import_dirs.emplace_back(kj::mv(*dir)); + int ret = 1; + KJ_IF_MAYBE(exception, kj::runCatchingExceptions([&]() { + if (argc < 3) { + std::cerr << "Usage: " << PROXY_BIN << " SRC_PREFIX INCLUDE_PREFIX SRC_FILE [IMPORT_PATH...]\n"; + exit(1); + } + std::vector import_paths; + std::vector> import_dirs; + auto fs = kj::newDiskFilesystem(); + auto cwd = fs->getCurrentPath(); + kj::Own src_dir; + KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(argv[1]))) { + src_dir = kj::mv(*dir); } else { - throw std::runtime_error(std::string("Failed to open import directory: ") + argv[i]); + throw std::runtime_error(std::string("Failed to open src_prefix prefix directory: ") + argv[1]); } - } - for (const char* path : {CMAKE_INSTALL_PREFIX "/include", capnp_PREFIX "/include"}) { - KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(path))) { - import_paths.emplace_back(path); - import_dirs.emplace_back(kj::mv(*dir)); + for (int i = 4; i < argc; ++i) { + KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(argv[i]))) { + import_paths.emplace_back(argv[i]); + import_dirs.emplace_back(kj::mv(*dir)); + } else { + throw std::runtime_error(std::string("Failed to open import directory: ") + argv[i]); + } + } + for (const char* path : {CMAKE_INSTALL_PREFIX "/include", capnp_PREFIX "/include"}) { + KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(path))) { + import_paths.emplace_back(path); + import_dirs.emplace_back(kj::mv(*dir)); + } + // No exception thrown if _PREFIX directories do not exist } - // No exception thrown if _PREFIX directories do not exist + Generate(argv[1], argv[2], argv[3], import_paths, *src_dir, import_dirs); + ret = 0; + })) { + std::cerr << "mpgen error: " << kj::str(*exception).cStr() << '\n'; } - Generate(argv[1], argv[2], argv[3], import_paths, *src_dir, import_dirs); - return 0; + return ret; } From afac4b644ff943097999cef83c3df1325c523879 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 17 Apr 2026 11:04:35 -0400 Subject: [PATCH 17/19] doc: Remove trailing whitespace Bitcoin Core linter rejects it: https://github.com/bitcoin/bitcoin/actions/runs/24568789956/job/71835997334?pr=32387 --- doc/design.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/design.md b/doc/design.md index b8bc5fd0..319ee3de 100644 --- a/doc/design.md +++ b/doc/design.md @@ -120,7 +120,7 @@ sequenceDiagram participant PMT as ProxyMethodTraits participant Impl as Actual C++ Method - serverInvoke->>SF1: SF1::invoke + serverInvoke->>SF1: SF1::invoke SF1->>SF2: SF2::invoke SF2->>SR: SR::invoke SR->>SC: SC::invoke @@ -167,7 +167,7 @@ Thread mapping enables each client thread to have a dedicated server thread proc Thread mapping is initialized by defining an interface method with a `ThreadMap` parameter and/or response. The example below adds `ThreadMap` to the `construct` method because libmultiprocess calls the `construct` method automatically. ```capnp -interface InitInterface $Proxy.wrap("Init") { +interface InitInterface $Proxy.wrap("Init") { construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :Proxy.ThreadMap); } ``` From 998d700d608a439cf09201a417e03858d7b09f31 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Tue, 21 Apr 2026 16:43:46 -0400 Subject: [PATCH 18/19] cmake: Replace capnp_PREFIX path construction with cmake-provided symbols Use target_compile_definitions on mpgen to expose CAPNP_EXECUTABLE, CAPNPC_CXX_EXECUTABLE (via $ generator expressions on the CapnProto::capnp_tool and CapnProto::capnpc_cpp imported targets), and CAPNP_INCLUDE_DIRS (from the CAPNP_INCLUDE_DIRS variable set by find_package). gen.cpp uses these directly instead of constructing paths from capnp_PREFIX. Remove capnp_PREFIX from config.h.in as it is no longer needed there. Add compat fallbacks in compat_config.cmake to synthesize the tool imported targets and CAPNP_INCLUDE_DIRS from older variables when using an older CapnProto package. Co-Authored-By: Claude Sonnet 4.6 --- CMakeLists.txt | 4 ++++ cmake/compat_config.cmake | 24 ++++++++++++++++++++++++ include/mp/config.h.in | 1 - src/mp/gen.cpp | 8 ++++---- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6354b0ee..34fa6bd3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -209,6 +209,10 @@ target_link_libraries(mpgen PRIVATE CapnProto::capnp-rpc) target_link_libraries(mpgen PRIVATE CapnProto::capnpc) target_link_libraries(mpgen PRIVATE CapnProto::kj) target_link_libraries(mpgen PRIVATE Threads::Threads) +target_compile_definitions(mpgen PRIVATE + "CAPNP_EXECUTABLE=\"$\"" + "CAPNPC_CXX_EXECUTABLE=\"$\"" + "CAPNP_INCLUDE_DIRS=\"${CAPNP_INCLUDE_DIRS}\"") set_target_properties(mpgen PROPERTIES INSTALL_RPATH_USE_LINK_PATH TRUE) set_target_properties(mpgen PROPERTIES diff --git a/cmake/compat_config.cmake b/cmake/compat_config.cmake index f9d3004f..03d4b273 100644 --- a/cmake/compat_config.cmake +++ b/cmake/compat_config.cmake @@ -12,6 +12,30 @@ if (NOT DEFINED capnp_PREFIX AND DEFINED CAPNP_INCLUDE_DIRS) get_filename_component(capnp_PREFIX "${CAPNP_INCLUDE_DIRS}" DIRECTORY) endif() +if (NOT DEFINED CAPNP_INCLUDE_DIRS AND DEFINED capnp_PREFIX) + set(CAPNP_INCLUDE_DIRS "${capnp_PREFIX}/include") +endif() + +if (NOT TARGET CapnProto::capnp_tool) + if (DEFINED CAPNP_EXECUTABLE) + add_executable(CapnProto::capnp_tool IMPORTED GLOBAL) + set_target_properties(CapnProto::capnp_tool PROPERTIES IMPORTED_LOCATION "${CAPNP_EXECUTABLE}") + elseif (DEFINED capnp_PREFIX) + add_executable(CapnProto::capnp_tool IMPORTED GLOBAL) + set_target_properties(CapnProto::capnp_tool PROPERTIES IMPORTED_LOCATION "${capnp_PREFIX}/bin/capnp") + endif() +endif() + +if (NOT TARGET CapnProto::capnpc_cpp) + if (DEFINED CAPNPC_CXX_EXECUTABLE) + add_executable(CapnProto::capnpc_cpp IMPORTED GLOBAL) + set_target_properties(CapnProto::capnpc_cpp PROPERTIES IMPORTED_LOCATION "${CAPNPC_CXX_EXECUTABLE}") + elseif (DEFINED capnp_PREFIX) + add_executable(CapnProto::capnpc_cpp IMPORTED GLOBAL) + set_target_properties(CapnProto::capnpc_cpp PROPERTIES IMPORTED_LOCATION "${capnp_PREFIX}/bin/capnpc-c++") + endif() +endif() + if (NOT DEFINED CAPNPC_OUTPUT_DIR) set(CAPNPC_OUTPUT_DIR "${CMAKE_CURRENT_BINARY_DIR}") endif() diff --git a/include/mp/config.h.in b/include/mp/config.h.in index 9d3c6240..4a8c9168 100644 --- a/include/mp/config.h.in +++ b/include/mp/config.h.in @@ -6,7 +6,6 @@ #define MP_CONFIG_H #cmakedefine CMAKE_INSTALL_PREFIX "@CMAKE_INSTALL_PREFIX@" -#cmakedefine capnp_PREFIX "@capnp_PREFIX@" #cmakedefine HAVE_KJ_FILESYSTEM #cmakedefine HAVE_PTHREAD_GETNAME_NP @HAVE_PTHREAD_GETNAME_NP@ diff --git a/src/mp/gen.cpp b/src/mp/gen.cpp index 435ba441..7eb735b4 100644 --- a/src/mp/gen.cpp +++ b/src/mp/gen.cpp @@ -272,7 +272,7 @@ static void Generate(kj::StringPtr src_prefix, if (p != std::string::npos) include_base.erase(p); std::vector args; - args.emplace_back(capnp_PREFIX "/bin/capnp"); + args.emplace_back(CAPNP_EXECUTABLE); args.emplace_back("compile"); args.emplace_back("--src-prefix="); args.back().append(src_prefix.cStr(), src_prefix.size()); @@ -280,11 +280,11 @@ static void Generate(kj::StringPtr src_prefix, args.emplace_back("--import-path="); args.back().append(import_path.cStr(), import_path.size()); } - args.emplace_back("--output=" capnp_PREFIX "/bin/capnpc-c++"); + args.emplace_back("--output=" CAPNPC_CXX_EXECUTABLE); args.emplace_back(src_file); const int status = mp::WaitProcess(mp::StartProcess(args)); if (status) { - throw std::runtime_error("Invoking " capnp_PREFIX "/bin/capnp failed"); + throw std::runtime_error("Invoking " CAPNP_EXECUTABLE " failed"); } const capnp::SchemaParser parser; @@ -716,7 +716,7 @@ int main(int argc, char** argv) throw std::runtime_error(std::string("Failed to open import directory: ") + argv[i]); } } - for (const char* path : {CMAKE_INSTALL_PREFIX "/include", capnp_PREFIX "/include"}) { + for (const char* path : {CMAKE_INSTALL_PREFIX "/include", CAPNP_INCLUDE_DIRS}) { KJ_IF_MAYBE(dir, fs->getRoot().tryOpenSubdir(cwd.evalNative(path))) { import_paths.emplace_back(path); import_dirs.emplace_back(kj::mv(*dir)); From 6cc729f9312641c0299b774228f89494dd85494a Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Wed, 22 Apr 2026 15:35:05 -0400 Subject: [PATCH 19/19] cmake: Fix CapnProto tool paths broken by Ubuntu Noble packaging bug Ubuntu Noble's libcapnp-dev 1.0.1 cmake config file is installed under /usr/lib/x86_64-linux-gnu/cmake/CapnProto/ but its _IMPORT_PREFIX calculation goes up only 3 directory levels to /usr/lib instead of 4 levels to /usr, so IMPORTED_LOCATION for CapnProto::capnp_tool is set to /usr/lib/bin/capnp (non-existent) rather than /usr/bin/capnp. The previous compat_config.cmake fallback only fired when the target didn't exist at all (NOT TARGET), so it didn't catch this case where the target exists but has a wrong path. Add a validation pass that iterates over both tool targets after they are created (either by the package or by our own fallback). For each target, check whether any IMPORTED_LOCATION (config-specific or generic) resolves to an existing file. If none do, use find_program (with capnp_PREFIX/bin as a hint) to locate the actual binary and override all stored locations on that target. Co-Authored-By: Claude Sonnet 4.6 --- cmake/compat_config.cmake | 45 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/cmake/compat_config.cmake b/cmake/compat_config.cmake index 03d4b273..51bda36b 100644 --- a/cmake/compat_config.cmake +++ b/cmake/compat_config.cmake @@ -36,6 +36,51 @@ if (NOT TARGET CapnProto::capnpc_cpp) endif() endif() +# Validate CapnProto tool target locations and fix if broken. +# Some packaged capnproto versions (e.g., Ubuntu Noble libcapnp-dev 1.0.1) +# have incorrect IMPORTED_LOCATION paths due to a packaging bug where the cmake +# config file is installed under /usr/lib/.../cmake/ but the _IMPORT_PREFIX +# calculation goes up too few directory levels, yielding /usr/lib/bin/capnp +# instead of the correct /usr/bin/capnp. +foreach(_mp_tool IN ITEMS capnp_tool capnpc_cpp) + if (TARGET "CapnProto::${_mp_tool}") + get_target_property(_mp_configs "CapnProto::${_mp_tool}" IMPORTED_CONFIGURATIONS) + set(_mp_valid FALSE) + foreach(_mp_cfg IN LISTS _mp_configs) + get_target_property(_mp_loc "CapnProto::${_mp_tool}" "IMPORTED_LOCATION_${_mp_cfg}") + if (EXISTS "${_mp_loc}") + set(_mp_valid TRUE) + break() + endif() + endforeach() + if (NOT _mp_valid) + get_target_property(_mp_loc "CapnProto::${_mp_tool}" IMPORTED_LOCATION) + if (EXISTS "${_mp_loc}") + set(_mp_valid TRUE) + endif() + endif() + if (NOT _mp_valid) + if ("${_mp_tool}" STREQUAL "capnp_tool") + find_program(_mp_fixed capnp HINTS "${capnp_PREFIX}/bin") + else() + find_program(_mp_fixed capnpc-c++ HINTS "${capnp_PREFIX}/bin") + endif() + if (_mp_fixed) + foreach(_mp_cfg IN LISTS _mp_configs) + set_target_properties("CapnProto::${_mp_tool}" PROPERTIES "IMPORTED_LOCATION_${_mp_cfg}" "${_mp_fixed}") + endforeach() + set_target_properties("CapnProto::${_mp_tool}" PROPERTIES IMPORTED_LOCATION "${_mp_fixed}") + endif() + unset(_mp_fixed CACHE) + endif() + endif() +endforeach() +unset(_mp_tool) +unset(_mp_configs) +unset(_mp_valid) +unset(_mp_cfg) +unset(_mp_loc) + if (NOT DEFINED CAPNPC_OUTPUT_DIR) set(CAPNPC_OUTPUT_DIR "${CMAKE_CURRENT_BINARY_DIR}") endif()