From 4f8de4bd86a16ac568f28478768034f64857f9f7 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 20 Oct 2026 18:37:05 -0700 Subject: [PATCH 1/7] quest: claim m1/resume-closed Co-Authored-By: Claude Opus 5.5 From 8303e96bedb23a217ed36048be751757fb3cc5e0 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 20 Oct 2026 18:51:58 -0700 Subject: [PATCH 2/7] fix(net): end a resumed track the way a direct one ends A resumed JS subscriber settles closed once its serving copy ends cleanly, leaves the route and lets go of copies with nothing left to read, keeps the first end any copy declared (refusing a different one), resolves largest() undefined at a clean end, and carries a close(abort) to the groups it handed out and any recovery in flight. Co-Authored-By: Claude Opus 5.5 --- js/net/src/group.ts | 5 + js/net/src/internal.ts | 10 +- js/net/src/resume.test.ts | 159 +++++++++++++++++++++++++++++++ js/net/src/resume.ts | 192 ++++++++++++++++++++++++++++---------- js/net/src/track.ts | 15 ++- quest/m1/README.md | 1 - quest/m1/resume-closed.md | 85 ----------------- 7 files changed, 328 insertions(+), 139 deletions(-) delete mode 100644 quest/m1/resume-closed.md diff --git a/js/net/src/group.ts b/js/net/src/group.ts index b2f6274f3b..52cd497cfd 100644 --- a/js/net/src/group.ts +++ b/js/net/src/group.ts @@ -886,6 +886,11 @@ export class Consumer { /** Closes the group, optionally with an error to abort readers. Idempotent. */ close(abort?: Error) { + // A copy failure recovery was continuing past ends with the abort instead. + if (abort && this.#recover && this.#state.closed.peek() instanceof Error) { + this.#terminal ??= abort; + if (this.#verdict.peek() === undefined) this.#verdict.set(this.#terminal); + } this.#abandon(); this.#recover = undefined; // Closing is terminal for this reader, even with frames still unread. diff --git a/js/net/src/internal.ts b/js/net/src/internal.ts index c342ed8218..1500268d8a 100644 --- a/js/net/src/internal.ts +++ b/js/net/src/internal.ts @@ -298,7 +298,7 @@ export interface ResumeSource { forgetPastBudget(delivered: Delivered): void; /** The newest sequence any copy has seen. */ latest(): number | undefined; - /** The serving copy's declared exclusive end. */ + /** The logical track's declared exclusive end, as the first copy to know it declared it. */ end(): Location | undefined; /** Resolve with the logical track's exclusive end once known. */ finished(): Promise; @@ -413,6 +413,11 @@ export const hooks: { tryRecvDatagram: (subscriber: Subscriber) => RecvDatagram; /** Whether a buffered group in `[start, end)` that `delivered` lacks still waits to be read. */ hasUnreadGroup: (subscriber: Subscriber, start: number, end: number | undefined, delivered: Delivered) => boolean; + /** + * Whether a subscriber's track ended and it holds nothing left to read: no datagram, and no + * group in `[start, end)` that `delivered` lacks. + */ + spent: (subscriber: Subscriber, start: number, end: number | undefined, delivered: Delivered) => boolean; /** How many runs of handed-out groups a subscriber remembers: one per gap, plus the floor. Tests only. */ deliveredRuns: (subscriber: Subscriber) => number; /** Forget the gaps in another reader's `delivered` below each group past this subscriber's budget. */ @@ -566,6 +571,9 @@ export const hooks: { hasUnreadGroup: () => { throw new Error("track.ts not loaded"); }, + spent: () => { + throw new Error("track.ts not loaded"); + }, staleAt: () => { throw new Error("track.ts not loaded"); }, diff --git a/js/net/src/resume.test.ts b/js/net/src/resume.test.ts index 0693e5bbbe..d81d5457b3 100644 --- a/js/net/src/resume.test.ts +++ b/js/net/src/resume.test.ts @@ -1354,3 +1354,162 @@ test("the largest follows the serving copy across a swap without moving back, an a.close(); b.close(); }); + +test("a group-only reader of a finite track settles closed and lets go of its copy", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const ta = copy(a); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + ta.write(0); + // Declared but still open, so the copy's subscription is what holds the demand. + ta.producer.finishAt({ group: 1, frame: 0 }); + expect(drain(sub)).toEqual([0]); + expect(await sub.recvGroup()).toBeUndefined(); + await settle(); + expect(sub.closed.peek()).toBeNull(); + expect(ta.producer.demand().wanted.peek()).toBeUndefined(); + + sub.close(); + front.consumer.close(); + a.close(); +}); + +test("a group-only fork of a finite track settles closed and lets go of its copy", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const ta = copy(a); + front.serve(a.consume()); + const parent = front.consumer.track("seq").subscribe(); + const fork = parent.fork(); + parent.close(); + ta.write(0); + ta.producer.finishAt({ group: 1, frame: 0 }); + expect(drain(fork)).toEqual([0]); + expect(await fork.recvGroup()).toBeUndefined(); + await settle(); + expect(fork.closed.peek()).toBeNull(); + expect(ta.producer.demand().wanted.peek()).toBeUndefined(); + + fork.close(); + front.consumer.close(); + a.close(); +}); + +test("the serving copy closing cleanly settles closed and leaves its datagrams readable", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const ta = copy(a); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + ta.write(0); + ta.producer.appendDatagram(undefined, enc.encode("d")); + ta.producer.close(); + await settle(); + expect(sub.closed.peek()).toBeNull(); + expect(drain(sub)).toEqual([0]); + expect(dec.decode((await sub.recvDatagram())?.payload)).toBe("d"); + expect(await sub.recvDatagram()).toBeUndefined(); + expect(await sub.recvGroup()).toBeUndefined(); + + sub.close(); + front.consumer.close(); + a.close(); +}); + +test("an end a replaced copy declared holds across a swap", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const b = new BroadcastProducer(); + const ta = copy(a); + const tb = copy(b); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + ta.write(0); + ta.producer.finishAt({ group: 2, frame: 0 }); + await settle(); + + // b serves before its subscription learns the end. + front.serve(b.consume()); + await settle(); + expect(sub.end()).toEqual({ group: 2, frame: 0 }); + expect(await sub.finished()).toEqual({ group: 2, frame: 0 }); + + // A copy declaring another end claims different content. + tb.producer.finishAt({ group: 3, frame: 0 }); + await settle(); + expect(sub.closed.peek()).toBeInstanceOf(Error); + expect(sub.end()).toEqual({ group: 2, frame: 0 }); + + sub.close(); + front.consumer.close(); + a.close(); + b.close(); +}); + +test("largest ends once the serving copy reaches its end, with the front still open", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const ta = copy(a); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + ta.write(0); + ta.producer.finishAt({ group: 1, frame: 0 }); + expect(await sub.largest()).toEqual({ group: 0, frame: 0 }); + const next = sub.largest(); + expect(await isPending(next)).toBe(false); + expect(await next).toBeUndefined(); + + sub.close(); + front.consumer.close(); + a.close(); +}); + +test("closing with an abort rejects a pending frame read of a group handed out", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const ta = copy(a); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + ta.write(0, 1, false); + const group = await sub.recvGroup(); + if (!group) throw new Error("expected group 0"); + expect(dec.decode((await group.readFrame())?.payload)).toBe("0.0"); + const read = group.readFrame(); + const abort = new Error("aborted"); + sub.close(abort); + await expect(read).rejects.toBe(abort); + + front.consumer.close(); + a.close(); +}); + +test("closing with an abort settles a recovery in flight with it", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const b = new BroadcastProducer(); + const ta = copy(a); + const tb = copy(b); + const stalled = ta.write(0, 1, false); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + const group = sub.tryRecvGroup(); + if (!group) throw new Error("expected group 0"); + expect(dec.decode((await group.readFrame())?.payload)).toBe("0.0"); + + // The next route does not have the group yet: recovery fetches it there and waits. + front.serve(b.consume()); + await settle(); + stalled.close(new StreamError(StreamCode.SessionClosed)); + const read = group.readFrame(); + await settle(); + const abort = new Error("aborted"); + sub.close(abort); + // A replacement arriving afterwards is not adopted. + tb.write(0, 2); + await expect(read).rejects.toBe(abort); + + front.consumer.close(); + a.close(); + b.close(); +}); diff --git a/js/net/src/resume.ts b/js/net/src/resume.ts index afc76f21b2..ccbb17d230 100644 --- a/js/net/src/resume.ts +++ b/js/net/src/resume.ts @@ -22,14 +22,20 @@ * subscriber already handed out, the same record that keeps a direct reader from taking a * re-created group twice. * + * The logical track ends cleanly once its serving copy does. Its `closed` settles and the reader + * leaves the route, keeping each copy only while it holds something left to read, as a direct + * reader keeps its buffer. Its end is the first one any copy declared, which every later copy + * has to match. + * * @module */ import { type Dispose, type Getter, race, Signal } from "@moq/signals"; import * as broadcast from "./broadcast.ts"; -import type { Delivered } from "./delivered.ts"; +import { Delivered } from "./delivered.ts"; import { NotFound, StreamCode, StreamError } from "./error.ts"; import type { Consumer as GroupConsumer } from "./group.ts"; import { + finalSequence, type GroupRecover, groupBounds, hooks, @@ -260,8 +266,16 @@ class Reader implements ResumeSource { #changed = new Signal(0); #dispose: Dispose[] = []; #closed = false; - /** The channels that ran out cleanly. */ - #drained = new Set<"groups" | "datagrams">(); + /** The logical track ended cleanly: the reader drains what its copies hold and follows no route. */ + #ended = false; + /** The serving copy's largest when the track ended, kept once that copy goes. */ + #last?: { largest?: track.Location; ended: true }; + /** The first end any copy declared: the logical track's, whichever copy serves. */ + #declared?: track.Location; + /** The record of groups handed out, once groups are read. */ + #delivered?: Delivered; + /** Groups handed out and still open, which end with the reader. */ + #out = new Set(); /** Records the properties a copy committed, and which source it came from. */ #record: (info: track.Info, source: Source) => void; @@ -299,7 +313,7 @@ class Reader implements ResumeSource { /** Follow the route: subscribe to a new copy and mark the ones it replaced. */ sync(): void { - if (this.#closed) return; + if (this.#closed || this.#ended) return; const route = this.route; if (this.#generation === route.generation) return; this.#generation = route.generation; @@ -337,6 +351,19 @@ class Reader implements ResumeSource { }, () => {}, ); + copy.sub.finished().then( + // A copy already let go says nothing more about the track. + (end) => { + if (this.#copies.includes(copy)) this.#declare(end); + }, + () => {}, + ); + void copy.sub.closed.then((closed) => { + // The serving copy closing cleanly is the logical track's end. + if (closed !== null) return; + this.sync(); + if (this.#serving() === copy) this.#conclude(); + }); this.#changed.update((n) => n + 1); } @@ -418,6 +445,8 @@ class Reader implements ResumeSource { } tryGroup(ordered: boolean, delivered: Delivered): Recv { + if (this.#closed) return { kind: "done" }; + this.#delivered = delivered; this.sync(); // The lowest group any copy holds next, in either order: a replaced copy's buffered group // comes before a later one the serving copy already has. Arrival order still delivers a @@ -451,8 +480,8 @@ class Reader implements ResumeSource { break; } } - this.#letGo(delivered); - return this.#end("groups", (copy) => copy.done); + this.#letGo(); + return this.#end((copy) => copy.done); } // The copy whose next group is the lowest deliverable one, dropping any it holds next that @@ -479,6 +508,8 @@ class Reader implements ResumeSource { } tryDatagram(delivered: Delivered | undefined): RecvDatagram { + if (this.#closed) return { kind: "done" }; + if (delivered) this.#delivered = delivered; this.sync(); for (let index = this.#copies.length - 1; index >= 0; index--) { const copy = this.#copies[index]; @@ -497,8 +528,8 @@ class Reader implements ResumeSource { break; } } - this.#letGo(delivered); - const end = this.#end("datagrams", (copy) => copy.datagramsDone); + this.#letGo(); + const end = this.#end((copy) => copy.datagramsDone); if (end.kind === "error" || end.kind === "done") return end; return { kind: "idle" }; } @@ -507,53 +538,71 @@ class Reader implements ResumeSource { * Whether the track ended, once the serving copy's channel (as `done` reads it) ran out: * cleanly, or with a verdict a route failure does not excuse. */ - #end( - channel: "groups" | "datagrams", - done: (copy: Copy) => Error | null | undefined, - ): Recv & { kind: "idle" | "done" | "error" } { + #end(done: (copy: Copy) => Error | null | undefined): Recv & { kind: "idle" | "done" | "error" } { const route = this.route; const last = this.#copies.at(-1); const serving = last?.generation === this.#generation ? last : undefined; const ended = serving ? done(serving) : undefined; // The serving copy drained a complete track: that is its end, whichever route would - // serve it next. - if (serving && ended === null) return this.#finish({ kind: "done" }, channel); + // serve it next. Once the track ended, a copy let go held nothing more. + if (serving ? ended === null : this.#ended) return this.#finish({ kind: "done" }); if (ended instanceof Error) { // A refusal from a route that still serves is the track's answer. - if (!routeFailed(ended)) return this.#finish({ kind: "error", error: ended }, channel); - if (route.refusal) return this.#finish({ kind: "error", error: route.refusal }, channel); + if (!routeFailed(ended)) return this.#finish({ kind: "error", error: ended }); + if (route.refusal) return this.#finish({ kind: "error", error: route.refusal }); } // Refused before any copy fed it: nothing will. - if (!last && route.refusal) return this.#finish({ kind: "error", error: route.refusal }, channel); + if (!last && route.refusal) return this.#finish({ kind: "error", error: route.refusal }); if (route.concluded) { // No front is left to replace the copy: the track ends as it does. - if (!serving) return this.#finish({ kind: "error", error: dropped() }, channel); - if (ended instanceof Error) return this.#finish({ kind: "error", error: ended }, channel); + if (!serving) return this.#finish({ kind: "error", error: dropped() }); + if (ended instanceof Error) return this.#finish({ kind: "error", error: ended }); } return { kind: "idle" }; } - // End the logical track on a failure, or cleanly once both channels drained: groups and - // datagrams end apart, and one running out leaves the other's backlog readable. - #finish(end: T, channel: "groups" | "datagrams"): T { - if (end.kind === "error") { - this.#shell.close(end.error); - return end; - } - this.#drained.add(channel); - if (this.#drained.size === 2) this.#shell.close(); + // End the logical track: at once on a failure, or cleanly once a channel drained it. The + // other channel's backlog stays readable. + #finish(end: T): T { + if (end.kind === "error") this.#shell.close(end.error); + else this.#conclude(); return end; } + /** + * End the logical track cleanly: settle `closed` and leave the route, keeping each copy only + * while it holds something left to read. + */ + #conclude(): void { + if (this.#closed || this.#ended) return; + this.#ended = true; + // Learned before any copy that knows them goes. + this.#learnEnd(); + const serving = this.#serving(); + this.#last = { largest: serving && hooks.peekLargest(serving.sub).largest, ended: true }; + this.#shell.close(); + this.route.leave(this); + this.#letGo(); + this.wake(); + } + /** * Drop replaced copies only once neither a handed-out nor an unread group needs them. One not - * yet replaced stays, even between routes: it is still the newest copy there is. + * yet replaced stays, even between routes: it is still the newest copy there is, until the + * track ended and it holds nothing left to read. */ - #letGo(delivered?: Delivered): void { + #letGo(): void { + const delivered = this.#delivered; const cursor = this.#shell.cursor.peek(); this.#copies = this.#copies.filter((copy) => { + if (copy.lease > 0) return true; // The serving copy, or one between routes with nothing replacing it yet. - if (!copy.replaced || copy.lease > 0) return true; + if (!copy.replaced) { + const start = Math.max(cursor.start, copy.floor ?? 0); + if (!this.#ended || !hooks.spent(copy.sub, start, undefined, delivered ?? new Delivered())) return true; + this.#drop(copy); + return false; + } // Its group channel ran out, or nobody reads groups at all: it feeds nothing more. if (copy.done !== undefined || !delivered) { this.#drop(copy); @@ -567,8 +616,8 @@ class Reader implements ResumeSource { }); } - #drop(copy: Copy): void { - copy.sub.close(); + #drop(copy: Copy, abort?: Error): void { + copy.sub.close(abort); copy.source.release(); } @@ -580,9 +629,26 @@ class Reader implements ResumeSource { const recover = new Recover(this, group, copy, backups); hooks.recoverGroup(group, recover); recover.watch(group); + // Closed with the reader, as a direct reader's close reaches the groups it handed out. + this.#out.add(group); + void group.closed.then(() => this.#out.delete(group)); return group; } + /** The logical track ended cleanly, so no route continues it. */ + get ended(): boolean { + return this.#ended; + } + + /** `copy`'s last lease ended: let go of it once nothing else needs it. */ + freed(copy: Copy): void { + // Once the track ended, no read may come to let go of it. + if (this.#ended) this.#letGo(); + else if (!copy.replaced) return; + // A parked read lets go of a replaced copy nothing needs any more. + this.wake(); + } + changed(fn: () => void): Dispose { let fired = false; const disposes: Dispose[] = []; @@ -620,17 +686,44 @@ class Reader implements ResumeSource { return last?.generation === this.#generation ? last : undefined; } + // The logical track's end, learning it from any copy that declared one. + #learnEnd(): track.Location | undefined { + for (const copy of this.#copies) { + const end = copy.sub.end(); + if (end) this.#declare(end); + } + return this.#declared; + } + + // Record `end` as the logical track's end. One logical track has one end, as it has one set + // of properties: a copy declaring another is refused. A mid-group end refines the group + // boundary it falls just before, as a wire end may. + #declare(end: track.Location): void { + if (this.#closed) return; + const declared = this.#declared; + if (!declared) { + this.#declared = end; + this.wake(); + } else if (declared.group === end.group && declared.frame === end.frame) { + return; + } else if (finalSequence(declared) === finalSequence(end) && (declared.frame === 0 || end.frame === 0)) { + if (declared.frame === 0) this.#declared = end; + } else { + this.#shell.close(new Incompatible(this.route.name)); + } + } + end(): track.Location | undefined { this.sync(); - return this.#serving()?.sub.end(); + return this.#learnEnd(); } async finished(): Promise { for (;;) { this.sync(); - const serving = this.#serving(); - const end = serving?.sub.end(); + const end = this.#learnEnd(); if (end) return end; + const serving = this.#serving(); const failed = serving?.sub.closed.peek(); // A copy that failed is only the end of the track if no front replaces it, and a // refused replacement leaves the refusal as the answer. @@ -661,14 +754,14 @@ class Reader implements ResumeSource { peekLargest(): { largest?: track.Location; ended: boolean } { this.sync(); const serving = this.#serving(); - return serving ? hooks.peekLargest(serving.sub) : { ended: false }; + return serving ? hooks.peekLargest(serving.sub) : (this.#last ?? { ended: false }); } async largest(seen: track.Location | undefined): Promise { for (;;) { this.sync(); const serving = this.#serving(); - const view = serving && hooks.peekLargest(serving.sub); + const view = serving ? hooks.peekLargest(serving.sub) : this.#last; const largest = view?.largest; const { start, end } = this.#shell.cursor.peek(); // A replacement copy can start below what an earlier one reported. @@ -679,8 +772,10 @@ class Reader implements ResumeSource { (!seen || largest.group > seen.group || (largest.group === seen.group && largest.frame > seen.frame)) ) return largest; - // A copy ending is its route failing: the logical track's stream ends only with the route. + // A copy failing is its route failing: the logical track's stream ends only with the + // route, or once the serving copy reaches its end cleanly. if (this.#shell.closed.peek() !== undefined) return undefined; + if (view?.ended && !(serving?.sub.closed.peek() instanceof Error)) return undefined; if ((this.route.concluded || this.route.refusal) && (!serving || view?.ended)) return undefined; await new Promise((resolve) => { const disposes: Dispose[] = []; @@ -710,12 +805,15 @@ class Reader implements ResumeSource { ); } - close(): void { + close(abort?: Error): void { if (this.#closed) return; this.#closed = true; for (const dispose of this.#dispose) dispose(); this.#dispose = []; - for (const copy of this.#copies) this.#drop(copy); + // A recovery in flight settles with the group rather than adopting a replacement. + for (const group of this.#out) group.close(abort); + this.#out.clear(); + for (const copy of this.#copies) this.#drop(copy, abort); this.#copies = []; this.route.leave(this); } @@ -831,10 +929,7 @@ class Recover implements GroupRecover { const released = this.#copy ? [this.#copy, ...this.#backups] : this.#backups; this.#copy = undefined; this.#backups = []; - let freed = false; - for (const copy of released) if (--copy.lease === 0 && copy.replaced) freed = true; - // A parked read lets go of a replaced copy nothing needs any more. - if (freed) this.#reader.wake(); + for (const copy of released) if (--copy.lease === 0) this.#reader.freed(copy); } get changed(): Getter { @@ -894,6 +989,8 @@ class Recover implements GroupRecover { const held = reader.holding(this.#sequence, current)[0]; const cached = held && hooks.cachedGroup(held.sub, this.#sequence); if (cached) return this.#replacement(cached, held); + // Once the track ended, no route continues it: only a copy holding it could. + if (reader.ended) throw failed; } if (!serving) { // Nothing serves: a failed copy waits for a route, unless none will come. @@ -930,10 +1027,11 @@ class Recover implements GroupRecover { expiry: hooks.staleAt(copy.sub, this.#sequence), adopt: () => { // A later switch must keep this copy subscribed while the recovered group still - // reads it, just like a group handed out from that copy directly. + // reads it, just like a group handed out from that copy directly. Leased first, so + // letting go of the copies it leaves never drops it. + copy.lease++; this.#release(); this.#copy = copy; - copy.lease++; }, discard: () => group.close(), }; diff --git a/js/net/src/track.ts b/js/net/src/track.ts index d6f11592e3..1af28b19ec 100644 --- a/js/net/src/track.ts +++ b/js/net/src/track.ts @@ -1820,6 +1820,14 @@ export class Subscriber { !delivered.has(group.sequence) && !(group.closed.peek() instanceof Error), ); + hooks.spent = (subscriber, start, end, delivered) => { + const state = subscriber.#state; + return ( + (state.closed.peek() !== undefined || subscriber.#complete()) && + state.datagrams.peek().length === 0 && + !hooks.hasUnreadGroup(subscriber, start, end, delivered) + ); + }; hooks.staleAt = (subscriber, sequence) => ({ expired: () => subscriber.#staleAt(sequence, subscriber.#drift(), true), changed: subscriber.#expiryChanged(), @@ -2211,9 +2219,9 @@ export class Subscriber { #tryResumed(ordered: boolean): Recv { const resume = this.#resume as ResumeSource; this.#groupsRead = true; + // A clean end leaves what the copies hold readable, as a direct reader's buffer is. const closed = this.#state.closed.peek(); if (closed instanceof Error) return { kind: "error", error: closed }; - if (closed !== undefined) return { kind: "done" }; const recv = resume.tryGroup(ordered, this.#delivered); if (recv.kind === "group") this.#handOut(recv.group.sequence, ordered); return recv; @@ -2312,10 +2320,7 @@ export class Subscriber { // The synchronous half of #recvDatagram. #tryDatagram(): RecvDatagram { const closed = this.#state.closed.peek(); - if (this.#resume) { - if (closed instanceof Error) return { kind: "error", error: closed }; - if (closed !== undefined) return { kind: "done" }; - } + if (this.#resume && closed instanceof Error) return { kind: "error", error: closed }; // The group range bounds datagrams too, judged when each is read. One outside it is // gone for good: nothing holds it for a later raise, unlike a group past the cap. const { start, end } = this.#cursor.peek(); diff --git a/quest/m1/README.md b/quest/m1/README.md index 19cd0cc438..2b3694b1d5 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -103,7 +103,6 @@ lands before the cut. - [TS fidelity and recovery](/quest/m1/ts-reliability/README.md) - both modes have explicit profiles, automated validation, loss tests, and a dedicated guide - [A widened max age re-offers cached groups](/quest/m1/resolved-demand.md) - a warm relay serves a reader with a wider max age the cached groups a cold relay would, with no wire change - [Drop set](/quest/m1/drop-set.md) - each cursor judges missing groups against its own budget, so a narrow reader drops promptly and a widening re-offers groups already reported dropped on lite-07 -- [Resumed close](/quest/m1/resume-closed.md) - a resumed JS track keeps its declared end across copies, and its `closed` and `largest()` settle on a clean end - [Resumed in-flight groups](/quest/m1/resume-in-flight.md) - a replaced route copy stays while a group below its bound is in flight or it backs a handed-out group late, and no longer than the reader's budget - [Resume through an outage](/quest/m1/resume-outage.md) - a same-epoch resume after an unclosed route dies delivers the backlog its max age still holds, pinned by a regression test - [Resumed datagrams](/quest/m1/resume-datagrams.md) - a resumed track skips only datagrams it already delivered, not every lower sequence diff --git a/quest/m1/resume-closed.md b/quest/m1/resume-closed.md deleted file mode 100644 index 4196a29b6f..0000000000 --- a/quest/m1/resume-closed.md +++ /dev/null @@ -1,85 +0,0 @@ -# [M] A resumed track's end holds across copies and settles every reader - -## Goal - -A resumed JS track subscriber (`Track.Subscriber` reading a front's route -copies) ends the way a direct one does: - -- It settles `closed` once its logical track ends cleanly, whether the - application reads groups, datagrams, or both. A group-only reader that reaches - `recvGroup()` EOF on a finite track sees `closed` settle and releases its - `Reader`, copies, and route sources, while a reader of the other channel still - drains what is buffered. -- A declared end, once any copy reported it, stays the logical track's end - across route swaps: `end()` and `finished()` never regress to unknown because - the new copy hasn't learned it yet, and a copy reporting a different end is - refused. -- `largest()` resolves `undefined` once the serving copy ends cleanly at its - declared end, even while the front stays open for other tracks. -- `close(abort)` reaches every group already handed out, so a pending frame - read rejects with the abort rather than ending cleanly. - -## Plan - -Found by hosted Codex reviews of #5836. - -**Closed with one channel polled** (`resume.ts` `Reader.#finish`). Today a -clean end adds the channel that ran out to `#drained`, and the shell closes only -once both `"groups"` and `"datagrams"` are there. Nothing adds `"datagrams"` for -a reader that never calls `recvDatagram()`, so `closed` stays pending and the -route keeps the reader. Errors already close at once. - -- Settle `closed` when the serving copy's track ends cleanly, from the copy's - own end (its `closed` and declared end), instead of waiting for both read - channels to be polled to EOF. -- Keep each channel's buffered backlog readable after that: one channel running - out must not cut off the other's unread data, as `draining groups to their end - leaves buffered datagrams readable` checks today. - -**A declared end across swaps** (`Reader.end()` and `finished()`). Both read the -serving copy, so when copy A has declared the end and copy B takes over before -its subscription learns it, `end()` returns `undefined` and `finished()` waits -on B. Wire publishers can then advertise or wait on the wrong boundary. - -- Record the first end any copy declares on the reader and answer from it. - Check each later copy's end against it, as `resolveTrackInfo` checks track - properties, and refuse a mismatch. - -**Largest after a clean end** (`Reader.largest(seen)`). It returns only once -the route or shell concluded, or the route was refused. A finite serving copy -that reaches its declared end cleanly sets `view.ended` with `closed` `null`, -neither of which ends the wait, so `largest()` waits on a stream that has ended. - -- Treat a clean end of the serving copy as terminal there, while a copy that - failed with its route still waits for the next route. - -**An abort reaches the copies** (`Reader.close()`). `Subscriber.close(abort)` -in `js/net/src/track.ts` calls `this.#resume?.close(abort)`, but -`Reader.close()` takes no argument and `#drop` clean-closes every copy. A group -already handed out is backed by a copy rather than the outer buffer, so its -pending frame read can end with clean EOF instead of the abort a direct -subscriber gives. - -- Take the abort in `Reader.close(abort?)` and close each copy with it, so a - handed-out group and its `Recover` end with the same error. A recovery in - flight must settle with the abort too, not adopt a replacement. - -For all four, check whether a direct (non-resumed) `Track.Subscriber` and the -Rust `resume::Subscriber` already behave this way, and match them. - -Regression tests: - -- A group-only reader of a finite resumed track sees `closed` settle and the - route release its copy. -- The same for a fork: a group-only fork of a finite resumed track sees its - `closed` settle and the route release the fork's copy. Forks became logical - readers on the route in [#6024](https://github.com/moq-dev/moq/pull/6024), - so they share the `Reader.#finish` path this quest fixes. -- A swap after copy A declared the end keeps `end()` at that end, and - `finished()` resolves with it before B learns it. -- `largest()` on a finite resumed track resolves `undefined` once its serving - copy ends cleanly, with the front still open. -- `close(abort)` on a resumed subscriber with a group handed out and a frame - read pending rejects that read with the abort. - -Public API: behavior only. Wire: none. From 847dd75b1125d5707e18671b0fa78090ff38e179 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 11:23:52 -0700 Subject: [PATCH 3/7] fix(net): a resumed copy ignores out-of-range datagrams and refuses a late end A datagram below the group floor no longer keeps a finite track's copy after a clean end, since a read would only discard it. A retained copy declaring another end after the track ended cleanly fails the reads with Incompatible instead of serving content it claims differently. Co-Authored-By: Claude Opus 5.5 --- js/net/src/internal.ts | 4 +-- js/net/src/resume.test.ts | 59 +++++++++++++++++++++++++++++++++++++++ js/net/src/resume.ts | 10 +++++-- js/net/src/track.ts | 11 +++++++- 4 files changed, 79 insertions(+), 5 deletions(-) diff --git a/js/net/src/internal.ts b/js/net/src/internal.ts index b45889b92a..38dbdabb97 100644 --- a/js/net/src/internal.ts +++ b/js/net/src/internal.ts @@ -432,8 +432,8 @@ export const hooks: { /** Whether a buffered group in `[start, end)` that `delivered` lacks still waits to be read. */ hasUnreadGroup: (subscriber: Subscriber, start: number, end: number | undefined, delivered: Delivered) => boolean; /** - * Whether a subscriber's track ended and it holds nothing left to read: no datagram, and no - * group in `[start, end)` that `delivered` lacks. + * Whether a subscriber's track ended and it holds nothing left to read: no datagram within its + * cursor, and no group in `[start, end)` that `delivered` lacks. */ spent: (subscriber: Subscriber, start: number, end: number | undefined, delivered: Delivered) => boolean; /** How many runs of handed-out groups a subscriber remembers: one per gap, plus the floor. Tests only. */ diff --git a/js/net/src/resume.test.ts b/js/net/src/resume.test.ts index e4558da7d4..7c1d9bc12c 100644 --- a/js/net/src/resume.test.ts +++ b/js/net/src/resume.test.ts @@ -1812,3 +1812,62 @@ test.each(["lands", "expired"])( } }, ); + +test("a datagram below the group floor does not keep a finite track's copy", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const ta = copy(a); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + ta.producer.appendDatagram(undefined, enc.encode("d")); + await settle(); + // The floor passes the datagram: a datagram read would only discard it. + sub.setGroups({ start: { included: 1 } }); + ta.write(1); + ta.producer.finishAt({ group: 2, frame: 0 }); + expect(drain(sub)).toEqual([1]); + expect(await sub.recvGroup()).toBeUndefined(); + await settle(); + expect(sub.closed.peek()).toBeNull(); + expect(ta.producer.demand().wanted.peek()).toBeUndefined(); + + sub.close(); + front.consumer.close(); + a.close(); +}); + +test("a retained copy declaring another end after a clean end fails the reads", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const b = new BroadcastProducer(); + const ta = copy(a); + const tb = copy(b); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe({ maxDelay: SECOND }); + try { + ta.write(0); + ta.write(2); + expect(drain(sub)).toEqual([0, 2]); + + // The new route ends at 3, while the old one still owes group 1. + tb.write(2); + tb.producer.finishAt({ group: 3, frame: 0 }); + tb.producer.close(); + front.serve(b.consume()); + await settle(); + ta.write(1); + await settle(); + expect(sub.closed.peek()).toBeNull(); + + // The copy still holding group 1 claims another end: its content is refused. + ta.producer.finishAt({ group: 5, frame: 0 }); + await settle(); + expect(sub.end()).toEqual({ group: 3, frame: 0 }); + await expect(sub.recvGroup()).rejects.toThrow("changed its properties"); + } finally { + sub.close(); + front.consumer.close(); + a.close(); + b.close(); + } +}); diff --git a/js/net/src/resume.ts b/js/net/src/resume.ts index e5a0591448..ab171ae211 100644 --- a/js/net/src/resume.ts +++ b/js/net/src/resume.ts @@ -275,6 +275,8 @@ class Reader implements ResumeSource { #changed = new Signal(0); #dispose: Dispose[] = []; #closed = false; + /** A copy declared another end after the track ended cleanly: reads fail with this. */ + #refused?: Error; /** The logical track ended cleanly: the reader drains what its copies hold and follows no route. */ #ended = false; /** The serving copy's largest when the track ended, kept once that copy goes. */ @@ -470,7 +472,7 @@ class Reader implements ResumeSource { } tryGroup(ordered: boolean, delivered: Delivered): Recv { - if (this.#closed) return { kind: "done" }; + if (this.#closed) return this.#refused ? { kind: "error", error: this.#refused } : { kind: "done" }; this.#delivered = delivered; this.sync(); this.#groupsRead = true; @@ -540,7 +542,7 @@ class Reader implements ResumeSource { } tryDatagram(delivered: Delivered): RecvDatagram { - if (this.#closed) return { kind: "done" }; + if (this.#closed) return this.#refused ? { kind: "error", error: this.#refused } : { kind: "done" }; this.#delivered = delivered; this.sync(); for (let index = this.#copies.length - 1; index >= 0; index--) { @@ -837,6 +839,10 @@ class Reader implements ResumeSource { return; } else if (finalSequence(declared) === finalSequence(end) && (declared.frame === 0 || end.frame === 0)) { if (declared.frame === 0) this.#declared = end; + } else if (this.#ended) { + // `closed` already settled cleanly: stop reading the copies that claim other content. + this.#refused = new Incompatible(this.route.name); + this.close(this.#refused); } else { this.#shell.close(new Incompatible(this.route.name)); } diff --git a/js/net/src/track.ts b/js/net/src/track.ts index 6327406c32..b77ed44f8b 100644 --- a/js/net/src/track.ts +++ b/js/net/src/track.ts @@ -1849,9 +1849,18 @@ export class Subscriber { ); hooks.spent = (subscriber, start, end, delivered) => { const state = subscriber.#state; + // A datagram outside the cursor is gone for good, as a read would discard it. + const cursor = subscriber.#cursor.peek(); + const readable = state.datagrams + .peek() + .some( + (datagram) => + datagram.sequence >= cursor.start && + (cursor.end === undefined || datagram.sequence < cursor.end), + ); return ( (state.closed.peek() !== undefined || subscriber.#complete()) && - state.datagrams.peek().length === 0 && + !readable && !hooks.hasUnreadGroup(subscriber, start, end, delivered) ); }; From b25fdae65c578db0a7cf8531c45b142fa7e76f98 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 11:35:37 -0700 Subject: [PATCH 4/7] fix(net): keep the ended serving copy while a group read elsewhere may continue from it After a clean end, the serving copy stays while it holds or may still get a group handed out from a copy it replaced, so a failure of that copy continues from it instead of ending the group. Co-Authored-By: Claude Opus 5.5 --- js/net/src/resume.test.ts | 39 +++++++++++++++++++++++++++++++++++++++ js/net/src/resume.ts | 18 +++++++++++++----- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/js/net/src/resume.test.ts b/js/net/src/resume.test.ts index 7c1d9bc12c..5292bcad7b 100644 --- a/js/net/src/resume.test.ts +++ b/js/net/src/resume.test.ts @@ -1836,6 +1836,45 @@ test("a datagram below the group floor does not keep a finite track's copy", asy a.close(); }); +test("a clean end keeps the serving copy while a group read from a replaced one may continue from it", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const b = new BroadcastProducer(); + const ta = copy(a); + const tb = copy(b); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe({ maxDelay: Milli(3_600_000) }); + try { + ta.write(0); + expect(drain(sub)).toEqual([0]); + front.serve(b.consume()); + await settle(); + + // Group 1 is handed out from the old route, its first frame still unread. + const stalled = ta.write(1, 1, false); + const group = await sub.recvGroup(); + if (group?.sequence !== 1) throw new Error("expected group 1"); + + // The new route has all of group 1 and ends there. + tb.write(1, 2); + tb.producer.close(); + await settle(); + expect(sub.closed.peek()).toBeNull(); + expect(dec.decode((await group.readFrame())?.payload)).toBe("1.0"); + + // The old route fails: the group continues from the copy that ended. + stalled.close(new StreamError(StreamCode.SessionClosed)); + expect(dec.decode((await group.readFrame())?.payload)).toBe("1.1"); + expect(await group.readFrame()).toBeUndefined(); + group.close(); + } finally { + sub.close(); + front.consumer.close(); + a.close(); + b.close(); + } +}); + test("a retained copy declaring another end after a clean end fails the reads", async () => { const front = new Front(); const a = new BroadcastProducer(); diff --git a/js/net/src/resume.ts b/js/net/src/resume.ts index ab171ae211..cc8c99f408 100644 --- a/js/net/src/resume.ts +++ b/js/net/src/resume.ts @@ -639,6 +639,8 @@ class Reader implements ResumeSource { if (!this.#ended || copy.lease > 0) return true; const start = Math.max(cursor.start, copy.floor ?? 0); if (!hooks.spent(copy.sub, start, undefined, delivered)) return true; + // A group read from a copy it replaced may still continue from it. + if (this.#backs(copy, true, grace(this.#budget()))) return true; } else { const retains = this.#retains(copy, delivered, edge); if (retains === "wait") waits = true; @@ -679,19 +681,25 @@ class Reader implements ResumeSource { if (hooks.hasUnreadGroup(copy.sub, cursor.start, end, delivered)) return "keep"; } if (copy.lease > 0) return "keep"; - // A group still read that it holds or may yet get within the grace a late group is waited - // for: a failure of the copy that group is read from continues here. const lands = copy.since !== undefined && !this.#expired(copy, wait); + return this.#backs(copy, lands, wait) ? "keep" : undefined; + } + + /** + * Whether `copy` holds a group still read, or may yet get it within `wait` when it `lands`: a + * failure of the copy that group is read from continues here. + */ + #backs(copy: Copy, lands: boolean, wait: Milli): boolean { for (const recover of this.out) { const sequence = recover.sequence; - if (hooks.holdsGroup(copy.sub, sequence)) return "keep"; + if (hooks.holdsGroup(copy.sub, sequence)) return true; // A group at or past where it was asked to start may land even before its first // arrival; without a floor, only what follows its first arrival can. const asked = copy.floor !== undefined && copy.floor <= sequence; const land = { start: sequence, end: sequence + 1, floor: asked, grace: wait }; - if (lands && hooks.mayLand(copy.sub, land)) return "keep"; + if (lands && hooks.mayLand(copy.sub, land)) return true; } - return undefined; + return false; } /** From 1708a99e64f402ced9da71370e9e5f0749d14c8f Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 11:44:22 -0700 Subject: [PATCH 5/7] fix(net): a mismatched end releases the reader, and a dropped copy's end is kept A copy declaring another end now closes the reader with Incompatible, dropping its copies and leaving the route, before a clean end as well as after. Ends are learned before letting go of copies, so a copy dropped before its finished() callback still binds later copies. Co-Authored-By: Claude Opus 5.5 --- js/net/src/resume.test.ts | 29 +++++++++++++++++++++++++++++ js/net/src/resume.ts | 13 ++++++++----- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/js/net/src/resume.test.ts b/js/net/src/resume.test.ts index 5292bcad7b..b40255b0ae 100644 --- a/js/net/src/resume.test.ts +++ b/js/net/src/resume.test.ts @@ -1697,6 +1697,35 @@ test("an end a replaced copy declared holds across a swap", async () => { await settle(); expect(sub.closed.peek()).toBeInstanceOf(Error); expect(sub.end()).toEqual({ group: 2, frame: 0 }); + // Refused, the reader holds no copy. + expect(ta.producer.demand().wanted.peek()).toBeUndefined(); + expect(tb.producer.demand().wanted.peek()).toBeUndefined(); + + sub.close(); + front.consumer.close(); + a.close(); + b.close(); +}); + +test("an end a replaced copy declared holds once the copy is let go in the same turn", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const b = new BroadcastProducer(); + const ta = copy(a); + const tb = copy(b); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + front.serve(b.consume()); + await settle(); + + // A datagram read lets go of the replaced copy before its end is reported. + ta.producer.finishAt({ group: 2, frame: 0 }); + expect(hooks.tryRecvDatagram(sub).kind).toBe("idle"); + expect(sub.end()).toEqual({ group: 2, frame: 0 }); + + tb.producer.finishAt({ group: 3, frame: 0 }); + await settle(); + expect(sub.closed.peek()).toBeInstanceOf(Error); sub.close(); front.consumer.close(); diff --git a/js/net/src/resume.ts b/js/net/src/resume.ts index cc8c99f408..8e889eef7b 100644 --- a/js/net/src/resume.ts +++ b/js/net/src/resume.ts @@ -275,7 +275,7 @@ class Reader implements ResumeSource { #changed = new Signal(0); #dispose: Dispose[] = []; #closed = false; - /** A copy declared another end after the track ended cleanly: reads fail with this. */ + /** A copy declared another end: reads fail with this. */ #refused?: Error; /** The logical track ended cleanly: the reader drains what its copies hold and follows no route. */ #ended = false; @@ -630,6 +630,9 @@ class Reader implements ResumeSource { * a replaced one, which a clean end waits for. */ #letGo(delivered: Delivered): boolean { + // An end a copy declared outlives the copy. + this.#learnEnd(); + if (this.#closed) return false; const last = this.#copies.at(-1); const edge = last && hooks.latestSequence(last.sub); const cursor = this.#shell.cursor.peek(); @@ -847,12 +850,12 @@ class Reader implements ResumeSource { return; } else if (finalSequence(declared) === finalSequence(end) && (declared.frame === 0 || end.frame === 0)) { if (declared.frame === 0) this.#declared = end; - } else if (this.#ended) { - // `closed` already settled cleanly: stop reading the copies that claim other content. + } else { + // Stop reading the copies that claim other content. `closed` settles with the refusal, + // unless it already settled cleanly. this.#refused = new Incompatible(this.route.name); + this.#shell.close(this.#refused); this.close(this.#refused); - } else { - this.#shell.close(new Incompatible(this.route.name)); } } From 74550db97807b8c1237634e0900c9c076ecddb08 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 21 Oct 2026 22:49:03 -0700 Subject: [PATCH 6/7] fix(net): a resumed reader backs groups handed out before a swap, and refuses data past its end Every group handed out is recorded for the copies that may back it, not only those handed out while copies overlap: a later route's copy that ends cleanly now stays while it holds a group read from the copy it replaced. The declared end is checked against every group and datagram any copy holds or the reader handed out, before each read: a copy declaring an end below them, or holding data past it, claims other content and is refused with Incompatible, as a direct track refuses an end below what it received. Co-Authored-By: Claude Opus 5.5 --- js/net/src/resume.test.ts | 95 ++++++++++++++++++++++++++++++++++++++- js/net/src/resume.ts | 59 ++++++++++++++++-------- 2 files changed, 134 insertions(+), 20 deletions(-) diff --git a/js/net/src/resume.test.ts b/js/net/src/resume.test.ts index 3a2bb54c20..8573f514ac 100644 --- a/js/net/src/resume.test.ts +++ b/js/net/src/resume.test.ts @@ -1553,8 +1553,8 @@ test.each([true, false])("a replaced copy backs a group before its first arrival (await next())?.close(); front.serve(b.consume()); await settle(); - // Closed once replaced: the serving copy closing cleanly would end the track. - ta.producer.close(); + // Its route fails once replaced: closing cleanly would declare the track ends at group 1. + ta.producer.close(new StreamError(StreamCode.SessionClosed)); front.serve(c.consume()); await settle(); @@ -1997,3 +1997,94 @@ test("a retained copy declaring another end after a clean end fails the reads", b.close(); } }); + +test("a clean end keeps the serving copy while a group handed out before the swap may continue from it", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const b = new BroadcastProducer(); + const ta = copy(a); + const tb = copy(b); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe({ maxDelay: Milli(3_600_000) }); + try { + // Group 0 is handed out while the old route is the only one. + const stalled = ta.write(0, 1, false); + const group = await sub.recvGroup(); + if (group?.sequence !== 0) throw new Error("expected group 0"); + expect(dec.decode((await group.readFrame())?.payload)).toBe("0.0"); + + // The new route has all of group 0 and ends there. + front.serve(b.consume()); + await settle(); + tb.write(0, 2); + tb.producer.close(); + await settle(); + expect(sub.closed.peek()).toBeNull(); + + // The old route fails: the group continues from the copy that ended. + stalled.close(new StreamError(StreamCode.SessionClosed)); + expect(dec.decode((await group.readFrame())?.payload)).toBe("0.1"); + expect(await group.readFrame()).toBeUndefined(); + group.close(); + } finally { + sub.close(); + front.consumer.close(); + a.close(); + b.close(); + } +}); + +test.each(["handed out", "buffered"])("an end below a group another copy %s is refused", async (seen) => { + const front = new Front(); + const a = new BroadcastProducer(); + const b = new BroadcastProducer(); + const ta = copy(a); + const tb = copy(b); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + try { + ta.write(5); + if (seen === "handed out") expect(drain(sub)).toEqual([5]); + front.serve(b.consume()); + await settle(); + + // The new route ends the track before a group the old one has. + tb.producer.finishAt({ group: 3, frame: 0 }); + await settle(); + expect(() => sub.tryRecvGroup()).toThrow("changed its properties"); + expect(sub.closed.peek()).toBeInstanceOf(Error); + } finally { + sub.close(); + front.consumer.close(); + a.close(); + b.close(); + } +}); + +test("a group past the declared end on another copy is refused", async () => { + const front = new Front(); + const a = new BroadcastProducer(); + const b = new BroadcastProducer(); + const ta = copy(a); + const tb = copy(b); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + try { + ta.write(0); + ta.producer.finishAt({ group: 2, frame: 0 }); + expect(drain(sub)).toEqual([0]); + front.serve(b.consume()); + await settle(); + + // The new route has a group the end says never exists. + tb.write(3); + await settle(); + expect(() => sub.tryRecvGroup()).toThrow("changed its properties"); + expect(sub.closed.peek()).toBeInstanceOf(Error); + } finally { + sub.close(); + front.consumer.close(); + a.close(); + b.close(); + } +}); diff --git a/js/net/src/resume.ts b/js/net/src/resume.ts index 73366d696b..7bff8108de 100644 --- a/js/net/src/resume.ts +++ b/js/net/src/resume.ts @@ -29,7 +29,8 @@ * The logical track ends cleanly once its serving copy does and no group the reader lacks may * still land below that end on a copy it replaced. Its `closed` settles and the reader leaves the * route, keeping each copy only while it holds something left to read, as a direct reader keeps - * its buffer. Its end is the first one any copy declared, which every later copy has to match. + * its buffer. Its end is the first one any copy declared, which every later copy has to match, and + * no copy may hold a group or datagram at or past it. * * @module */ @@ -287,7 +288,7 @@ class Reader implements ResumeSource { #delivered = new Delivered(); /** Whether groups were ever asked for; until then the reader keeps no replaced copy. */ #groupsRead = false; - /** The groups handed out while copies overlap and still read; see {@link #retains}. */ + /** The groups handed out and still read, which other copies may back; see {@link #backs}. */ readonly out = new Set(); /** Every group handed out and still open, which ends with the reader. */ #open = new Set(); @@ -371,7 +372,9 @@ class Reader implements ResumeSource { copy.sub.finished().then( // A copy already let go says nothing more about the track. (end) => { - if (this.#copies.includes(copy)) this.#declare(end); + if (!this.#copies.includes(copy)) return; + this.#declare(end); + this.#learnEnd(); }, () => {}, ); @@ -472,9 +475,11 @@ class Reader implements ResumeSource { } tryGroup(ordered: boolean, delivered: Delivered): Recv { - if (this.#closed) return this.#refused ? { kind: "error", error: this.#refused } : { kind: "done" }; this.#delivered = delivered; this.sync(); + // Before anything is handed out, so a copy claiming other content hands out none of it. + this.#learnEnd(); + if (this.#closed) return this.#refused ? { kind: "error", error: this.#refused } : { kind: "done" }; this.#groupsRead = true; // The lowest group any copy holds next, in either order: a replaced copy's buffered group // comes before a later one the serving copy already has. Arrival order still delivers a @@ -542,9 +547,10 @@ class Reader implements ResumeSource { } tryDatagram(delivered: Delivered): RecvDatagram { - if (this.#closed) return this.#refused ? { kind: "error", error: this.#refused } : { kind: "done" }; this.#delivered = delivered; this.sync(); + this.#learnEnd(); + if (this.#closed) return this.#refused ? { kind: "error", error: this.#refused } : { kind: "done" }; for (let index = this.#copies.length - 1; index >= 0; index--) { const copy = this.#copies[index]; // A replaced copy's datagrams are the serving one's to deliver. @@ -767,9 +773,9 @@ class Reader implements ResumeSource { #handOut(copy: Copy, group: GroupConsumer): GroupConsumer { this.#newest = Math.max(this.#newest ?? 0, group.sequence); // Another copy holding the group, now or once it lands there, is where a failure continues - // it, so it stays while the group is read; see #retains. Only while copies overlap: a - // passthrough read has none. - const recover = new Recover(this, group, copy, this.#copies.length > 1); + // it, so it stays while the group is read; see #backs. Recorded even with one copy, since a + // later route's copy may come to hold it. + const recover = new Recover(this, group, copy); hooks.recoverGroup(group, recover); recover.watch(group); // Closed with the reader, as a direct reader's close reaches the groups it handed out. @@ -828,12 +834,25 @@ class Reader implements ResumeSource { return last?.generation === this.#generation ? last : undefined; } - // The logical track's end, learning it from any copy that declared one. + // The logical track's end, learning it from any copy that declared one. A group or datagram + // at or past it, on any copy or already handed out, claims other content: a direct track + // refuses an end below what it received. #learnEnd(): track.Location | undefined { for (const copy of this.#copies) { const end = copy.sub.end(); if (end) this.#declare(end); } + const declared = this.#declared; + if (declared && !this.#closed) { + const final = finalSequence(declared); + const seen = (sequence: number | undefined) => sequence !== undefined && sequence >= final; + if ( + seen(this.#newest) || + seen(this.#datagram) || + this.#copies.some((copy) => seen(hooks.latestSequence(copy.sub))) + ) + this.#refuse(); + } return this.#declared; } @@ -851,14 +870,18 @@ class Reader implements ResumeSource { } else if (finalSequence(declared) === finalSequence(end) && (declared.frame === 0 || end.frame === 0)) { if (declared.frame === 0) this.#declared = end; } else { - // Stop reading the copies that claim other content. `closed` settles with the refusal, - // unless it already settled cleanly. - this.#refused = new Incompatible(this.route.name); - this.#shell.close(this.#refused); - this.close(this.#refused); + this.#refuse(); } } + // Stop reading the copies, which claim other content. `closed` settles with the refusal, + // unless it already settled cleanly. + #refuse(): void { + this.#refused = new Incompatible(this.route.name); + this.#shell.close(this.#refused); + this.close(this.#refused); + } + start(): number | "pending" | undefined { this.sync(); const serving = this.#serving(); @@ -1063,15 +1086,15 @@ class Recover implements GroupRecover { #copy?: Copy; /** - * `out` puts the group on the reader's list of those still read, so the other copies that - * hold it, or may yet get it, stay for a failure to continue from. + * Puts the group on the reader's list of those still read, so the other copies that hold it, + * or may yet get it, stay for a failure to continue from. */ - constructor(reader: Reader, group: GroupConsumer, copy: Copy, out: boolean) { + constructor(reader: Reader, group: GroupConsumer, copy: Copy) { this.#reader = reader; this.sequence = group.sequence; this.#copy = copy; copy.lease++; - if (out) reader.out.add(this); + reader.out.add(this); } /** From 76b12239fee2ee5445a0159a4c7cc8aea56f5bc6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 22 Oct 2026 14:19:05 -0700 Subject: [PATCH 7/7] fix(net): refuse a resumed end below a datagram another copy holds The declared end was checked against every copy's latest group, which counts no datagrams, so a replaced copy's buffered datagram past the end let a clean end through. Check each copy's largest too, which does. Co-Authored-By: Claude Opus 5.5 --- js/net/src/resume.test.ts | 28 ++++++++++++++++++++++++++++ js/net/src/resume.ts | 5 ++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/js/net/src/resume.test.ts b/js/net/src/resume.test.ts index 1f32235400..c366ce3193 100644 --- a/js/net/src/resume.test.ts +++ b/js/net/src/resume.test.ts @@ -2148,6 +2148,34 @@ test.each(["handed out", "buffered"])("an end below a group another copy %s is r } }); +test.each(["handed out", "buffered"])("an end below a datagram another copy %s is refused", async (seen) => { + const front = new Front(); + const a = new BroadcastProducer(); + const b = new BroadcastProducer(); + const ta = copy(a); + const tb = copy(b); + front.serve(a.consume()); + const sub = front.consumer.track("seq").subscribe(); + try { + ta.producer.insertDatagram(5, undefined, enc.encode("5")); + if (seen === "handed out") expect(hooks.tryRecvDatagram(sub).kind).toBe("datagram"); + front.serve(b.consume()); + await settle(); + // The new route delivers an older datagram last, then ends the track before the old route's. + tb.producer.insertDatagram(2, undefined, enc.encode("2")); + if (seen === "handed out") expect(hooks.tryRecvDatagram(sub).kind).toBe("datagram"); + tb.producer.finishAt({ group: 3, frame: 0 }); + await settle(); + expect(hooks.tryRecvDatagram(sub).kind).toBe("error"); + expect(sub.closed.peek()).toBeInstanceOf(Error); + } finally { + sub.close(); + front.consumer.close(); + a.close(); + b.close(); + } +}); + test("a group past the declared end on another copy is refused", async () => { const front = new Front(); const a = new BroadcastProducer(); diff --git a/js/net/src/resume.ts b/js/net/src/resume.ts index 0220b84ae2..01e67a49ca 100644 --- a/js/net/src/resume.ts +++ b/js/net/src/resume.ts @@ -870,7 +870,10 @@ class Reader implements ResumeSource { if ( seen(this.#newest) || seen(this.#newestDatagram) || - this.#copies.some((copy) => seen(hooks.latestSequence(copy.sub))) + // The largest counts datagrams too, which the latest group does not. + this.#copies.some( + (copy) => seen(hooks.latestSequence(copy.sub)) || seen(hooks.peekLargest(copy.sub).largest?.group), + ) ) this.#refuse(); }