Skip to content

Commit 3f221b5

Browse files
committed
Merge #274: Add nonunix platform support
1b0f605 doc: Remove trailing whitespace (Ryan Ofsky) d8f8ca3 ipc: Wrap mpgen main() in try-catch to print errors (Ryan Ofsky) fbe5a14 ci: Check out bitcoin/bitcoin PR #35084 instead of master (Ryan Ofsky) 39d3690 types: Replace SFINAE with requires clauses to avoid MSVC C2039 error (Ryan Ofsky) ba68520 proxy, refactor: Fix C4305 truncation warning in Accessor on MSVC (Ryan Ofsky) 1d81d47 util, refactor: Fix PtrOrValue constructor for move-only types on MSVC (Ryan Ofsky) b883fe1 proxy: Fix shutdownWrite() exception handling on macOS with dynamic libraries (Ryan Ofsky) 0012411 proxy: Call shutdownWrite() in Connection destructor (Ryan Ofsky) 38312ad proxy, refactor: Change ConnectStream and ServeStream to accept stream objects (Ryan Ofsky) e96d5d7 proxy, refactor: Replace EventLoop wakeup fd integers with KJ stream objects (Ryan Ofsky) db4f9a3 cmake: Bump minimum required Cap'n Proto version to 0.9 (Ryan Ofsky) 652934f util, refactor: Add SocketPair() and use it in SpawnProcess (Ryan Ofsky) 1c6ef7a util, refactor: Do not fork() and exec() separately (Ryan Ofsky) 1389cf3 util, refactor: Add SpawnConnectInfo type alias and use it (Ryan Ofsky) c7ca1f0 util, refactor: Add SocketId type alias and use it (Ryan Ofsky) be46a35 util, refactor: Add ProcessId type alias and use it (Ryan Ofsky) 91a78db doc: Bump version 13 > 14 (Ryan Ofsky) Pull request description: This PR implements API changes and fixes needed to allow libmultiprocess to work on nonunix platforms. These changes were originally part of #231, which adds windows support, but were split out to allow windows and nonwindows changes to be reviewed separately. ACKs for top commit: Sjors: ACK 1b0f605 ViniciusCestarii: ACK 1b0f605 I like that these changes make the code more readable too, require clauses and KJ Stream abstractions are nice Tree-SHA512: 3f9e92487b29734f5681f797786a317c708c6df3e5242189729955f3a797e944a8db1d2af2863b570928582b1a202ee761ca7c9758744e80840fa16bc0d188e2
2 parents e8de5c7 + 1b0f605 commit 3f221b5

22 files changed

Lines changed: 295 additions & 193 deletions

.github/workflows/bitcoin-core-ci.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ concurrency:
1818

1919
env:
2020
BITCOIN_REPO: bitcoin/bitcoin
21+
# Temporary: use PR #35084 until it merges; revert to refs/heads/master after
22+
BITCOIN_CORE_REF: refs/pull/35084/merge
2123
LLVM_VERSION: 22
2224
LIBCXX_DIR: /tmp/libcxx-build/
2325

@@ -78,6 +80,7 @@ jobs:
7880
uses: actions/checkout@v4
7981
with:
8082
repository: ${{ env.BITCOIN_REPO }}
83+
ref: ${{ env.BITCOIN_CORE_REF }}
8184
fetch-depth: 1
8285

8386
- name: Checkout libmultiprocess
@@ -194,6 +197,7 @@ jobs:
194197
uses: actions/checkout@v4
195198
with:
196199
repository: ${{ env.BITCOIN_REPO }}
200+
ref: ${{ env.BITCOIN_CORE_REF }}
197201
fetch-depth: 1
198202

199203
- name: Checkout libmultiprocess

CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ endif()
1313
include("cmake/compat_find.cmake")
1414

