[kixelated]

Finishes quest/m0/clean-quit.md, from moq-dev/studio#23 and moq-dev/studio#22.

Problem

Engine::stop drained and finished the program video, but main then returned and dropped the runtime, so each relay session went with the tail still on its way. Source threads were never stopped at exit. And Engine::start started the program thread before the audio and autosave threads, so one of those failing to start left the program thread running, holding the engine and its publication beside the retry setup makes.

Approach

• Sources stop at quit. New moq_scene::source::Sources::stop drops every feed, as removing a source would, so each driver returns and its own track finishes cleanly (the existing Drop paths of Video, Audio, and Still). Engine::stop calls it after joining the program and audio threads, before stopping the browser helper.
• Engine::close (async), run by main after stop and beside recorder.done(), before the runtime drops:
  • waits for each source thread to end. spawn now returns a oneshot receiver that closes as the thread's task ends; the engine keeps them, forgetting ended ones as new sources start.
  • Output::close closes every relay connection in parallel with moq_tokio::Connection::close, which withdraws the announcement and drains finished tracks' tails (bounded at one second by the session), then Client::close so the relay gets the QUIC close rather than waiting out its idle timeout. Output now keeps each relay's client for that.
  • the whole close is bounded at 3 s (CLOSE), so a source or relay that never answers cannot hang quitting.
