Skip to content

Fix startup race in example#315

Open
xyzconstant wants to merge 2 commits into
bitcoin-core:masterfrom
xyzconstant:fix-race-in-mpexample
Open

Fix startup race in example#315
xyzconstant wants to merge 2 commits into
bitcoin-core:masterfrom
xyzconstant:fix-race-in-mpexample

Conversation

@xyzconstant

@xyzconstant xyzconstant commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Since #274, connection setup has been split into two calls: MakeStream followed by ConnectStream. Between these calls, the event loop's reference count can temporarily drop to zero, allowing EventLoop::loop() to exit (and the EventLoop to be destroyed) before ConnectStream posts its work. This crashes mpexample on startup.

Fix this by keeping an EventLoopRef alive in main() so the loop stays running while it is in use. This same pattern is already used in Bitcoin Core (m_loop_ref member of CapnpProtocol).

A second commit documents the reference-counted EventLoop lifetime in the EventLoopRef class comment and in doc/usage.md.

NOTE: On macOS, mpexample was crashing intermittently on startup (sometimes failing the m_post_fn == nullptr assert in the EventLoop destructor, sometimes with a mutex lock failure), depending on timing. With this change, the crashes are gone.

@DrahtBot

DrahtBot commented Jul 22, 2026

Copy link
Copy Markdown

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

Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers
ACK ViniciusCestarii

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

@xyzconstant
xyzconstant force-pushed the fix-race-in-mpexample branch from b031c1f to ff9f84c Compare July 22, 2026 03:59
@xyzconstant

Copy link
Copy Markdown
Contributor Author

failed CI job seems unrelated

Since bitcoin-core#274, connection setup has been split into two calls: `MakeStream`
followed by `ConnectStream`. Between these calls, the event loop's reference
count can temporarily drop to zero, allowing `EventLoop::loop()` to exit
before `ConnectStream` posts its work, crashing the example on startup.

Keep an `EventLoopRef` alive in main() so the loop stays running while it
is in use.
`EventLoop::loop()` exits when the last `EventLoopRef` is released, so
code making multiple calls against the loop needs to hold its own
reference. State this in the `EventLoopRef` comment and usage.md.
@xyzconstant
xyzconstant force-pushed the fix-race-in-mpexample branch from ff9f84c to f2fabf6 Compare July 22, 2026 04:19

@ViniciusCestarii ViniciusCestarii left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK f2fabf6 nice catch! I was able to reproduce on linux and it also intermittently fails without this fix. I liked the doc explanation too.

nit: a more surgical fix for against against master could look like:

+#include "mp/proxy.h"
 #include <init.capnp.h>
 #include <init.capnp.proxy.h>
 
int main(int argc, char** argv)
         loop.loop();
     });
     mp::EventLoop* loop = promise.get_future().get();
+    mp::EventLoopRef loop_ref{*loop};
 
     auto [printer_init, printer_pid] = Spawn(*loop, argv[0], "mpprinter");
     auto [calc_init, calc_pid] = Spawn(*loop, argv[0], "mpcalculator");
int main(int argc, char** argv)
     mp::WaitProcess(calc_pid);
     printer_init.reset();
     mp::WaitProcess(printer_pid);
+    loop_ref.reset();
     loop_thread.join();
     std::cout << "Bye!" << std::endl;
     return 0;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants