diff --git a/quest/m1/README.md b/quest/m1/README.md index 87bf0ee398b..bd29b6fddde 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -77,7 +77,6 @@ above them. - [Draft-20 FETCH edge cases](/quest/m1/fetch-edge-cases.md) - an exact-final range reports End of Track and a FETCH_OK past the requested group closes the session, on main then release - [A local close never throws](/quest/m1/subscribe-close-throw.md) - closing a `@moq/net` lite subscription never raises ProtocolViolation for the end of stream it caused, on main - [Subscribe-close backport](/quest/m1/subscribe-close-backport.md) - the same fix and the `race` loser fix on `release`, so closing a subscription during setup leaves no unhandled rejection in a later 0.17 patch -- [Main live-feed check](/quest/m1/fetch-live-feed.md) - tests pin that a fetch-only or parked warm cache never supplies Largest Object on main, fixing it if it does - [Claim answer epochs](/quest/m1/claim-answer-epochs.md) - a claim answers each output by publishing it at its exact path under its own epoch, so a restarted output never reaches a returning reader as the old one's cache - [Untimed decisions](/quest/m1/untimed-decisions.md) - the maintainer decides whether moq-archive keeps refusing untimed tracks, and whether a malformed FETCH object ends its track - [FFI shape](/quest/m1/ffi-shape/README.md) - the bindings mirror Rust's layers: net at the root, then media, json, flate, audio, and video namespaces built from the handle below diff --git a/quest/m1/fetch-edge-cases.md b/quest/m1/fetch-edge-cases.md index c01dadafd50..5b4ddb01342 100644 --- a/quest/m1/fetch-edge-cases.md +++ b/quest/m1/fetch-edge-cases.md @@ -34,7 +34,3 @@ later 0.17 patch. Public API: none. Wire: behavior only, within the existing draft-20 through draft-22 FETCH form. - -## Related - -- [Main live-feed check](/quest/m1/fetch-live-feed.md) - the other follow-up from the same backport review diff --git a/quest/m1/fetch-live-feed.md b/quest/m1/fetch-live-feed.md deleted file mode 100644 index d34659878fa..00000000000 --- a/quest/m1/fetch-live-feed.md +++ /dev/null @@ -1,29 +0,0 @@ -# [S] Pin which caches count as a live feed on main - -## Goal - -On `main`, a draft-20+ FETCH takes the cache's newest object as Largest -Object only when the cache is fed live. A cache only fetches have filled, -never accepted, is not live, and neither is a parked warm cache with no copy -spliced after it. Tests pin both on `main`, and fix it there if either is -misclassified. - -## Plan - -The `release` backport ([#5402](https://github.com/moq-dev/moq/pull/5402)) -treated both as live, so a FETCH past the stale edge was refused -INVALID_RANGE or capped. It was fixed there in `track::Consumer::is_live` and -`resume::Consumer::is_live`, with `a_fetch_only_cache_is_not_live` and -`a_parked_warm_cache_is_not_live`. A review said `main`'s `Feed::Live` check -already tells them apart; nobody has verified that. - -Mirror those two tests against `main`'s live-feed check -(`track::Consumer::is_live` and its routes). If both pass, landing the tests -completes the quest. If either fails, fix the check at the source in the same -PR. - -Public API: none. Wire: behavior only, if a fix is needed. - -## Related - -- [Draft-20 FETCH edge cases](/quest/m1/fetch-edge-cases.md) - the other follow-up from the same backport review diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 53e4190e67f..9015f2f711c 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -7320,6 +7320,36 @@ mod serve_tests { } } + /// A track whose cache only ever answered a fetch has no live feed, so its newest + /// object is not Largest Object: a draft-20 FETCH past it echoes the requested end. + #[moq_net_sim::test] + async fn a_draft20_fetch_of_a_fetch_only_cache_is_not_capped() { + for version in [Version::Draft20, Version::Draft21, Version::Draft22] { + let h = serve(version); + let request = h._broadcast.reserve_track("cam").unwrap(); + let dynamic = request.dynamic(); + let fetching = request.consume().fetch_group(4, None); + let mut fetched = dynamic.requested_group().await.unwrap().accept(None).unwrap(); + for object in 0..2 { + fetched.write_frame(None, format!("4-{object}").into_bytes()).unwrap(); + } + fetched.finish().unwrap(); + fetching.await.unwrap(); + + let fetch_type = fetch_range( + version, + "cam", + Location { group: 4, object: 2 }, + Location { group: 4, object: 6 }, + ); + let buf = run_fetch(&h, fetch_type, GroupOrder::Ascending, true).await; + let (ok, objects) = fetch_answer(buf, version); + assert_eq!(ok.end_location, Location { group: 4, object: 5 }, "{version}"); + assert!(!ok.end_of_track, "{version}"); + assert_eq!(objects, Vec::new(), "{version}"); + } + } + /// The last group of a finished track ends the response at its last object, with End /// of Track set. #[moq_net_sim::test] diff --git a/rs/moq-net/src/model/resume.rs b/rs/moq-net/src/model/resume.rs index ef45e0e8d6e..dc0b54ab858 100644 --- a/rs/moq-net/src/model/resume.rs +++ b/rs/moq-net/src/model/resume.rs @@ -1149,6 +1149,30 @@ mod test { ); } + /// Retaining an old copy's cache does not keep a parked logical track live. + #[test] + fn a_parked_warm_cache_is_not_live() { + let routes = Producer::new(); + let logical = logical(&routes); + let resumed = logical.consume(); + let source = copy(); + let mut warm = source.create_group(group::Info { sequence: 2 }).unwrap(); + warm.write_frame(crate::Timestamp::from_millis(0).unwrap(), b"2-0".as_ref()) + .unwrap(); + warm.finish().unwrap(); + routes.serve(source.consume()); + assert!(resumed.is_live(), "a serving live copy is live"); + + routes.park(); + assert!(source.consume().peek_group(2).is_some(), "the old copy stays warm"); + assert!(routes.consume().serving().is_none(), "no copy is spliced after it"); + assert!(!resumed.is_live(), "a parked warm cache is not live"); + + let next = copy(); + routes.serve(next.consume()); + assert!(resumed.is_live(), "the next serving copy is live"); + } + #[test] fn a_reader_reads_the_serving_copy() { let routes = Producer::new(); diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index f6e64ccaf02..bbea59faf02 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -3130,12 +3130,14 @@ impl Consumer { } /// Whether the cache is fed live, so the newest object it holds is the track's largest. - /// A relay's copy with no upstream subscription is not, even once it learns the end. + /// Fetch-only, closed, and parked copies are not, even once they learn the end. pub(crate) fn is_live(&self) -> bool { - if let Some(serving) = self.serving() { - return serving.is_live(); - } - self.state.read().feed == Feed::Live + let state = self.state.read(); + let Some(routes) = state.routes.clone() else { + return state.published && !state.closed && state.feed == Feed::Live; + }; + drop(state); + routes.serving().is_some_and(|serving| serving.is_live()) } /// Fetching a single past group, without holding a live subscription. @@ -7408,6 +7410,43 @@ mod test { assert_eq!(group.sequence, 0); } + /// A fetched group installs metadata but never accepts the track's live feed. + #[test] + fn a_fetch_only_cache_is_not_live() { + let waiter = kio::Waiter::noop(); + let request = Request::new(Arc::new(broadcast::Info::default()), "test"); + let dynamic = request.dynamic(); + let consumer = request.consume(); + let mut pending = consumer.fetch_group(2, None); + let Poll::Ready(Ok(req)) = dynamic.poll_requested_group(&waiter) else { + panic!("fetch request should be ready"); + }; + let mut fetched = req.accept(None).unwrap(); + fetched.write_frame(None, b"2-0".as_ref()).unwrap(); + fetched.finish().unwrap(); + let Poll::Ready(Ok(mut group)) = kio::Task::poll(&mut *pending, &waiter) else { + panic!("fetched group should be cached"); + }; + assert_eq!(group.sequence, 2); + let Poll::Ready(Ok(Some(frame))) = group.poll_read_frame(&waiter) else { + panic!("fetched frame should be cached"); + }; + assert_eq!(frame.payload.as_ref(), b"2-0"); + assert!(!consumer.is_live(), "a fetch-only cache is not live"); + + let mut producer = request.accept(None); + assert!(consumer.is_live(), "accepting the track starts its live feed"); + producer.set_idle(); + assert!(!consumer.is_live(), "an idle accepted copy is not live"); + producer.set_live(None); + assert!(consumer.is_live(), "the answered copy is live again"); + + // The fetch handler holds the same producer lifetime, so both leave. + drop(dynamic); + drop(producer); + assert!(!consumer.is_live(), "a track whose producer left is not live"); + } + /// A one-shot fetch populates the shared cache but never the live arrival cursor, /// so it cannot make content available to a subscription look stale. #[moq_net_sim::test]