Skip to content

moq-boy: exit non-zero on reconnect give-up instead of hanging#1589

Merged
kixelated merged 2 commits into
mainfrom
claude/hopeful-tharp-6af1b9
Jun 2, 2026
Merged

moq-boy: exit non-zero on reconnect give-up instead of hanging#1589
kixelated merged 2 commits into
mainfrom
claude/hopeful-tharp-6af1b9

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Problem

moq-boy gives up reconnecting to the relay after the mandatory 5m backoff timeout (added in #1443), but the process doesn't exit — it hangs as a zombie. This defeats systemd's Restart=always, so a relay outage (or any future deploy/relay rebuild) longer than the reconnect window silently kills the games and they never come back.

Root cause

The emulator runs on a spawn_blocking thread that loops forever (rs/moq-boy/src/main.rs:274). When the reconnect loop gives up, run() returns Err, main returns it, and the tokio runtime is dropped. Dropping the runtime blocks indefinitely joining that never-ending blocking thread, so the process hangs instead of exiting. The ctrl_c branch already side-stepped this with an explicit std::process::exit(0); the give-up path did not.

Fix

Capture the select! result and exit explicitly via std::process::exit (0 on clean shutdown, 1 on error) instead of returning from main. process::exit terminates immediately without dropping the runtime, so it can't hang on the blocking thread, and a real non-zero exit code reaches systemd. The give-up error is logged on the way out.

The 5m timeout is kept as-is (a permanent relay outage still surfaces as a non-zero exit rather than silently spinning forever), so Restart=always now correctly restarts the service.

moq-cli

No change needed. moq-cli has no spawn_blocking task, so returning Err from main already exits non-zero cleanly. It shares the same 5m give-up shape but never happened to hit the window.

Test plan

  • cargo check -p moq-boy
  • cargo clippy -p moq-boy (via nix toolchain)

(Written by Claude)

The emulator runs on a spawn_blocking thread that loops forever. When the
reconnect loop gives up after the 5m backoff timeout, run() returns Err and
main returns it, which drops the tokio runtime. Dropping the runtime blocks
indefinitely joining the blocking thread, so the process hangs as a zombie
instead of exiting. systemd's Restart=always never fires, silently killing the
games after a relay outage longer than the reconnect window.

Exit explicitly via std::process::exit (0 on clean shutdown, 1 on error) so a
real exit code reaches the supervisor without waiting on the runtime drop. The
ctrl_c branch already did this; extend it to every exit path.

moq-cli has no blocking task, so returning Err already exits non-zero cleanly;
no change needed there.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kixelated
kixelated enabled auto-merge (squash) June 2, 2026 02:45
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2437c185-014d-42b8-99a4-5288c00a1a1c

📥 Commits

Reviewing files that changed from the base of the PR and between f5a4ffe and b58e454.

📒 Files selected for processing (1)
  • rs/moq-boy/src/main.rs

Walkthrough

The PR changes rs/moq-boy/src/main.rs so the top-level tokio::select! result is captured into a variable. The Ctrl-C branch now returns Ok(()) instead of calling std::process::exit directly. After the select!, main matches the result: it logs and exits with code 1 on error, or exits with code 0 on success, preventing implicit runtime drop while background tasks run.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing moq-boy to exit with a non-zero code when reconnect attempts are exhausted, rather than hanging.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, clearly explaining the problem, root cause, fix, and testing approach.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/hopeful-tharp-6af1b9

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rs/moq-boy/src/main.rs`:
- Around line 449-453: The select arm handling Ctrl-C currently swallows any
io::Error by mapping tokio::signal::ctrl_c() to Ok(()); update the
tokio::select! arms (the block that awaits run(&config), jemalloc, and
tokio::signal::ctrl_c()) to propagate the Err from tokio::signal::ctrl_c()
instead of treating it as a clean shutdown — replace the `_ =
tokio::signal::ctrl_c() => Ok(())` arm with one that returns the Result from
tokio::signal::ctrl_c() (and add context as needed) so failures registering the
ctrl-c handler bubble up alongside errors from run and jemalloc.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 72411194-69f1-4686-a06f-a2f658fe9e3d

📥 Commits

Reviewing files that changed from the base of the PR and between b7a08ff and f5a4ffe.

📒 Files selected for processing (1)
  • rs/moq-boy/src/main.rs

Comment thread rs/moq-boy/src/main.rs
tokio::signal::ctrl_c() returns io::Result<()>; the `_ =` select arm matched
both Ok and Err, so a failure registering the SIGINT handler would exit the
process 0 at startup. Bind the result and add context so the error bubbles up
alongside run() and jemalloc errors and exits non-zero.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kixelated
kixelated disabled auto-merge June 2, 2026 02:54
@kixelated
kixelated merged commit ebf2999 into main Jun 2, 2026
@kixelated
kixelated deleted the claude/hopeful-tharp-6af1b9 branch June 2, 2026 02:54
@moq-bot moq-bot Bot mentioned this pull request Jun 2, 2026
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.

1 participant