diff --git a/js/net/src/group.ts b/js/net/src/group.ts index 4612ae1005..4bb02d5c0a 100644 --- a/js/net/src/group.ts +++ b/js/net/src/group.ts @@ -948,6 +948,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 0c5764c540..2bef0d5825 100644 --- a/js/net/src/internal.ts +++ b/js/net/src/internal.ts @@ -351,7 +351,7 @@ export interface ResumeSource { latest(): number | undefined; /** The serving copy's declared start, raised to its cursor floor, or `"pending"` until resolved. */ start(): number | "pending" | 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; @@ -469,6 +469,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 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. */ deliveredRuns: (subscriber: Subscriber) => number; /** Forget the gaps in another reader's `delivered` below each group past this subscriber's budget. */ @@ -642,6 +647,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 71f27a1753..c366ce3193 100644 --- a/js/net/src/resume.test.ts +++ b/js/net/src/resume.test.ts @@ -1638,9 +1638,10 @@ test.each([true, false])("a replaced copy backs a group before its first arrival const next = () => (view ? view.nextGroup() : sub.recvGroup()); ta.write(0); (await next())?.close(); - ta.producer.close(); front.serve(b.consume()); await settle(); + // 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(); @@ -1756,6 +1757,453 @@ test.each([Milli(100), Milli(3_600_000)])( }, ); +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 }); + // 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(); + 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(); +}); + +test.each(["lands", "expired"])( + "the serving copy closing cleanly settles closed once a group the replaced copy may carry is in (%s)", + async (outcome) => { + const { clock, restore } = manual(); + 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 has only the group already handed out, and ends there. Nothing reads. + tb.write(2); + tb.producer.close(); + front.serve(b.consume()); + await settle(); + expect(sub.closed.peek()).toBeUndefined(); + + if (outcome === "lands") ta.write(1); + else await clock.advance(Nano.fromMilli(SECOND)); + await settle(); + // Settled like a direct reader's, with the group that landed still readable. + expect(sub.closed.peek()).toBeNull(); + expect(drain(sub)).toEqual(outcome === "lands" ? [1] : []); + expect(await sub.recvGroup()).toBeUndefined(); + await settle(); + expect(ta.producer.demand().wanted.peek()).toBeUndefined(); + } finally { + sub.close(); + front.consumer.close(); + a.close(); + b.close(); + restore(); + } + }, +); + +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 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(); + 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(); + } +}); + +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.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(); + 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(); + } +}); + test.each([ ["fails", (answer: Lister) => answer.abort(new Error("old route failed"))], ["ends", (answer: Lister) => answer.finish()], diff --git a/js/net/src/resume.ts b/js/net/src/resume.ts index 19ee4167c5..01e67a49ca 100644 --- a/js/net/src/resume.ts +++ b/js/net/src/resume.ts @@ -14,10 +14,8 @@ * route got after the switch is not lost. It is kept while it may still deliver a group under its * bound the reader lacks (unread, or still landing within the grace a passed group is waited for), * and while a group still read is read from it or may continue from it (it holds that group, or may - * yet get it). Nothing under the bound is waited for past the reader's budget. The serving copy - * draining cleanly ends the track only once nothing it replaced may still deliver below that end. - * Datagrams are ephemeral, so no copy is kept for them: a reader that never asked for a group keeps - * none. + * yet get it). Nothing under the bound is waited for past the reader's budget. Datagrams are + * ephemeral, so no copy is kept for them: a reader that never asked for a group keeps none. * * A group carries on across the switch through its {@link Recover}: once its copy stalls while a * newer route serves, it continues from the serving route's copy of the same group at the same @@ -28,12 +26,18 @@ * 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 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, and + * no copy may hold a group or datagram at or past it. + * * @module */ import { Instant, Milli, now, sleepUntil } from "@moq/clock"; 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, SessionError, StreamCode, StreamError, sessionClosed } from "./error.ts"; import type { Consumer as GroupConsumer } from "./group.ts"; import { @@ -273,16 +277,28 @@ class Reader implements ResumeSource { * budget: a publisher cannot grow it. */ #datagrams = new Map(); + /** The newest datagram handed out, which a declared end must stay past. */ + #newestDatagram?: number; /** Bumped when this reader subscribes a new copy or a copy's last lease ends, waking its parked reads. */ #changed = new Signal(0); #dispose: Dispose[] = []; #closed = false; - /** The channels that ran out cleanly. */ - #drained = new Set<"groups" | "datagrams">(); + /** 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; + /** 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 subscriber's record of groups handed out, as the last read passed it. */ + #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(); /** The soonest deadline a read waits on, armed to wake it then. */ #wall?: { at: number; timer: AbortController }; /** Records the properties a copy committed, and which source it came from. */ @@ -322,7 +338,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; @@ -359,6 +375,30 @@ 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)) return; + this.#declare(end); + this.#learnEnd(); + }, + () => {}, + ); + void copy.sub.closed.then(async (closed) => { + // The serving copy closing cleanly is the logical track's end, once nothing it replaced + // may still deliver below it. + if (closed !== null) return; + for (;;) { + this.sync(); + if (this.#closed || this.#serving() !== copy || this.#conclude()) return; + await new Promise((resolve) => { + const dispose = this.changed(() => { + dispose(); + resolve(); + }); + }); + } + }); this.#changed.update((n) => n + 1); } @@ -441,7 +481,11 @@ class Reader implements ResumeSource { } tryGroup(ordered: boolean, delivered: Delivered): Recv { + 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 @@ -475,11 +519,8 @@ class Reader implements ResumeSource { break; } } - const drained = this.#boundAtEnd(); - // The serving copy drained, but a copy it replaced may still deliver a group below its - // end: the track ends once that copy has. - if (this.#letGo(delivered) && drained) return { kind: "idle" }; - return this.#end("groups", (copy) => copy.done); + this.#letGo(delivered); + return this.#end((copy) => copy.done); } // Whether `copy` may still feed groups: its channel has not run out, and what it holds is @@ -512,7 +553,10 @@ class Reader implements ResumeSource { } tryDatagram(delivered: Delivered): RecvDatagram { + this.#delivered = delivered; this.sync(); + this.#learnEnd(); + if (this.#closed) return this.#refused ? { kind: "error", error: this.#refused } : { kind: "done" }; // Like groups, the last delivery stays fresh; earlier identities cost memory only // while the reader's budget still covers their successor's arrival. const at = now(); @@ -537,6 +581,7 @@ class Reader implements ResumeSource { this.#datagrams.set(this.#datagram, at); } this.#datagram = sequence; + this.#newestDatagram = Math.max(this.#newestDatagram ?? 0, sequence); return recv; } if (recv.kind === "done") copy.datagramsDone = copy.sub.closed.peek() ?? null; @@ -545,7 +590,7 @@ class Reader implements ResumeSource { } } this.#letGo(delivered); - const end = this.#end("datagrams", (copy) => copy.datagramsDone); + const end = this.#end((copy) => copy.datagramsDone); if (end.kind === "error" || end.kind === "done") return end; return { kind: "idle" }; } @@ -554,58 +599,83 @@ 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.#conclude() ? { kind: "done" } : { kind: "idle" }; 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.#fail(ended); + if (route.refusal) return this.#fail(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.#fail(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.#fail(dropped()); + if (ended instanceof Error) return this.#fail(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(); - return end; + #fail(error: Error): Recv & { kind: "error" } { + this.#shell.close(error); + return { kind: "error", error }; + } + + /** + * End the logical track cleanly, once its serving copy did and nothing it replaced may still + * deliver a group below that end: settle `closed` and leave the route, keeping each copy only + * while it holds something left to read. The other channel's backlog stays readable. Whether + * the track ended. + */ + #conclude(): boolean { + if (this.#closed) return false; + if (this.#ended) return true; + // Learned before any copy that knows it goes. + this.#learnEnd(); + const serving = this.#serving(); + if (serving) this.#boundAtEnd(serving); + if (this.#letGo(this.#delivered) || this.#closed) return false; + this.#ended = true; + this.#last = { largest: serving && hooks.peekLargest(serving.sub).largest, ended: true }; + this.#shell.close(); + this.route.leave(this); + this.#letGo(this.#delivered); + this.wake(); + return true; } /** - * Drop the replaced copies nothing needs any more; see {@link #retains}. One not yet replaced - * stays, even between routes: it is still the newest copy there is. Whether one may still - * deliver a group the reader lacks, which a clean end waits for. + * Drop the copies nothing needs any more. A replaced one goes by {@link #retains}. One not 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. Whether a group the reader lacks may still land on + * 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(); let waits = false; this.#copies = this.#copies.filter((copy) => { - if (!copy.replaced) return true; - const retains = this.#retains(copy, delivered, edge); - if (retains === "wait") waits = true; - if (retains) return true; + if (!copy.replaced) { + 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; + if (retains) return true; + } this.#drop(copy); return false; }); @@ -613,9 +683,9 @@ class Reader implements ResumeSource { } /** - * How the reader still needs replaced `copy`: `"wait"` while it may deliver a group the reader - * lacks, `"keep"` while it backs a group handed out. `edge` is the newest group the serving - * copy has. + * How the reader still needs replaced `copy`: `"wait"` while a group the reader lacks may still + * land on it, `"keep"` while it holds one unread or backs a group handed out. `edge` is the + * newest group the serving copy has. */ #retains(copy: Copy, delivered: Delivered, edge: number | undefined): "wait" | "keep" | undefined { // Datagrams are ephemeral, so only groups keep a copy: a reader that only reads datagrams @@ -638,32 +708,36 @@ class Reader implements ResumeSource { // A group still to land is waited for once something superseded it, as a gap is. const land = { start, end, floor, grace: wait, delivered }; if (copy.since !== undefined && hooks.mayLand(copy.sub, land)) return "wait"; - if (hooks.hasUnreadGroup(copy.sub, cursor.start, end, delivered)) return "wait"; + 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; } /** - * Whether the serving copy drained cleanly. Bounds every replaced copy not yet bounded at that - * end, so a group it still carries below the end is waited for and nothing past it. + * Bound every replaced copy not yet bounded at the end of `serving`, which ended cleanly, so a + * group it still carries below the end is waited for and nothing past it. */ - #boundAtEnd(): boolean { - const serving = this.#serving(); - if (!serving || serving.done !== null) return false; - const declared = serving.sub.end(); + #boundAtEnd(serving: Copy): void { + const declared = this.#declared; const until = declared !== undefined ? finalSequence(declared) - 1 @@ -675,7 +749,6 @@ class Reader implements ResumeSource { this.#update(copy); this.#bound(copy); } - return true; } /** The reader's budget: how far behind a group may fall before it is skipped. */ @@ -713,22 +786,38 @@ class Reader implements ResumeSource { ); } - #drop(copy: Copy): void { - copy.sub.close(); + #drop(copy: Copy, abort?: Error): void { + copy.sub.close(abort); copy.source.release(); } #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. + this.#open.add(group); + void group.closed.then(() => this.#open.delete(group)); return group; } + /** The logical track ended cleanly, so no route continues it. */ + get ended(): boolean { + return this.#ended; + } + + /** A group handed out ended or moved copies: let go of what nothing needs any more. */ + freed(): void { + // Once the track ended, no read may come to let go of a copy. + if (this.#ended) this.#letGo(this.#delivered); + // 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[] = []; @@ -766,6 +855,57 @@ class Reader implements ResumeSource { return last?.generation === this.#generation ? last : undefined; } + // 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.#newestDatagram) || + // 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(); + } + 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.#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(); @@ -774,15 +914,15 @@ class Reader implements ResumeSource { 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. @@ -813,14 +953,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. @@ -831,8 +971,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[] = []; @@ -862,16 +1004,21 @@ class Reader implements ResumeSource { ); } - close(): void { + close(abort?: Error): void { if (this.#closed) return; this.#closed = true; this.#wall?.timer.abort(); this.#wall = undefined; 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.#open) group.close(abort); + this.#open.clear(); + for (const copy of this.#copies) this.#drop(copy, abort); this.#copies = []; this.route.leave(this); + // A read still waiting on the reader looks again and finds it closed. + this.wake(); } } @@ -963,15 +1110,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); } /** @@ -986,9 +1133,8 @@ class Recover implements GroupRecover { #release(wake = false): void { const copy = this.#copy; this.#copy = undefined; - const freed = copy !== undefined && --copy.lease === 0 && copy.replaced; - // A parked read lets go of a replaced copy nothing needs any more. - if (wake || freed) this.#reader.wake(); + const freed = copy !== undefined && --copy.lease === 0; + if (wake || freed) this.#reader.freed(); } get changed(): Getter { @@ -1056,6 +1202,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; // A verdict would repeat from the route that gave it. if (generation === current.generation && !this.#undelivered(failed, current)) throw failed; } @@ -1102,10 +1250,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 589fd5c1de..4c6e06c986 100644 --- a/js/net/src/track.ts +++ b/js/net/src/track.ts @@ -2026,6 +2026,23 @@ export class Subscriber { !delivered.has(group.sequence) && !(group.closed.peek() instanceof Error), ); + 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()) && + !readable && + !hooks.hasUnreadGroup(subscriber, start, end, delivered) + ); + }; hooks.staleAt = (subscriber, sequence) => ({ expired: () => subscriber.#staleAt(sequence, subscriber.#drift(), true), changed: subscriber.#expiryChanged(), @@ -2427,9 +2444,9 @@ export class Subscriber { // A group from the routes' copies; see #resume. #tryResumed(ordered: boolean): Recv { const resume = this.#resume as ResumeSource; + // 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; @@ -2529,10 +2546,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 2c2775e56e..95b59adc69 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -123,7 +123,6 @@ tracks, so per-group stats stop costing cluster announces. - [JS release preflight](/quest/m1/js-publish-preflight.md) - `release.ts` refuses a package whose `@moq/*` dependency is not on npm, so no dependent publishes uninstallable - [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 -- [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 - [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 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. diff --git a/quest/m1/resume-handed-out.md b/quest/m1/resume-handed-out.md index 8810f407cb..48ee6b87c0 100644 --- a/quest/m1/resume-handed-out.md +++ b/quest/m1/resume-handed-out.md @@ -34,7 +34,3 @@ 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 - -## Related - -- [Resumed close](/quest/m1/resume-closed.md) - carries the JS half of this fix diff --git a/quest/m1/resume-rust-end.md b/quest/m1/resume-rust-end.md index 679381b8dd..ad25800dc8 100644 --- a/quest/m1/resume-rust-end.md +++ b/quest/m1/resume-rust-end.md @@ -29,11 +29,7 @@ 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: #6136, then #6224 -(#6209 and #6251 have landed). +JS's `Incompatible` (`js/net/src/resume.ts`). Merge order: #6224 next +(#6136, #6209 and #6251 have landed). Public API: behavior only. Wire: none. - -## Related - -- [Resumed close](/quest/m1/resume-closed.md) - the JS half of a declared end across copies