Skip to content

Commit 197b2aa

Browse files
committed
ipc: Use EventLoopRef instead of addClient/removeClient
Use EventLoopRef to avoid reference counting bugs and be more exception safe and deal with removal of addClient/removeClient methods in bitcoin-core/libmultiprocess#160 A test update is also required due to bitcoin-core/libmultiprocess#160 to deal with changed reference count semantics. In IpcPipeTest(), it is is now necessary to destroy the client Proxy object instead of just the client Connection object to decrease the event loop reference count and allow the loop to exit so the test does not hang on shutdown.
1 parent b9e16ff commit 197b2aa

File tree

2 files changed

+7
-14
lines changed

2 files changed

+7
-14
lines changed

src/ipc/capnp/protocol.cpp

+3-8
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,7 @@ class CapnpProtocol : public Protocol
4141
public:
4242
~CapnpProtocol() noexcept(true)
4343
{
44-
if (m_loop) {
45-
std::unique_lock<std::mutex> lock(m_loop->m_mutex);
46-
m_loop->removeClient(lock);
47-
}
44+
m_loop_ref.reset();
4845
if (m_loop_thread.joinable()) m_loop_thread.join();
4946
assert(!m_loop);
5047
};
@@ -83,10 +80,7 @@ class CapnpProtocol : public Protocol
8380
m_loop_thread = std::thread([&] {
8481
util::ThreadRename("capnp-loop");
8582
m_loop.emplace(exe_name, &IpcLogFn, &m_context);
86-
{
87-
std::unique_lock<std::mutex> lock(m_loop->m_mutex);
88-
m_loop->addClient(lock);
89-
}
83+
m_loop_ref.emplace(*m_loop);
9084
promise.set_value();
9185
m_loop->loop();
9286
m_loop.reset();
@@ -96,6 +90,7 @@ class CapnpProtocol : public Protocol
9690
Context m_context;
9791
std::thread m_loop_thread;
9892
std::optional<mp::EventLoop> m_loop;
93+
std::optional<mp::EventLoopRef> m_loop_ref;
9994
};
10095
} // namespace
10196

src/test/ipc_test.cpp

+4-6
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,17 @@ static std::string TempPath(std::string_view pattern)
5353
//! on the object through FooInterface (defined in ipc_test.capnp).
5454
void IpcPipeTest()
5555
{
56-
// Setup: create FooImplemention object and listen for FooInterface requests
56+
// Setup: create FooImplementation object and listen for FooInterface requests
5757
std::promise<std::unique_ptr<mp::ProxyClient<gen::FooInterface>>> foo_promise;
58-
std::function<void()> disconnect_client;
5958
std::thread thread([&]() {
6059
mp::EventLoop loop("IpcPipeTest", [](bool raise, const std::string& log) { LogPrintf("LOG%i: %s\n", raise, log); });
6160
auto pipe = loop.m_io_context.provider->newTwoWayPipe();
6261

6362
auto connection_client = std::make_unique<mp::Connection>(loop, kj::mv(pipe.ends[0]));
6463
auto foo_client = std::make_unique<mp::ProxyClient<gen::FooInterface>>(
6564
connection_client->m_rpc_system->bootstrap(mp::ServerVatId().vat_id).castAs<gen::FooInterface>(),
66-
connection_client.get(), /* destroy_connection= */ false);
65+
connection_client.release(), /* destroy_connection= */ true);
6766
foo_promise.set_value(std::move(foo_client));
68-
disconnect_client = [&] { loop.sync([&] { connection_client.reset(); }); };
6967

7068
auto connection_server = std::make_unique<mp::Connection>(loop, kj::mv(pipe.ends[1]), [&](mp::Connection& connection) {
7169
auto foo_server = kj::heap<mp::ProxyServer<gen::FooInterface>>(std::make_shared<FooImplementation>(), connection);
@@ -125,8 +123,8 @@ void IpcPipeTest()
125123
auto script2{foo->passScript(script1)};
126124
BOOST_CHECK_EQUAL(HexStr(script1), HexStr(script2));
127125

128-
// Test cleanup: disconnect pipe and join thread
129-
disconnect_client();
126+
// Test cleanup: disconnect and join thread
127+
foo.reset();
130128
thread.join();
131129
}
132130

0 commit comments

Comments
 (0)