• Failed start rolls back. A Starting guard holds the engine while the worker threads start; if any clocked spawn fails, dropping the guard calls Engine::stop, which ends and joins the threads already running.
• Further canvases (from moq-dev/studio#42): Engine::stop already joins every canvas's encoders. A further canvas's Broadcast now rides in the Ended its line leaves, so it drops only after those encode threads, and its Drop finishes the canvas's catalog before closing the publication. Before, the catalog was dropped unfinished as the line ended, cutting it at the relay; this applies to removing a canvas too.
• Media sources (from moq-dev/studio#44): Sources::stop covers them as is; their picture (Encoding) and sound (Audio) tracks finish in their Drop as the driver returns.
• README: a bullet on what quitting while live does.

Tests

• engine::tests::closing_delivers_every_tail (default suite, paused tokio clock, local moq_relay::test_relay): two tracks each get a head group read through the relay, then a 500 KB last group and a finish; Output::close runs while readers at the relay read. Each reader gets its whole last group and then a clean end, and the relay's announcements end studio. Mutation-checked: dropping the output instead of closing it cuts the tail with Session(Cancel). Stress-run 30 times on the paused clock with no failure.
• publish::tests::stopping_the_sources_finishes_their_tracks (moq-scene): Sources::stop finishes a color source's still track cleanly and takes it out of the local catalog.
• publish::tests::stopping_the_sources_finishes_a_media_files_tracks (moq-scene, paused clock): a looping media file on program, shown and heard, has both its picture and sound tracks read to a clean end after Sources::stop. Mutation-checked: skipping Encoding's end in its Drop fails it.
• engine::tests::the_program_of_a_further_canvas_has_its_own_broadcast (GPU, nightly render-sw) now reads the canvas's video and catalog to a clean end through the relay on removal, then undoes the removal and does the same on quit (stop then close). Mutation-checked: not finishing the catalog fails it.
• engine::tests::the_program_reaches_a_relay (GPU, nightly render-sw) now quits with stop then close and reads both the program video and the program audio to a clean end at the relay. Passes locally on macOS (Metal and VideoToolbox), as does the_program_records_without_a_relay.

Choices made without asking

• The paused-clock test covers the relay close, not the whole engine. The engine needs a GPU and runs its program, audio, and source workers on threads with their own runtimes, so it cannot run on one paused clock. The engine-level quit is covered by the GPU test above on a real clock.
• The 4 MB tail of the moq-tokio integration test is too big here. In a debug build over loopback, 8 MB does not drain within the session's one-second bound, so the test uses 500 KB per track, which is still in flight when the close starts (the mutation check proves it).
• The main broadcast's catalog track is not finished at quit. It is shared with health and the guest feeds, so finishing it would turn their later writes into errors. A viewer learns the broadcast ended from the withdrawn announcement, which the close sends first.
• Rollback stops synchronously. The guard's Drop joins the started threads inside the async start, which takes about a frame plus the encoders' tails. It only runs on a failed start.
• The guest room's own session (guests::Renewal) is not closed gracefully. It carries the guests' return feeds, not the broadcast, so it is outside this quest's goal.
• The rollback is not unit-tested: no thread spawn can be made to fail without a fault hook in production code, as moq-dev/studio#22 found.

Validation

• just ci passes locally on 47d47b7 (225 tests).
• cargo nextest run -p moq-studio --features openh264 --run-ignored only -E 'test(/^engine::tests::the_program_/)' passes on macOS (Metal, VideoToolbox): all three, including the further canvas test.

Review

Codex (quest review), gpt-6.1-sol, medium effort, against origin/main (206e91e, merge base e012e2e).

• Round 2 on 47d47b7a2a83bbdd0113f3ebaccb977c79076a1b (after merging main with moq-dev/studio#42, #43, #44): no findings.
• Round 1 on 235e970: no findings.
• Pullfrog (sources and relays closing in parallel can cut a source's tail): refuted, since no relay session carries a source's track; see the thread. A comment at the join now says so.

Follow-ups

None.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 27 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: ef5af6e5-429e-4b63-b60e-47cbc6f07762

Commits

Reviewing files that changed from the base of the PR and between 50fed75 and 74a80a6.

Files selected for processing: 7

• README.md
• quest/m0/README.md
• quest/m0/clean-quit.md
• rs/moq-scene/src/publish.rs
• rs/moq-scene/src/source.rs
• rs/moq-studio/src/engine.rs
• rs/moq-studio/src/main.rs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 235e970

No actionable findings. Joining the program/audio workers before graceful connection closure addresses the earlier relay-drain concern, and startup rollback joins workers already started. The shutdown ordering fits the existing source and transport lifecycles.

Verification: GitHub-only static review, including the pinned transport. I did not run tests, GPU/capture execution or thread-failure injection; hosted Check was queued when inspected.

[pullfrog]

Important

The relay close starts in parallel with the source-thread wait, so a source's last group can still be cut — the failure this PR sets out to remove, now only a race away.

Reviewed changes

• Sources stop at quit: Sources::stop drops every feed, so each Driver returns and its Video/Audio/Still finishes its own track; Engine::stop calls it after joining the program, audio, and encode threads.
• Relay sessions close before the runtime goes: Engine::close waits on the source threads and closes each relay Connection (drain) then its Client (QUIC close), bounded by CLOSE; main joins it with recorder.done().
• Failed starts roll back: a Starting guard holds the engine until every clocked spawn succeeds, dropping into Engine::stop on the first failure.
• Tests and docs: a paused-clock closing_delivers_every_tail, stopping_the_sources_finishes_their_tracks, the GPU quit test now reading the program audio to its end, the README quit bullet, and the completed quest deleted.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using big-pickle (free) ｜ 𝕏}

[kixelated]

Automated review by review (OpenAI)

Reviewed commit 74a80a6. No actionable findings in the substantive changes since 235e970, accounting for merged main changes. Further-canvas broadcasts remain alive until encoders finish, then finish their catalogs before closing. The source/relay parallel close remains appropriate because source tracks stay local. The lifetime handling is a reasonable direction.

Verification: GitHub-only static review, including relevant pinned transport and catalog behavior. I did not run tests, GPU/capture execution, or thread-failure injection.

[pullfrog]

✅ No new issues found.

Reviewed changes

Since the prior Pullfrog review on 235e970, the branch merged main and added 47d47b7 to finish a further canvas's catalog after its video.

• A further canvas's catalog finishes after its video: Line::end now takes the canvas's Broadcast and carries it in Ended, so catalog.finish() and publication.close() run only once the encode threads have finished, on removal and at quit alike.
• Source tracks confirmed local at the join: a comment at Engine::close records that source tracks live in the studio's own broadcast, which no relay session carries, so the concurrent source wait and relay close cannot cut a tail.
• Media source stop test: stopping_the_sources_finishes_a_media_files_tracks reads a looping media file's picture and sound to a clean end after Sources::stop.
• Further canvas GPU test extended: it now shares routed_tall/ended helpers and reads the canvas's video and then catalog to a clean end both on removal and on quit.

  ^{｜ View workflow run ｜ Using big-pickle (free) ｜ 𝕏}