ipc: hold event loop references when using it in `CapnpProtocol` #36572

pull xyzconstant wants to merge 1 commits into bitcoin:master from xyzconstant:2026-10-16-fix-capnp-protocol-races changing 1 files +68 −23

xyzconstant commented at 1:51 AM on October 19, 2026: contributor

EventLoop::loop() exits when its reference count reaches zero. In serve() mode, the parent connection is the only reference keeping the loop running. Other CapnpProtocol methods called from threads not handling parent IPC requests do not hold their own references and therefore depend on the parent connection remaining alive.

Add loopRef() (code suggested by @ryanofsky in https://github.com/bitcoin-core/libmultiprocess/pull/376#discussion_r4268952676), which returns a new reference to the event loop or throws if the loop is not running or is shutting down, and use it in the methods that need the loop. Protect m_loop with a new m_loop_mutex to prevent serve() from destroying the loop while loopRef() is accessing it.

ipc: hold event loop references when using it in CapnpProtocol

EventLoop::loop() exits when its reference count reaches zero. In serve()
mode, the parent connection is the only reference keeping the loop running.
Other CapnpProtocol methods called from threads not handling parent IPC
requests do not hold their own references and therefore depend on the
parent connection remaining alive.

Add loopRef(), which returns a new reference to the event loop or throws
if the loop is not running or is shutting down, and use it in the methods
that need the loop. Protect m_loop with a new m_loop_mutex to prevent
serve() from destroying the loop while loopRef() is accessing it.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>

dcae543524

DrahtBot commented at 11:46 AM on October 19, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36572
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36572.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36572#author-ralph


Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #36268 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36268.svg"></sub> (refactor: prune unused semi-colons by fanquake)
- #35932 <sub><img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/35932.svg"></sub> (ipc: make ipc::disconnectIncoming wait for in-progress calls to complete by ryanofsky)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in src/ipc/capnp/protocol.cpp:81 in dcae543524

  76 | @@ -76,77 +77,121 @@ class CapnpProtocol : public Protocol
  77 |      {
  78 |          m_loop_ref.reset();
  79 |          if (m_loop_thread.joinable()) m_loop_thread.join();
  80 | -        assert(!m_loop);
  81 | +        assert(WITH_LOCK(m_loop_mutex, return !m_loop));
  82 |      };

Husteem commented at 1:34 PM on October 20, 2026:

A tiny Nit: redundant trailing semicolon.

Hamza1610 commented at 4:27 PM on October 20, 2026: none

Tested Ack dcae5435245ec01c49cc81f1eeec438476752e3d I tested the changes using ./build/bin/test_bitcoin -t ipc_tests and confirm, no error is detected
DrahtBot added the label Needs rebase on Oct 29, 2026
DrahtBot commented at 7:00 PM on October 29, 2026: contributor



🐙 This pull request conflicts with the target branch and needs rebase.

Contributors

Labels

ipc: hold event loop references when using it in `CapnpProtocol` #36572

External sites

Reviews

Conflicts