diff --git a/js/net/bench/resume.ts b/js/net/bench/resume.ts index fdacf811a9f..59b93166500 100644 --- a/js/net/bench/resume.ts +++ b/js/net/bench/resume.ts @@ -137,6 +137,57 @@ for (const swaps of swapCounts) { } if (checksum === 0) throw new Error("benchmark did no work"); +// Two routes keep the same live group while taking turns. Sweep audience and flap +// count: the cost per swap must not grow with redundant subscriptions to those routes. +async function measureFlaps(readers: number, flaps: number): Promise { + const front = new Front(); + const sources = [new BroadcastProducer(), new BroadcastProducer()]; + const handles = sources.map((source) => source.consume()); + const tracks = sources.map((source) => source.createTrack("seq")); + const groups = tracks.map((track) => { + const group = track.appendGroup(); + group.writeFrame({ payload }); + return group; + }); + front.serve(handles[0]); + const subscribers = Array.from({ length: readers }, () => + front.consumer.track("seq").subscribe({ maxDelay: Milli(3_600_000) }), + ); + const held = subscribers.map((subscriber) => { + const group = subscriber.tryRecvGroup(); + if (!group) throw new Error("missing held group"); + return group; + }); + await flush(); + const start = performance.now(); + for (let flap = 0; flap < flaps; flap++) { + front.serve(handles[(flap + 1) % 2]); + await flush(); + for (const subscriber of subscribers) { + if (subscriber.tryRecvGroup()) throw new Error("repeated held group"); + } + } + const elapsed = performance.now() - start; + for (const group of held) group.close(); + for (const subscriber of subscribers) subscriber.close(); + front.consumer.close(); + for (const group of groups) group.close(); + for (const handle of handles) handle.close(); + for (const source of sources) source.close(); + await flush(); + return (elapsed * 1000) / readers / flaps; +} + +await measureFlaps(4, 16); +console.log("readers,flaps,flap_us"); +for (const readers of readerCounts) { + for (const flaps of [1, 16, 64]) { + const samples: number[] = []; + for (let rep = 0; rep < reps; rep++) samples.push(await measureFlaps(readers, flaps)); + console.log(`${readers},${flaps},${Math.min(...samples).toFixed(2)}`); + } +} + // An adopted copy may repeat a large prefix before it reaches unread content. Time readiness // separately from constructing the copies, sweeping both the replay and independent readers. async function measureReplay(readers: number, replayed: number): Promise { diff --git a/js/net/src/resume.test.ts b/js/net/src/resume.test.ts index c366ce31937..90461ec4f87 100644 --- a/js/net/src/resume.test.ts +++ b/js/net/src/resume.test.ts @@ -1,4 +1,4 @@ -import { expect, test } from "bun:test"; +import { expect, spyOn, test } from "bun:test"; import { Manual, Milli, Nano } from "@moq/clock"; import { Consumer as BroadcastConsumer, Producer as BroadcastProducer } from "./broadcast.ts"; import * as Epoch from "./epoch.ts"; @@ -104,6 +104,87 @@ async function setup() { }; } +test("returning to a route reuses its subscription and held group", async () => { + const clock = new Manual(); + const restore = clock.install(); + const front = new Front(); + const sources = [new BroadcastProducer(), new BroadcastProducer()]; + const handles = sources.map((source) => source.consume()); + const tracks = sources.map(copy); + const open = tracks[0].write(0, 1, false); + const mirrored = spyOn(open, "mirror"); + front.serve(handles[0]); + const sub = front.consumer.track("seq").subscribe({ maxDelay: Milli(3_600_000) }); + try { + await clock.advance(Nano.zero); + const held = sub.tryRecvGroup(); + if (!held) throw new Error("expected the held group"); + expect(dec.decode((await held.readFrame())?.payload)).toBe("0.0"); + for (let round = 0; round < 32; round++) { + front.serve(handles[round % 2]); + await clock.advance(Nano.zero); + expect(sub.tryRecvGroup()).toBeUndefined(); + expect(mirrored.mock.calls.length).toBe(1); + } + tracks[1].write(1); + expect(drain(sub)).toEqual([1]); + front.serve(handles[0]); + await clock.advance(Nano.zero); + open.writeFrame({ payload: enc.encode("0.1") }); + expect(dec.decode((await held.readFrame())?.payload)).toBe("0.1"); + tracks[0].write(2); + expect(drain(sub)).toEqual([2]); + held.close(); + } finally { + mirrored.mockRestore(); + open.close(); + sub.close(); + front.consumer.close(); + for (const handle of handles) handle.close(); + for (const source of sources) source.close(); + restore(); + } +}); + +test("a returning route resumes where a fresh copy would", async () => { + const clock = new Manual(); + const restore = clock.install(); + const front = new Front(); + const sources = [new BroadcastProducer(), new BroadcastProducer()]; + const handles = sources.map((source) => source.consume()); + const [ta, tb] = sources.map(copy); + const open = ta.write(0, 1, false); + const mirrored = spyOn(open, "mirror"); + front.serve(handles[0]); + const sub = front.consumer.track("seq").subscribe({ maxDelay: Milli(3_600_000) }); + try { + await clock.advance(Nano.zero); + const held = sub.tryRecvGroup(); + if (!held) throw new Error("expected the held group"); + front.serve(handles[1]); + await clock.advance(Nano.zero); + tb.write(10); + tb.write(20); + expect(drain(sub)).toEqual([10, 20]); + ta.write(15); + front.serve(handles[0]); + await clock.advance(Nano.zero); + expect(mirrored.mock.calls.length).toBe(1); + expect(drain(sub)).toEqual([]); + ta.write(26); + expect(drain(sub)).toEqual([26]); + held.close(); + } finally { + mirrored.mockRestore(); + open.close(); + sub.close(); + front.consumer.close(); + for (const handle of handles) handle.close(); + for (const source of sources) source.close(); + restore(); + } +}); + test("a routed source start follows the serving copy and its subscriber floor", async () => { const a = new BroadcastProducer(); const b = new BroadcastProducer(); diff --git a/js/net/src/resume.ts b/js/net/src/resume.ts index 01e67a49ca0..934dba616b4 100644 --- a/js/net/src/resume.ts +++ b/js/net/src/resume.ts @@ -347,6 +347,29 @@ class Reader implements ResumeSource { // The replaced copies keep delivering until the new one delivers its first group: a // group they get meanwhile may be one the new route never has. See #observe. for (const copy of this.#copies) copy.replaced = true; + // Cloned handles share closed, so a returning route reuses its subscription and + // leases rather than replaying its cache and retaining another copy per flap. + const index = this.#copies.findIndex((copy) => copy.source.from.closed === source.from.closed); + if (index !== -1) { + const [copy] = this.#copies.splice(index, 1); + source.retain(); + copy.source.release(); + copy.source = source; + copy.generation = route.generation; + copy.replaced = false; + copy.until = undefined; + copy.since = undefined; + copy.done = undefined; + copy.datagramsDone = undefined; + // Resume where a fresh copy would: a group it buffered while replaced, below the newest + // another route handed out, is not handed out. + if (this.#newest !== undefined) copy.floor = Math.max(copy.floor ?? 0, this.#newest); + this.#update(copy); + this.#bound(copy); + this.#copies.push(copy); + this.wake(); + return; + } const floor = this.#newest; this.#add({ generation: route.generation, diff --git a/quest/m1/README.md b/quest/m1/README.md index 9388d63fe5b..7cae7f52e1a 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -128,7 +128,6 @@ tracks, so per-group stats stop costing cluster announces. - [Stream sockets disable Nagle](/quest/m1/tcp-nodelay.md) - every qmux TCP, TLS, and WebSocket socket, accepted or dialed, sets `TCP_NODELAY` - [TS fidelity and recovery](/quest/m1/ts-reliability/README.md) - both modes have explicit profiles, automated validation, loss tests, and a dedicated guide - [Replica resume](/quest/m1/replica-resume.md) - a lagging same-epoch replica resumes with every frame exactly once and in order, on lite-07 and on moq-transport -- [Rust resume holds a declared end](/quest/m1/resume-rust-end.md) - a resumed Rust track keeps a declared end across copies, dedupes flapping copies, and judges retention inside the reader's window - [Rust resume checks compression](/quest/m1/resume-compress.md) - a same-epoch route copy with a different `compress` is refused instead of corrupting framing - [Rust resume backs single-route groups](/quest/m1/resume-handed-out.md) - a group first read from one route keeps the later copy that holds it, with a passthrough benchmark - [WebGPU on Safari](/quest/m1/webgpu-safari.md) - the WebGPU renderer is verified on Safari 26 for macOS and iOS diff --git a/quest/m1/resume-compress.md b/quest/m1/resume-compress.md index 9c8de034716..b3f1d8bd783 100644 --- a/quest/m1/resume-compress.md +++ b/quest/m1/resume-compress.md @@ -25,7 +25,3 @@ not `compress`. JS's `compatible()` (`js/net/src/resume.ts`) compares all four. B's groups decoded under A's metadata. Matching compression keeps resuming. Public API: behavior only. Wire: none. - -## Related - -- [Rust resume holds a declared end](/quest/m1/resume-rust-end.md) - the other Rust copy-compatibility check diff --git a/quest/m1/resume-handed-out.md b/quest/m1/resume-handed-out.md index 48ee6b87c07..449fe5269b0 100644 --- a/quest/m1/resume-handed-out.md +++ b/quest/m1/resume-handed-out.md @@ -26,11 +26,4 @@ read group 1; A's partial group 0 fails. B was dropped, so group 0 stalls. single copy, so the per-group record's cost shows up. - Regression test on a mocked clock for the reproduction above. -Lands after [#6224](https://github.com/moq-dev/moq/pull/6224), which edits the -same reader, so it does not widen a PR under review. - Public API: behavior only. Wire: none. - -## Required - -- [Rust resume holds a declared end](/quest/m1/resume-rust-end.md) - lands first; it edits the same reader diff --git a/quest/m1/resume-rust-end.md b/quest/m1/resume-rust-end.md deleted file mode 100644 index ad25800dc82..00000000000 --- a/quest/m1/resume-rust-end.md +++ /dev/null @@ -1,35 +0,0 @@ -# [S] Rust resume holds a declared end across copies - -## Goal - -A resumed Rust track (`resume::Reader` in `rs/moq-net/src/model/resume.rs`) -keeps a declared end once any copy reported it, as JS does with -[moq-dev/moq#6136](https://github.com/moq-dev/moq/pull/6136). A route that -flaps holds no more replaced copies than the distinct routes it needs, and a -replaced copy is kept only for groups inside the reader's window. - -## Plan - -Decided 2026-10-21 with the maintainer: one quest for three Rust resume gaps. A fourth, the -narrower Rust `route_failed`, moved to the recovery verdicts work. - -- **Declared end.** `poll_finished` reads only the serving copy, so an end - declared on a replaced copy is lost after a swap. Answer from the first - declared end, and refuse a copy declaring a different one, matching JS. -- **Flapping routes.** Swapping between the same two routes adds one replaced - copy per swap, redundant as backups - ([moq-dev/moq#6141](https://github.com/moq-dev/moq/pull/6141)). Dedupe them - per swap. -- **Window.** The "may still land" check ignores the reader's `Window`, so a - sparse-range reader keeps a replaced copy up to its budget for a gap outside - its ranges (#6141). Judge it inside the window. - -Regression test per gap, on a mocked clock, mirrored in JS where JS lacks one. - -Decided 2026-10-22 (maintainer), settled in -[#6224](https://github.com/moq-dev/moq/pull/6224)'s review: Rust refuses a copy -declaring a different end with the same non-route-failure classification as -JS's `Incompatible` (`js/net/src/resume.ts`). Merge order: #6224 next -(#6136, #6209 and #6251 have landed). - -Public API: behavior only. Wire: none. diff --git a/rs/moq-net/benches/origin.rs b/rs/moq-net/benches/origin.rs index e27d5f695a0..3e2d47aa63a 100644 --- a/rs/moq-net/benches/origin.rs +++ b/rs/moq-net/benches/origin.rs @@ -1310,7 +1310,7 @@ fn bench_copy_walk(c: &mut Criterion) { /// `(readers, retained, routes)` for [`bench_resume_overlap`]: readers of one track, /// the groups every route holds, and the routes it swapped through. -const RESUME_OVERLAP: [(usize, usize, usize); 15] = [ +const RESUME_OVERLAP: [(usize, usize, usize); 17] = [ (1, 1, 1), (1, 64, 1), (32, 64, 1), @@ -1326,6 +1326,8 @@ const RESUME_OVERLAP: [(usize, usize, usize); 15] = [ (1, 64, 8), (1, 64, 32), (32, 64, 8), + (32, 64, 32), + (256, 64, 8), ]; /// A track that swapped through its routes while its readers' local cap held every diff --git a/rs/moq-net/src/model/group.rs b/rs/moq-net/src/model/group.rs index a35f014980f..73d20ffc7b5 100644 --- a/rs/moq-net/src/model/group.rs +++ b/rs/moq-net/src/model/group.rs @@ -2761,7 +2761,7 @@ impl Consumer { // A front's reader may continue the group from another route's copy when it reads. if self .recover - .as_ref() + .as_mut() .is_some_and(|recover| recover.wants(Some(&aborted), waiter)) { return None; diff --git a/rs/moq-net/src/model/resume.rs b/rs/moq-net/src/model/resume.rs index 04b5086e3f4..1863f31dbd5 100644 --- a/rs/moq-net/src/model/resume.rs +++ b/rs/moq-net/src/model/resume.rs @@ -242,6 +242,8 @@ impl Consumer { groups_read: false, out: Vec::new(), peeked: None, + declared: None, + refused: false, }) }); // Subscribed to the serving copy at once, so groups it has now are the reader's @@ -465,10 +467,15 @@ impl Copy { let from = floor.map_or(start, |floor| floor.max(start)); // A group still to land is waited for once something superseded it, as a gap is. if self.since.is_some() - && self - .track - .may_land(from..end, floor.is_some(), grace, Some(reader.delivered)) - { + && reader + .window + .runs(from) + .take_while(|run| *run.start() < end) + .any(|run| { + let end = run.end().saturating_add(1).min(end); + self.track + .may_land(*run.start()..end, floor.is_some(), grace, Some(reader.delivered)) + }) { return Retains::Wait; } // Inspect without moving either cursor: a later group poll still chooses its @@ -506,6 +513,7 @@ impl Copy { /// What [`Copy::retains`] judges a replaced copy by: its reader, as it lets copies go. struct Retain<'a> { groups: (u64, Cap), + window: &'a Window, groups_read: bool, /// The newest group the serving copy has. edge: Option, @@ -731,6 +739,8 @@ struct Datagrams { latest: Option<(u64, u64)>, seen: BTreeSet<(u64, u64)>, superseded: VecDeque<(Instant, (u64, u64))>, + /// The highest identity ever handed out, which a declared end must stay past. + newest: Option<(u64, u64)>, } impl Datagrams { @@ -748,6 +758,7 @@ impl Datagrams { if self.latest == Some(id) || self.seen.contains(&id) { return false; } + self.newest = self.newest.max(Some(id)); if let Some(previous) = self.latest.replace(id) { if self.superseded.len() == super::track::MAX_DATAGRAMS && let Some((_, oldest)) = self.superseded.pop_front() @@ -764,6 +775,10 @@ impl Datagrams { /// A [`Subscriber`]'s state. struct Reader { peeked: Option, + /// The first exclusive end any copy declared, refined by a matching mid-group end. + declared: Option, + /// Copies of one logical track contradicted its end. + refused: bool, this: Weak>, route: Consumer, /// The reader's own preferences, mirrored into every copy's subscription. @@ -777,7 +792,7 @@ struct Reader { newest: Option, /// The reader's ranges resolved; see [`Reader::window`]. window: Window, - /// Identities handed out within the reader's budget, by Group and Object ID. + /// Identities handed out within the reader's budget, by Group and Object ID, and the highest ever. datagrams: Datagrams, /// The local group filter; see [`track::Subscriber::set_groups`]. groups: (u64, Cap), @@ -845,6 +860,11 @@ impl Reader { /// Follow the route: subscribe to a new copy, mark the ones it replaced, and mirror /// the reader's preferences into each. fn sync(&mut self, waiter: &kio::Waiter) { + // Learn before a swap or retention drops the only copy that knows the end. + self.learn_end(None); + if self.refused { + return; + } // Snapshot and re-arm together, including when a change was observed: another // update must wake a reader that parks after applying this one. let mut changed = None; @@ -912,6 +932,33 @@ impl Reader { for copy in &mut self.copies { copy.replaced = true; } + // Returning to a route reuses its subscription and leases, including an older + // group's in-flight tail. A fresh copy would replay it and retain one per flap. + if let Some(index) = self.copies.iter().position(|copy| copy.track.same_channel(&track)) { + let mut copy = self.copies.remove(index); + if let Some(candidate) = self + .peeked + .as_mut() + .filter(|candidate| candidate.generation == copy.generation) + { + candidate.generation = generation; + } + copy.generation = generation; + copy.replaced = false; + copy.until = None; + copy.since = None; + copy.done = None; + copy.datagrams_done = None; + // Resume where a fresh copy would: a group it buffered while replaced, below the + // newest another route handed out, is not handed out. + copy.floor = copy.floor.max(self.newest.map(Position::group)); + if let (Sub::Ready(sub), Some(floor)) = (&mut copy.sub, copy.floor) { + sub.raise_start_to(floor.group); + } + copy.update(&self.mirrored); + self.copies.push(copy); + return; + } let floor = self.newest.map(Position::group); // The wire request carries the floor from the first message: some sessions read // the demand only once. @@ -930,14 +977,92 @@ impl Reader { since: None, lease: Arc::new(()), }); + self.learn_end(None); + } + + /// One logical track has one end, whichever route first reports it. + fn learn_end(&mut self, waiter: Option<&kio::Waiter>) { + for copy in &mut self.copies { + let end = match waiter { + Some(waiter) => match copy.poll_ready(self.groups, waiter) { + Poll::Ready(Ok(sub)) => match sub.poll_finished(waiter) { + Poll::Ready(Ok(end)) => Some(end), + _ => None, + }, + _ => None, + }, + None => match &mut copy.sub { + Sub::Ready(sub) => match sub.poll_finished(&kio::Waiter::noop()) { + Poll::Ready(Ok(end)) => Some(end), + _ => None, + }, + Sub::Pending(_) => copy.track.declared_end(), + }, + }; + let Some(end) = end else { continue }; + match self.declared { + None => self.declared = Some(end), + Some(declared) if declared == end => {} + // FETCH can name only the group boundary just after a mid-group end. + Some(declared) + if declared.final_sequence() == end.final_sequence() && (declared.frame == 0 || end.frame == 0) => + { + if declared.frame == 0 { + self.declared = Some(end); + } + } + Some(_) => self.refused = true, + } + } + // A group, frame, or datagram at or past it, handed out or on any copy, claims + // other content. + if let Some(declared) = self.declared { + let end = declared.final_sequence(); + let seen = |sequence: Option| sequence.zip(end).is_some_and(|(sequence, end)| sequence >= end); + let cut = |copy: &Copy| { + declared.frame != 0 + && copy + .track + .peek_group(declared.group) + .is_some_and(|group| group.frame_count() as u64 > declared.frame) + }; + if seen(self.newest) + || self + .datagrams + .newest + .is_some_and(|(group, frame)| Position { group, frame } >= declared) + || self + .copies + .iter() + .any(|copy| seen(copy.track.latest_sequence()) || cut(copy)) + { + self.refused = true; + } + } + if self.refused { + self.peeked = None; + } + } + + /// Whether `at` is past the declared end, which refuses the track: another copy + /// contradicts the copy that declared it. + fn past_end(&mut self, at: Position) -> bool { + if self.declared.is_some_and(|end| at >= end) { + self.refused = true; + self.peeked = None; + } + self.refused } /// Drop the replaced copies nothing needs any more; see [`Copy::retains`]. Whether one /// may still deliver a group the reader lacks, which a clean end waits for. fn let_go(&mut self, delivered: &Delivered, waiter: &kio::Waiter) -> bool { + self.learn_end(None); + self.window(); self.out.retain(|(_, out)| out.strong_count() > 0); let reader = Retain { groups: self.groups, + window: &self.window, groups_read: self.groups_read, edge: self.copies.last().and_then(|copy| copy.track.latest_sequence()), mirrored: &self.mirrored, @@ -972,7 +1097,11 @@ impl Reader { if !matches!(serving.done, Some(Ok(()))) { return false; } - let until = match serving.track.final_sequence() { + let until = match self + .declared + .and_then(Position::final_sequence) + .or_else(|| serving.track.final_sequence()) + { Some(end) => end.checked_sub(1), None => self.newest.max(serving.track.latest_sequence()), }; @@ -1076,6 +1205,9 @@ impl Reader { Poll::Pending => return Some(Poll::Pending), }, }; + if self.past_end(Position::group(candidate.sequence)) { + return Some(Poll::Ready(Err(Error::ProtocolViolation))); + } let index = self .copies .iter() @@ -1090,11 +1222,18 @@ impl Reader { }; // Selection reserves the exact group, so eviction can abort that group but // cannot substitute a later one and advance the logical reader past a gap. - let Poll::Ready(Ok(Some(group))) = result else { + let Poll::Ready(Ok(Some(mut group))) = result else { unreachable!("the peek reserved a group") }; assert_eq!(group.sequence, candidate.sequence, "the peek reserved this exact group"); self.observe(index, group.sequence); + // A mid-group end caps the group it falls in, as it caps a direct track's. + if let Some(end) = self + .declared + .filter(|end| end.frame != 0 && end.group == group.sequence) + { + group.end_at(..end.frame); + } let group = self.hand_out(index, group); Some(Poll::Ready(Ok(Some(group)))) } @@ -1109,6 +1248,9 @@ impl Reader { waiter: &kio::Waiter, ) -> Option>>> { self.sync(waiter); + if self.refused { + return Some(Poll::Ready(Err(Error::ProtocolViolation))); + } self.groups_read = true; let groups = self.groups; let budget = self.mirrored.max_delay; @@ -1254,6 +1396,9 @@ impl Reader { waiter: &kio::Waiter, ) -> Option>>> { self.sync(waiter); + if self.refused { + return Some(Poll::Ready(Err(Error::ProtocolViolation))); + } self.let_go(delivered, waiter); let at = Instant::now(); self.datagrams.expire(at, self.mirrored.max_delay); @@ -1283,6 +1428,12 @@ impl Reader { if !self.datagrams.insert(id, at) { continue; } + if self.past_end(Position { + group: id.0, + frame: id.1, + }) { + return Some(Poll::Ready(Err(Error::ProtocolViolation))); + } return Some(Poll::Ready(Ok(Some(datagram)))); } Poll::Ready(Ok(None)) => { @@ -1305,6 +1456,13 @@ impl Reader { fn poll_finished(&mut self, waiter: &kio::Waiter) -> Poll> { loop { self.sync(waiter); + self.learn_end(Some(waiter)); + if self.refused { + return Poll::Ready(Err(Error::ProtocolViolation)); + } + if let Some(end) = self.declared { + return Poll::Ready(Ok(end)); + } let groups = self.groups; let concluded = self.route.end(); if let Some(Some(Err(err))) = concluded { @@ -1455,12 +1613,24 @@ impl Clone for Recover { impl Recover { /// Whether to look past the copy being read: it stalled or failed with a verdict on the - /// group while a newer route serves, or it failed delivering the group. - pub(super) fn wants(&self, err: Option<&Error>, waiter: &kio::Waiter) -> bool { - match err { - Some(err) if self.undelivered(err) => true, - _ => self.route.poll_changed(self.generation, waiter).is_ready(), + /// group while a newer route serves, or it failed delivering the group. A route that + /// returns to serve the copy being read is no change. + pub(super) fn wants(&mut self, err: Option<&Error>, waiter: &kio::Waiter) -> bool { + if err.is_some_and(|err| self.undelivered(err)) { + return true; + } + while self.route.poll_changed(self.generation, waiter).is_ready() { + // A route that returned serves the copy being read again, which the reader reuses. + let route = self.route.state.read(); + if route.end.is_some() + || route.concluded + || !route.copy.as_ref().is_some_and(|copy| copy.same_channel(&self.copy)) + { + return true; + } + self.generation = route.generation; } + false } /// Whether the copy being read failed delivering the group rather than judging it, so @@ -1492,7 +1662,11 @@ impl Recover { // Subscribe the reader to a new route from its newest group, which is this one or // later, so that route's subscription delivers the rest of it. if let Some(reader) = self.reader.as_ref().and_then(Weak::upgrade) { - reader.lock().unwrap_or_else(PoisonError::into_inner).sync(waiter); + let mut reader = reader.lock().unwrap_or_else(PoisonError::into_inner); + reader.sync(waiter); + if reader.refused { + return Poll::Ready(Err(Error::ProtocolViolation)); + } } loop { let (generation, serving, end, closed) = { @@ -1906,6 +2080,301 @@ mod test { ); } + #[moq_clock::test] + async fn a_declared_end_survives_a_swap_before_it_is_polled() { + let routes = Producer::new(); + let logical = logical(&routes); + let mut a = copy(); + routes.serve(a.consume()); + let mut sub = subscribe(&logical, HOUR); + let end = Position { group: 4, frame: 3 }; + a.finish_at(end).unwrap(); + let b = copy(); + routes.serve(b.consume()); + assert_eq!( + sub.poll_finished(&kio::Waiter::noop()).map(Result::unwrap), + Poll::Ready(end) + ); + // A datagram-only reader lets the replaced copy go, but keeps what it declared. + assert!(sub.poll_recv_datagram(&kio::Waiter::noop()).is_pending()); + assert_eq!(copies(&routes), 1); + assert_eq!( + sub.poll_finished(&kio::Waiter::noop()).map(Result::unwrap), + Poll::Ready(end) + ); + } + + #[moq_clock::test] + async fn copies_must_agree_on_the_declared_end() { + for next in [Position::group(5), Position { group: 4, frame: 3 }, Position::group(6)] { + let routes = Producer::new(); + let logical = logical(&routes); + let mut a = copy(); + routes.serve(a.consume()); + let mut sub = subscribe(&logical, HOUR); + a.finish_at(Position::group(5)).unwrap(); + let mut b = copy(); + b.finish_at(next).unwrap(); + routes.serve(b.consume()); + let result = sub.poll_finished(&kio::Waiter::noop()); + if next == Position::group(6) { + assert!(matches!(result, Poll::Ready(Err(Error::ProtocolViolation)))); + assert!(matches!( + sub.poll_recv_group(&kio::Waiter::noop()), + Poll::Ready(Err(Error::ProtocolViolation)) + )); + assert!(matches!( + sub.poll_recv_datagram(&kio::Waiter::noop()), + Poll::Ready(Err(Error::ProtocolViolation)) + )); + } else { + assert_eq!(result.map(Result::unwrap), Poll::Ready(next)); + } + } + } + + /// A copy that delivers past an end another copy declared contradicts it. + #[moq_clock::test] + async fn content_past_the_declared_end_refuses_the_track() { + for datagram in [false, true] { + let routes = Producer::new(); + let logical = logical(&routes); + let mut a = copy(); + routes.serve(a.consume()); + let mut sub = subscribe(&logical, HOUR); + a.finish_at(Position::group(5)).unwrap(); + let mut b = copy(); + routes.serve(b.consume()); + write(&b, 4); + assert_eq!(recv(&mut sub).sequence, 4); + let res = match datagram { + false => { + write(&b, 5); + sub.poll_recv_group(&kio::Waiter::noop()).map_ok(|_| ()) + } + true => { + b.insert_datagram(5, ts(5), b"d".as_ref()).unwrap(); + sub.poll_recv_datagram(&kio::Waiter::noop()).map_ok(|_| ()) + } + }; + assert!( + matches!(res, Poll::Ready(Err(Error::ProtocolViolation))), + "datagram={datagram} {res:?}" + ); + assert!(matches!( + sub.poll_finished(&kio::Waiter::noop()), + Poll::Ready(Err(Error::ProtocolViolation)) + )); + } + } + + /// A mid-group end another copy declared caps the group it falls in, even for frames + /// that arrive after the group was handed out. + #[moq_clock::test] + async fn a_mid_group_end_caps_another_copys_group() { + let routes = Producer::new(); + let logical = logical(&routes); + let mut a = copy(); + routes.serve(a.consume()); + let mut sub = subscribe(&logical, HOUR); + a.finish_at(Position { group: 4, frame: 2 }).unwrap(); + let b = copy(); + routes.serve(b.consume()); + let mut group = b.create_group(group::Info { sequence: 4 }).unwrap(); + group.write_frame(ts(0), b"x".as_ref()).unwrap(); + let mut reading = recv(&mut sub); + // Frames past the end arrive only once the group is handed out. + for frame in 1..4 { + group.write_frame(ts(frame), b"x".as_ref()).unwrap(); + } + group.finish().unwrap(); + assert_eq!(read(&mut reading), Some(b"x".to_vec())); + assert_eq!(read(&mut reading), Some(b"x".to_vec())); + assert_eq!(read(&mut reading), None); + // The copy holding them contradicts the end. + assert!(matches!( + sub.poll_finished(&kio::Waiter::noop()), + Poll::Ready(Err(Error::ProtocolViolation)) + )); + } + + /// An end below a group a copy handed out or holds claims other content. + #[moq_clock::test] + async fn an_end_below_a_seen_group_refuses_the_track() { + for handed_out in [false, true] { + let routes = Producer::new(); + let logical = logical(&routes); + let a = copy(); + routes.serve(a.consume()); + let mut sub = subscribe(&logical, HOUR); + write(&a, 5); + if handed_out { + assert_eq!(drain(&mut sub, false), [5]); + } + let mut b = copy(); + routes.serve(b.consume()); + b.finish_at(Position::group(3)).unwrap(); + assert!(matches!( + sub.poll_finished(&kio::Waiter::noop()), + Poll::Ready(Err(Error::ProtocolViolation)) + )); + } + } + + /// A mid-group end learned late, below a frame or datagram object already seen, claims + /// other content. + #[moq_clock::test] + async fn a_late_mid_group_end_below_a_seen_object_refuses_the_track() { + for datagram in [false, true] { + let routes = Producer::new(); + let logical = logical(&routes); + let mut a = copy(); + routes.serve(a.consume()); + let mut sub = subscribe(&logical, HOUR); + let _held = match datagram { + false => { + let mut group = a.create_group(group::Info { sequence: 4 }).unwrap(); + for frame in 0..4 { + group.write_frame(ts(frame), b"x".as_ref()).unwrap(); + } + Some((group, recv(&mut sub))) + } + true => { + a.insert_wire_datagram(super::super::datagram::Stored { + datagram: super::super::datagram::Datagram { + sequence: 4, + timestamp: Some(ts(4)), + payload: bytes::Bytes::from_static(b"d"), + }, + wire: Some(Arc::new(super::super::datagram::Wire { + object_id: Some(4), + priority: None, + end_of_group: false, + status: None, + properties: Default::default(), + })), + deflated: None, + }) + .unwrap(); + assert!(sub.poll_recv_datagram(&kio::Waiter::noop()).is_ready()); + None + } + }; + let mut b = copy(); + routes.serve(b.consume()); + b.finish_at(Position { group: 4, frame: 2 }).unwrap(); + let res = sub.poll_finished(&kio::Waiter::noop()); + assert!( + matches!(res, Poll::Ready(Err(Error::ProtocolViolation))), + "datagram={datagram}: {res:?}" + ); + } + } + + /// The newest datagram handed out bounds a late end after the budget forgets its identity. + #[moq_clock::test] + async fn an_end_below_an_expired_datagram_refuses_the_track() { + let routes = Producer::new(); + let logical = logical(&routes); + let mut a = copy(); + routes.serve(a.consume()); + let mut sub = subscribe(&logical, Duration::from_secs(1)); + a.insert_datagram(10, ts(10), b"d".as_ref()).unwrap(); + assert!(sub.poll_recv_datagram(&kio::Waiter::noop()).is_ready()); + let mut b = copy(); + routes.serve(b.consume()); + b.insert_datagram(1, ts(1), b"d".as_ref()).unwrap(); + assert!(sub.poll_recv_datagram(&kio::Waiter::noop()).is_ready()); + moq_clock::sleep(Duration::from_secs(2)).await; + assert!(sub.poll_recv_datagram(&kio::Waiter::noop()).is_pending()); + assert_eq!(copies(&routes), 1); + b.finish_at(Position::group(5)).unwrap(); + assert!(matches!( + sub.poll_finished(&kio::Waiter::noop()), + Poll::Ready(Err(Error::ProtocolViolation)) + )); + } + + #[moq_clock::test] + async fn route_flaps_keep_one_copy_per_distinct_route() { + let routes = Producer::new(); + let logical = logical(&routes); + let tracks = [copy(), copy()]; + routes.serve(tracks[0].consume()); + let mut sub = subscribe(&logical, HOUR); + write(&tracks[0], 0); + let held = recv(&mut sub); + for round in 0..32 { + routes.serve(tracks[round % 2].consume()); + assert!(sub.poll_recv_group(&kio::Waiter::noop()).is_pending()); + assert!( + copies(&routes) <= 2, + "round {round} retained {} copies", + copies(&routes) + ); + } + // Returning to a route must not leave its old replacement cap in place. + write(&tracks[1], 1); + assert_eq!(recv(&mut sub).sequence, 1); + routes.serve(tracks[0].consume()); + write(&tracks[0], 2); + assert_eq!(recv(&mut sub).sequence, 2); + drop(held); + } + + /// A returning route resumes where a fresh copy would: what it buffered while replaced, + /// below what another route already handed out, is not handed out again. + #[moq_clock::test] + async fn a_returning_route_resumes_at_the_readers_newest() { + let routes = Producer::new(); + let logical = logical(&routes); + let a = copy(); + let b = copy(); + routes.serve(a.consume()); + let mut sub = subscribe(&logical, HOUR); + write(&a, 0); + let held = recv(&mut sub); + routes.serve(b.consume()); + write(&b, 10); + write(&b, 20); + assert_eq!(drain(&mut sub, false), [10, 20]); + write(&a, 15); + assert_eq!(copies(&routes), 2, "the held group keeps the first route"); + routes.serve(a.consume()); + assert_eq!(drain(&mut sub, false), [] as [u64; 0]); + write(&a, 21); + assert_eq!(drain(&mut sub, false), [21]); + drop(held); + } + + #[moq_clock::test] + async fn a_replaced_copy_is_not_kept_for_a_gap_outside_the_window() { + let routes = Producer::new(); + let logical = logical(&routes); + let a = copy(); + routes.serve(a.consume()); + let subscription = Subscription::default() + .with_max_delay(HOUR) + .with_ranges([track::Range::groups(0..1), track::Range::groups(2..3)]); + let mut sub = logical + .consume() + .subscribe(subscription) + .now_or_never() + .unwrap() + .unwrap(); + write(&a, 0); + assert_eq!(drain(&mut sub, false), [0]); + let b = copy(); + routes.serve(b.consume()); + write(&b, 2); + assert_eq!(drain(&mut sub, false), [2]); + assert_eq!( + copies(&routes), + 1, + "the only missing group, 1, is outside the reader's window" + ); + } + /// Retaining an old copy's cache does not keep a parked logical track live. #[test] fn a_parked_warm_cache_is_not_live() { @@ -2718,7 +3187,7 @@ mod test { let mut sub = subscribe(&logical, HOUR); write(&a, 0); assert_eq!(drain(&mut sub, ordered), [0]); - a.finish().unwrap(); + a.clone().abort(Error::Dropped).unwrap(); let b = copy(); routes.serve(b.consume()); let c = copy(); @@ -3083,9 +3552,9 @@ mod test { sub.start_at(0); assert_eq!(drain(&mut sub, false), Vec::::new()); - // The replaced copy finishes with the held group. + // The replaced copy ends with the held group, declaring no end: another route continues the track. held.finish().unwrap(); - a.finish().unwrap(); + a.clone().abort(Error::Dropped).unwrap(); drop(reading); assert_eq!(drain(&mut sub, false), Vec::::new()); write(&b, 4); @@ -4069,6 +4538,56 @@ mod test { )); } + /// A flap back to the route being read is no route change for an in-flight group, so + /// a sparse group the reader never polled across it keeps reading its own copy. + #[test] + #[cfg(feature = "async")] + fn an_unpolled_flap_back_keeps_reading_a_sparse_group() { + let routes = Producer::new(); + let logical = logical(&routes); + let a = copy(); + let mut group = a.create_group(group::Info { sequence: 0 }).unwrap(); + group + .set_wire(group::Wire { + sparse: false, + first_object: true, + first_id: 6, + extensions: false, + has_end: true, + fetchable: false, + last: None, + }) + .unwrap(); + let object = |group: group::Producer, id: u64, payload: &'static [u8]| { + let header = crate::frame::Header { + info: crate::frame::Info { + size: payload.len() as u64, + timestamp: Some(ts(id)), + }, + fields: Some(Arc::new(super::super::object::Fields { + id, + properties: Default::default(), + })), + inflated: None, + }; + let mut frame = group.create_object(header).unwrap(); + frame.write(bytes::Bytes::from_static(payload)).unwrap(); + frame.finish().unwrap() + }; + group = object(group, 6, b"a"); + group = object(group, 8, b"b"); + routes.serve(a.consume()); + let mut sub = subscribe(&logical, Duration::MAX); + let mut reading = recv(&mut sub); + assert_eq!(read(&mut reading), Some(b"a".to_vec())); + assert_eq!(read(&mut reading), Some(b"b".to_vec())); + routes.serve(copy().consume()); + routes.serve(a.consume()); + assert!(reading.read_frame().now_or_never().is_none()); + let _group = object(group, 9, b"c"); + assert_eq!(read(&mut reading), Some(b"c".to_vec())); + } + /// A replacement copy that joined past a gap, holding the group from Object 6. #[cfg(feature = "async")] fn joined_past_a_gap(objects: &[(u64, &'static [u8])]) -> track::Producer { diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 5a0f04c654d..05c6622d3e7 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -4088,6 +4088,18 @@ enum Source { } impl Consumer { + /// Whether these handles resolve to the same cached track, without subscribing. + pub(super) fn same_channel(&self, other: &Self) -> bool { + self.with_state(|left| { + other.with_state(|right| left.zip(right).is_some_and(|(left, right)| left.same_channel(right))) + }) + } + + /// An already declared end, without resolving a pending subscription or registering a wake. + pub(super) fn declared_end(&self) -> Option { + self.with_state(|state| state?.read().end) + } + fn new(name: Arc, state: kio::ConsumerWeak) -> Self { let broadcast = state.read().broadcast.clone(); Self {