Repository navigation
quest: import #5407 and #5413 #5418
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5e0c212
7eefd21
52ecac0
3417238
69895fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| # [L] A panic closes only its connection | ||
|
|
||
| ## Goal | ||
|
|
||
| A panic inside one connection or session closes that connection and leaves | ||
| the process running: the relay, the CLI, and every FFI host (a library panic | ||
| reaches the host as `MOQ_ERROR_PANIC` or a binding exception). A panic in a | ||
| long-lived process task still exits the process, so nothing becomes a silent | ||
| zombie. | ||
|
|
||
| ## Plan | ||
|
|
||
| Found from [#5407](https://github.com/moq-dev/moq/issues/5407): the | ||
| workspace's `panic = "abort"` (dev and release profiles, added in #804) makes | ||
| moq-c's `catch_unwind` (`rs/moq-c/src/ffi.rs`) and uniffi's `rust_call` | ||
| dead in shipped builds, so any library panic kills the host app. | ||
|
|
||
| #804 chose abort because a rustls panic in a detached HTTPS task left the | ||
| relay up but serving nothing. That bug is gone (the provider is installed at | ||
| startup and the HTTPS config is built before binding), but the failure mode | ||
| returns for any detached long-lived task under unwind. | ||
|
|
||
| Decided 2026-10-13 (maintainer): | ||
|
|
||
| - Unwind workspace-wide: delete `panic = "abort"` from both profiles. Rejected: | ||
| a separate `ffi` profile for binding builds, since relay connections need | ||
| isolation too. wasm needs no override (stable std aborts there anyway). | ||
| Measure the moq-ffi staticlib against the Go-mirror size ceiling noted in | ||
| `Cargo.toml`. | ||
| - Ignore poison. kio's `Lock::lock` (`rs/kio/src/lock.rs`) panics on a | ||
| poisoned mutex, so one panic under a shared origin/broadcast/track lock | ||
| would cascade to every session, or double-panic into an abort in | ||
| `Producer::drop`. Use `unwrap_or_else(PoisonError::into_inner)` there | ||
| (loom-compatible) and at the cross-session std locks (moq-net cache, stats, | ||
| resume, frame, time; the relay; moq-c `STATE`; moq-ffi polls). Per-session | ||
| locks may keep panicking. Rejected: poison closing the channel (more kio | ||
| code), and parking_lot (no loom). | ||
| - Supervise every task. Every spawned task's `JoinHandle` is observed: | ||
| - a per-connection task's panic is logged (`tracing::error!` with the | ||
| connection span, a panic counter on `/metrics`) and ends only that | ||
| connection; dropping its driver already sends CONNECTION_CLOSE and | ||
| retracts its broadcasts; | ||
| - a long-lived task's panic propagates out of `Relay::run` (or the CLI's | ||
| main) and exits the process: the origin driver | ||
| (`moq_tokio::origin::spawn_config`), cache governor, HTTPS cert reload, | ||
| and the noq endpoint driver; | ||
| - a cluster dial task is per peer, so it counts as a connection, and its | ||
| peer is re-dialed instead of swallowed by `join_next`. | ||
| Rejected: a panic hook that aborts unless a task-local "isolated" flag is | ||
| set (catches tasks nobody listed, but hides policy in a hook), and | ||
| log-only (reintroduces #804). | ||
| - moq-c: await each session task's inner handle and report `MOQ_ERROR_PANIC` | ||
| through its terminal callback, so the entry doesn't leak. | ||
| - Regression test: a fake transport whose poll panics. Only that session | ||
| closes, its broadcasts leave the origin, and a second session keeps | ||
| working. | ||
|
Comment on lines
+54
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The only planned regression test exercises the per-session isolation branch. It would still pass if a cache governor, certificate reload, origin driver, or endpoint-driver panic were merely logged or its AGENTS.md reference: AGENTS.md:L18-L18 Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, fixed in the latest commit. (Written by Claude Opus 5.5) |
||
| - Regression test for the fatal side: inject a deterministic panic into one | ||
| named long-lived task (such as the cache governor) and assert | ||
| `Relay::run` returns with the panic instead of logging and continuing. | ||
| - Update comments and docs that assume abort inline, including | ||
| `doc/lib/cpp/index.md`, and document the panic counter's name and meaning | ||
| in `doc/bin/relay/http.md`. | ||
|
|
||
| Public API: none in Rust. Operational: a new relay `/metrics` panic counter. | ||
| Wire: none. | ||
|
|
||
| ## Related | ||
|
|
||
| - [Unset host clock](/quest/m0/wall-clock-omit.md) - removes the panic that surfaced this | ||
| - [Fuzz builds under LTO](/quest/m0/fuzz-lto.md) - also edits the release profile | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # [S] A quiet track's latest group survives rejoin on the 0.17 release line | ||
|
|
||
| ## Goal | ||
|
|
||
| On a 0.17.x moq-relay, a late subscriber to a live but quiet track (such as | ||
| a catalog published once) receives its latest group after earlier readers | ||
| left and rejoined and the cache expiry passed, including across a publisher | ||
| takeover. | ||
|
|
||
| ## Plan | ||
|
|
||
| Reported in [#5413](https://github.com/moq-dev/moq/issues/5413) against | ||
| moq-relay 0.17.3 / moq-net 0.3.11. Verified 2026-10-13: the reporter's | ||
| `rejoined_quiet_catalog_survives_cache_expiry` test fails on the | ||
| `moq-net-v0.3.11` tag and its port passes on `main`, where #4741 deleted the | ||
| warm-copy code (`WarmGroup`, `WarmCopy`, `TrackIo`) and readers read the | ||
| serving route's copy directly. | ||
|
|
||
| Root cause on `release` (`rs/moq-net/src/model/origin.rs`): the cache exempts | ||
| only a live track's latest group from expiry. On re-splice, dropping the | ||
| `WarmCopy` finishes its track, so its latest group is demoted while | ||
| `io.head` still holds it as the live edge. The new upstream copy resumes | ||
| after it, so nothing re-sends it, and the pool expires it. `TrackIo::retire` | ||
| returns early on a finished held group, so takeover loses it the same way. | ||
|
|
||
| Decided 2026-10-13 (maintainer): | ||
|
|
||
| - Backport to `release` only; `main` gets [a regression test](/quest/m1/quiet-edge-test.md). | ||
| - Keep the `WarmCopy` alive and unfinished until the new copy delivers a | ||
| group newer than its edge, so the one existing rule (a live track's | ||
| latest is protected) covers it. Give `retire` the same treatment for a | ||
| finished held group. Preferred over the reporter's patch (`WarmGroup` | ||
| holding a `cache::Protection` released by `Step::Held`), which adds a | ||
| second protection path to keep in sync. Fall back to it only if keeping | ||
| the copy alive proves invasive. | ||
| - Land the reporter's test (Lite05/06, superseded group still expires) and a | ||
| takeover variant. | ||
| - Ships with the next 0.17 patch alongside [draft-20+ FETCH](/quest/m0/release-22/fetch.md). | ||
|
|
||
| ## Closes | ||
|
|
||
| - [#5413](https://github.com/moq-dev/moq/issues/5413) - quiet catalog lost after reader rejoin and cache expiry |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| # [XS] An unset host clock publishes on the 0.17 release line | ||
|
|
||
| ## Goal | ||
|
|
||
| The next 0.17 patch (libmoq 0.6.x, moq-ffi 0.4.x, `@moq/publish`) carries | ||
| the unset-clock fix, so a publisher on a host whose wall clock reads before | ||
| 2020 publishes without a root `clock` instead of aborting the host. | ||
|
|
||
| ## Plan | ||
|
|
||
| Decided 2026-10-13 (maintainer): a crash qualifies for backport | ||
| (CONTRIBUTING). Port the `main` fix onto `release` as its own PR, including | ||
| its tests and the `@moq/publish` mirror. No wire change, so the release line | ||
| can take it in a patch. | ||
|
|
||
| Decided 2026-10-13 (maintainer, from review on #5418): keep the public Rust | ||
| API. `main` makes `Clock::wall()` return `Option`, which a patch can't | ||
| (release is moq-mux 0.10.x). On `release`, store the mapping as a private | ||
| `Option`, which `Producer::new` uses internally, and port `main`'s | ||
| whole-clock comparison in `Listing::stamp` with its withheld-catalog test, | ||
| since two wall-less clocks compare equal by mapping alone. Keep | ||
| `wall() -> hang::catalog::Clock`, which panics only when a caller asks an | ||
| unset host for its mapping; that host already crashed at construction | ||
| before. Rejected: a breaking moq-mux 0.11.0 cut from `release`, which takes | ||
| the number `main`'s next breaking release needs, and an additive public | ||
| accessor, the shim pattern the repo avoids. | ||
|
|
||
| ## Required | ||
|
|
||
| - [Unset host clock](/quest/m0/wall-clock-omit.md) - lands the fix on `main` first | ||
|
|
||
| ## Closes | ||
|
|
||
| - [#5407](https://github.com/moq-dev/moq/issues/5407) - moq-mux panics when the system clock is before 2020 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| # [S] An unset host clock publishes without a wall mapping | ||
|
|
||
| ## Goal | ||
|
|
||
| A `moq_mux::catalog::Producer` on a host whose wall clock reads before 2020, | ||
| such as an embedded or Android device booted before NTP sync, publishes | ||
| normally with no root `clock` in its catalog, instead of panicking in | ||
| `Clock::new()` and aborting the host process. Rust and `@moq/publish` alike. | ||
|
|
||
| Non-goals: moving the epoch, and panic isolation, which | ||
| [its own quest](/quest/m0/panic-isolation.md) owns. | ||
|
|
||
| ## Plan | ||
|
|
||
| Reported in [#5407](https://github.com/moq-dev/moq/issues/5407) from an | ||
| Android device (libmoq 0.6.13). Verified on `main` 2026-10-13: | ||
| `Producer::new` (`rs/moq-mux/src/catalog/producer.rs`) falls back to | ||
| `Clock::default()`, whose `Clock::new()` `expect`s `arrival(LEAD)`, and | ||
| `wall_clock()` refuses a time before the 2020 epoch. Every production | ||
| publisher without `Config::with_clock` reaches it: moq-ffi (every binding), | ||
| moq-c, moq-cli, moq-gst, moq-rtmp, moq-srt, moq-rtc, moq-hls import, | ||
| moq-transcode. `js/publish/src/catalog.ts` `pageClock()` throws the same way | ||
| through `u53`. | ||
|
|
||
| Decided 2026-10-13 (maintainer): | ||
|
|
||
| - Keep the 2020 epoch. The root `clock` is optional on the wire (hang draft, | ||
| Rust `Option`, zod `optional`), and only feeds HLS `PROGRAM-DATE-TIME` and | ||
| DASH `availabilityStartTime`. A reading before 2020 reliably means the | ||
| clock is unset. No clock is better for consumers than a false one: DASH | ||
| falls back to the relay's anchor, while a 1970 AST is unplayable. | ||
| Rejected: moving to a 1970 epoch under a renamed root key (hang-04), which | ||
| buys consistency at the cost of a wire rename and a mapping consumers | ||
| can't use. Rejected: failing `Producer::new`, which stops an offline | ||
| device from ever publishing, though media delivery needs no wall. | ||
| - `moq_mux::Clock`'s wall mapping becomes optional and `Clock::wall()` | ||
| returns `Option`. `Clock::new()` stays infallible: the monotonic part | ||
| needs no wall. `Producer::new` keeps its signature and sets | ||
| `catalog.clock = clock.wall()`. | ||
|
Comment on lines
+36
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a data producer exists while an initial reservation withholds the catalog, the first importer can replace the unanchored clock. Useful? React with 👍 / 👎.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. The quest now compares whole clocks (monotonic anchor, reading, and wall) in (Written by Claude Opus 5.5) |
||
| - Fail loud where a wall is actually needed: `reading(wall)`, and an | ||
| importer placing a source wall on a wall-less clock, return an error. | ||
| - Clock identity can't be the wall mapping. `Listing::stamp` | ||
| (`rs/moq-mux/src/catalog/data.rs`) detects an importer re-anchoring the | ||
| clock by comparing `wall()`, which is `None` for both clocks on an unset | ||
| host, so it would measure jitter, delay, and bitrate across unrelated | ||
| timelines. Compare whole clocks (monotonic anchor, reading, and wall). | ||
| Cover a withheld catalog plus a data producer and an importer in the test. | ||
| Found in review on #5418. | ||
| - Mirror in `pageClock()`: omit `clock` instead of throwing. | ||
| - Interplay with [catalog clock adopt](/quest/m1/catalog-clock-adopt.md), | ||
| which assumes `wall()` is always present: whichever lands second | ||
| reconciles. An absent wall matches an absent catalog clock. | ||
| - Test through a private `now` seam (until [Controlled time](/quest/m1/time/README.md) | ||
| makes the wall mockable): a pre-2020 host yields a clock-less catalog and | ||
| media still publishes. | ||
| - Fix the `Clock::TIMESCALE` doc's "until the year 2255", the 1970 figure | ||
| (2020 lasts until 2305). | ||
| - Then backport ([release line](/quest/m0/release-22/wall-clock-omit.md)). | ||
|
|
||
| Public API: `moq_mux::Clock::wall()` returns `Option`. Wire: none. | ||
|
|
||
| ## Related | ||
|
|
||
| - [Release backport](/quest/m0/release-22/wall-clock-omit.md) - cherry-picks this onto the 0.17 line | ||
| - [Panic isolation](/quest/m0/panic-isolation.md) - a library panic no longer aborts the host | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # [XS] Pin a quiet track's latest group across rejoin and takeover | ||
|
|
||
| ## Goal | ||
|
|
||
| `rs/moq-net/tests/rejoin.rs` proves that a late subscriber to a live, quiet | ||
| track receives its latest group after readers left and rejoined and the cache | ||
| expiry passed, and after a publisher takeover, on every version, while a | ||
| superseded group still expires. | ||
|
|
||
| ## Plan | ||
|
|
||
| `main` already behaves (verified 2026-10-13), but the 0.17 line broke it | ||
| ([#5413](https://github.com/moq-dev/moq/issues/5413)); the test keeps a | ||
| future origin rework from regressing. Decided 2026-10-13 (maintainer): land | ||
| the port of the reporter's test plus a takeover variant, on lite-05, 06 and | ||
| 07 and transport-19 and 22, using `moq_net_sim::test` time. Mention #5413 | ||
| without a closing keyword; the [release backport](/quest/m0/release-22/quiet-edge.md) | ||
| closes it. | ||
|
|
||
| ## Related | ||
|
|
||
| - [Release backport](/quest/m0/release-22/quiet-edge.md) - the 0.17 fix this pins on `main` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The planned panic counter changes the relay's externally scraped
/metricsbehavior, but the documentation work is limited to material that assumes abort and the impact section still says there is no public API change. Add the metric's name and semantics todoc/bin/relay/http.mdand record its operational API impact so deployers can discover and alert on the new counter.AGENTS.md reference: AGENTS.md:L107-L107
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, fixed in the latest commit.
(Written by Claude Opus 5.5)