1515
find_package(Threads REQUIRED)
16-
find_package(CapnProto 0.7 QUIET NO_MODULE)
16+
find_package(CapnProto 0.9 QUIET NO_MODULE)
1717
if(NOT CapnProto_FOUND)
1818
message(FATAL_ERROR
1919
"Cap'n Proto is required but was not found.\n"

ci/configs/olddeps.bash

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
CI_DESC="CI job using old Cap'n Proto and cmake versions"
22
CI_DIR=build-olddeps
33
export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wno-unused-parameter -Wno-error=array-bounds"
4-
NIX_ARGS=(--argstr capnprotoVersion "0.7.1" --argstr cmakeVersion "3.12.4")
4+
NIX_ARGS=(--argstr capnprotoVersion "0.9.2" --argstr cmakeVersion "3.12.4")
55
BUILD_ARGS=(-k)

doc/design.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ sequenceDiagram
120120
participant PMT as ProxyMethodTraits
121121
participant Impl as Actual C++ Method
122122
123-
serverInvoke->>SF1: SF1::invoke
123+
serverInvoke->>SF1: SF1::invoke
124124
SF1->>SF2: SF2::invoke
125125
SF2->>SR: SR::invoke
126126
SR->>SC: SC::invoke
@@ -167,7 +167,7 @@ Thread mapping enables each client thread to have a dedicated server thread proc
167167
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.
168168

169169
```capnp
170-
interface InitInterface $Proxy.wrap("Init") {
170+
interface InitInterface $Proxy.wrap("Init") {
171171
construct @0 (threadMap: Proxy.ThreadMap) -> (threadMap :Proxy.ThreadMap);
172172
}
173173
```

doc/versions.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,15 @@ Library versions are tracked with simple
77
Versioning policy is described in the [version.h](../include/mp/version.h)
88
include.
99

10-
## v13
10+
## v14
11+
- Adds support for nonunix platforms, making API changes that are not backwards compatible ([#274](https://github.com/bitcoin-core/libmultiprocess/pull/274)).
1112
- Current unstable version.
1213

14+
## [v13.0](https://github.com/bitcoin-core/libmultiprocess/commits/v13.0)
15+
- Fixes a `memcpy` ubsan warning by using `std::ranges::copy` ([#305](https://github.com/bitcoin-core/libmultiprocess/pull/305)).
16+
- Misc: map serialization round-trip test coverage ([#297](https://github.com/bitcoin-core/libmultiprocess/pull/297)) and version bump ([#306](https://github.com/bitcoin-core/libmultiprocess/pull/306)).
17+
- Used in Bitcoin Core 32.x and newer releases with [#35720](https://github.com/bitcoin/bitcoin/pull/35720).
18+
1319
## [v12.0](https://github.com/bitcoin-core/libmultiprocess/commits/v12.0)
1420
- Adds an optional `max_connections` parameter to `ListenConnections` ([#269](https://github.com/bitcoin-core/libmultiprocess/pull/269)).
1521
- Used in Bitcoin Core 32.x and newer releases with [#35684](https://github.com/bitcoin/bitcoin/pull/35684).
@@ -24,7 +30,7 @@ include.
2430
- Fixes a rare mptest hang on musl builds caused by a lost wakeup bug in `Waiter` ([#295](https://github.com/bitcoin-core/libmultiprocess/pull/295)).
2531
- Fixes a race condition in a log print detected by TSan ([#286](https://github.com/bitcoin-core/libmultiprocess/pull/286)).
2632
- Build improvements: makes `target_capnp_sources` work correctly when libmultiprocess is used as a CMake subproject ([#289](https://github.com/bitcoin-core/libmultiprocess/pull/289)), adds `mp_headers` target for better lint tool support ([#291](https://github.com/bitcoin-core/libmultiprocess/pull/291)), and fixes compatibility with recent Nix and CMake 4.0 ([#238](https://github.com/bitcoin-core/libmultiprocess/pull/238)).
27-
- Test, CI, documentation, and minor code improvements: design document corrections ([#278](https://github.com/bitcoin-core/libmultiprocess/pull/278)), field constant comments ([#279](https://github.com/bitcoin-core/libmultiprocess/pull/279)), clang-tidy fix ([#292](https://github.com/bitcoin-core/libmultiprocess/pull/292)), new smoke test for double-precision float values ([#294](https://github.com/bitcoin-core/libmultiprocess/pull/294)), new test for recursive async IPC calls ([#301](https://github.com/bitcoin-core/libmultiprocess/pull/301)), removal of libevent from Core CI builds ([#299](https://github.com/bitcoin-core/libmultiprocess/pull/299)), and rename of `EventLoop::m_num_clients` to `m_num_refs` ([#302](https://github.com/bitcoin-core/libmultiprocess/pull/302)).
33+
- Test, CI, documentation, and minor code improvements: design document corrections ([#278](https://github.com/bitcoin-core/libmultiprocess/pull/278)), field constant comments ([#279](https://github.com/bitcoin-core/libmultiprocess/pull/279)), clang-tidy fix ([#292](https://github.com/bitcoin-core/libmultiprocess/pull/292)), new smoke test for double-precision float values ([#294](https://github.com/bitcoin-core/libmultiprocess/pull/294)), new test for recursive async IPC calls ([#301](https://github.com/bitcoin-core/libmultiprocess/pull/301)), removal of libevent from Core CI builds ([#299](https://github.com/bitcoin-core/libmultiprocess/pull/299)), rename of `EventLoop::m_num_clients` to `m_num_refs` ([#302](https://github.com/bitcoin-core/libmultiprocess/pull/302)), and version bump ([#270](https://github.com/bitcoin-core/libmultiprocess/pull/270)).
2834
- Used in Bitcoin Core 32.x and newer releases with [#35661](https://github.com/bitcoin/bitcoin/pull/35661).
2935

3036
## [v10.0](https://github.com/bitcoin-core/libmultiprocess/commits/v10.0)

example/calculator.cpp

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
#include <init.capnp.h>
77
#include <init.capnp.proxy.h> // NOLINT(misc-include-cleaner) // IWYU pragma: keep
88

9-
#include <charconv>
10-
#include <cstring>
9+
#include <cstring> // IWYU pragma: keep
1110
#include <fstream>
1211
#include <functional>
1312
#include <iostream>
@@ -16,9 +15,9 @@
1615
#include <kj/memory.h>
1716
#include <memory>
1817
#include <mp/proxy-io.h>
18+
#include <mp/util.h>
1919
#include <stdexcept>
2020
#include <string>
21-
#include <system_error>
2221
#include <utility>
2322

2423
class CalculatorImpl : public Calculator
@@ -51,14 +50,10 @@ int main(int argc, char** argv)
5150
std::cout << "Usage: mpcalculator <fd>\n";
5251
return 1;
5352
}
54-
int fd;
55-
if (std::from_chars(argv[1], argv[1] + strlen(argv[1]), fd).ec != std::errc{}) {
56-
std::cerr << argv[1] << " is not a number or is larger than an int\n";
57-
return 1;
58-
}
53+
mp::SocketId socket{mp::StartSpawned(argv[1])};
5954
mp::EventLoop loop("mpcalculator", LogPrint);
6055
std::unique_ptr<Init> init = std::make_unique<InitImpl>();
61-
mp::ServeStream<InitInterface>(loop, fd, *init);
56+
mp::ServeStream<InitInterface>(loop, mp::MakeStream(loop, socket), *init);
6257
loop.loop();
6358
return 0;
6459
}

example/example.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,20 @@
1919
#include <string>
2020
#include <thread>
2121
#include <tuple>
22+
#include <utility>
2223
#include <vector>
2324

2425
namespace fs = std::filesystem;
2526

2627
static auto Spawn(mp::EventLoop& loop, const std::string& process_argv0, const std::string& new_exe_name)
2728
{
28-
int pid;
29-
const int fd = mp::SpawnProcess(pid, [&](int fd) -> std::vector<std::string> {
29+
const auto [pid, socket] = mp::SpawnProcess([&](mp::SpawnConnectInfo info) -> std::vector<std::string> {
3030
fs::path path = process_argv0;
3131
path.remove_filename();
3232
path.append(new_exe_name);
33-
return {path.string(), std::to_string(fd)};
33+
return {path.string(), std::move(info)};
3434
});
35-
return std::make_tuple(mp::ConnectStream<InitInterface>(loop, fd), pid);
35+
return std::make_tuple(mp::ConnectStream<InitInterface>(loop, mp::MakeStream(loop, socket)), pid);
3636
}
3737

3838
static void LogPrint(mp::LogMessage log_data)

example/printer.cpp

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,17 @@
77
#include <init.capnp.h>
88
#include <init.capnp.proxy.h> // NOLINT(misc-include-cleaner) // IWYU pragma: keep
99

10-
#include <charconv>
11-
#include <cstring>
10+
#include <cstring> // IWYU pragma: keep
1211
#include <fstream>
1312
#include <iostream>
1413
#include <kj/async.h>
1514
#include <kj/common.h>
1615
#include <kj/memory.h>
1716
#include <memory>
1817
#include <mp/proxy-io.h>
18+
#include <mp/util.h>
1919
#include <stdexcept>
2020
#include <string>
21-
#include <system_error>
2221

2322
class PrinterImpl : public Printer
2423
{
@@ -44,14 +43,10 @@ int main(int argc, char** argv)
4443
std::cout << "Usage: mpprinter <fd>\n";
4544
return 1;
4645
}
47-
int fd;
48-
if (std::from_chars(argv[1], argv[1] + strlen(argv[1]), fd).ec != std::errc{}) {
49-
std::cerr << argv[1] << " is not a number or is larger than an int\n";
50-
return 1;
51-
}
46+
mp::SocketId socket{mp::StartSpawned(argv[1])};
5247
mp::EventLoop loop("mpprinter", LogPrint);
5348
std::unique_ptr<Init> init = std::make_unique<InitImpl>();
54-
mp::ServeStream<InitInterface>(loop, fd, *init);
49+
mp::ServeStream<InitInterface>(loop, mp::MakeStream(loop, socket), *init);
5550
loop.loop();
5651
return 0;
5752
}

include/mp/proxy-io.h

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <cstdlib>
1919
#include <functional>
2020
#include <kj/function.h>
21+
#include <kj/io.h>
2122
#include <map>
2223
#include <memory>
2324
#include <optional>
@@ -213,6 +214,9 @@ class Logger
213214

214215
std::string LongThreadName(const char* exe_name);
215216

217+
//! Wrap a socket file descriptor as an async stream, taking ownership of the fd.
218+
Stream MakeStream(EventLoop&loop, SocketId socket);
219+
216220
//! Event loop implementation.
217221
//!
218222
//! Cap'n Proto threading model is very simple: all I/O operations are
@@ -311,11 +315,12 @@ class EventLoop
311315
//! Callback functions to run on async thread.
312316
std::optional<CleanupList> m_async_fns MP_GUARDED_BY(m_mutex);
313317

314-
//! Pipe read handle used to wake up the event loop thread.
315-
int m_wait_fd = -1;
318+
//! Socket pair used to post and wait for wakeups to the event loop thread.
319+
kj::Own<kj::AsyncIoStream> m_wait_stream;
320+
kj::Own<kj::AsyncIoStream> m_post_stream;
316321

317-
//! Pipe write handle used to wake up the event loop thread.
318-
int m_post_fd = -1;
322+
//! Synchronous writer used to write to m_post_stream.
323+
kj::Own<kj::OutputStream> m_post_writer;
319324

320325
//! Number of EventLoopRef instances referencing this event loop. This is a
321326
//! sum of the number of client and server objects (Connection, ProxyClient,
@@ -451,7 +456,7 @@ class Connection
451456
//! Capability::Client handles owned by ProxyClient objects), then schedules
452457
//! asynchronous cleanup functions to run in a worker thread (to run
453458
//! destructors of m_impl instances owned by ProxyServer objects).
454-
~Connection();
459+
~Connection() noexcept(false);
455460

456461
//! Register synchronous cleanup function to run on event loop thread (with
457462
//! access to capnp thread local variables) when disconnect() is called.
@@ -824,17 +829,15 @@ kj::Promise<T> ProxyServer<Thread>::post(Fn&& fn)
824829
return ret;
825830
}
826831

827-
//! Given stream file descriptor, make a new ProxyClient object to send requests
828-
//! over the stream. Also create a new Connection object embedded in the
829-
//! client that is freed when the client is closed.
832+
//! Given a stream, make a new ProxyClient object to send requests over it.
833+
//! Also create a new Connection object embedded in the client that is freed
834+
//! when the client is closed.
830835
template <typename InitInterface>
831-
std::unique_ptr<ProxyClient<InitInterface>> ConnectStream(EventLoop& loop, int fd)
836+
std::unique_ptr<ProxyClient<InitInterface>> ConnectStream(EventLoop& loop, Stream stream)
832837
{
833838
typename InitInterface::Client init_client(nullptr);
834839
std::unique_ptr<Connection> connection;
835840
loop.sync([&] {
836-
auto stream =
837-
loop.m_io_context.lowLevelProvider->wrapSocketFd(fd, kj::LowLevelAsyncIoProvider::TAKE_OWNERSHIP);
838841
connection = std::make_unique<Connection>(loop, kj::mv(stream));
839842
init_client = connection->m_rpc_system->bootstrap(ServerVatId().vat_id).castAs<InitInterface>();
840843
Connection* connection_ptr = connection.get();
@@ -905,22 +908,18 @@ void _Listen(const std::shared_ptr<Listener>& listener, EventLoop& loop, InitImp
905908
}));
906909
}
907910

908-
//! Given stream file descriptor and an init object, handle requests on the
909-
//! stream by calling methods on the Init object.
911+
//! Given a stream and an init object, handle requests on the stream by calling
912+
//! methods on the Init object.
910913
template <typename InitInterface, typename InitImpl>
911-
void ServeStream(EventLoop& loop, int fd, InitImpl& init)
914+
void ServeStream(EventLoop& loop, Stream stream, InitImpl& init)
912915
{
913-
_Serve<InitInterface>(
914-
loop,
915-
loop.m_io_context.lowLevelProvider->wrapSocketFd(fd, kj::LowLevelAsyncIoProvider::TAKE_OWNERSHIP),
916-
init,
917-
[] {});
916+
_Serve<InitInterface>(loop, kj::mv(stream), init, [] {});
918917
}
919918

920-
//! Given listening socket file descriptor and an init object, handle incoming
919+
//! Given listening socket identifier and an init object, handle incoming
921920
//! connections and requests by calling methods on the Init object.
922921
template <typename InitInterface, typename InitImpl>
923-
void ListenConnections(EventLoop& loop, int fd, InitImpl& init, std::optional<size_t> max_connections = std::nullopt)
922+
void ListenConnections(EventLoop& loop, SocketId fd, InitImpl& init, std::optional<size_t> max_connections = std::nullopt)
924923
{
925924
loop.sync([&]() {
926925
auto listener{std::make_shared<Listener>(

include/mp/proxy.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -315,21 +315,21 @@ template <typename Field, int flags>
315315
struct Accessor : public Field
316316
{
317317
//! Field is present from the Cap'n Proto Params struct (client -> server).
318-
static const bool in = flags & FIELD_IN;
318+
static constexpr bool in = (flags & FIELD_IN) != 0;
319319
//! Field is present from the Cap'n Proto Results struct (server -> client).
320-
static const bool out = flags & FIELD_OUT;
320+
static constexpr bool out = (flags & FIELD_OUT) != 0;
321321
//! Field has a companion has{Name} boolean field in the Cap'n Proto struct.
322322
//! This is used to represent optional primitive values (e.g. C++
323323
//! std::optional<int>) because Cap'n Proto doesn't allow primitive fields to
324324
//! be unset.
325-
static const bool optional = flags & FIELD_OPTIONAL;
325+
static constexpr bool optional = (flags & FIELD_OPTIONAL) != 0;
326326
//! Results field has a companion want{Name} boolean field in the Params
327327
//! struct. Used for optional output parameters (e.g. C++ int*) and set to
328328
//! true if the caller passed a non-null pointer and wants the result.
329-
static const bool requested = flags & FIELD_REQUESTED;
329+
static constexpr bool requested = (flags & FIELD_REQUESTED) != 0;
330330
//! Field is a Cap'n Proto pointer type (struct, list, text, data,
331331
//! interface) as opposed to a primitive type (bool, int, float, enum).
332-
static const bool boxed = flags & FIELD_BOXED;
332+
static constexpr bool boxed = (flags & FIELD_BOXED) != 0;
333333
};
334334

335335
//! Wrapper around std::function for passing std::function objects between client and servers.

0 commit comments

Comments
 (0)