diff --git a/js/net/src/answering.test.ts b/js/net/src/answering.test.ts new file mode 100644 index 0000000000..d567aec2c9 --- /dev/null +++ b/js/net/src/answering.test.ts @@ -0,0 +1,88 @@ +import { afterEach, beforeEach, expect, jest, test } from "bun:test"; +import { Broadcast, Origin, Path } from "./index.ts"; + +beforeEach(() => jest.useFakeTimers()); +afterEach(() => jest.useRealTimers()); + +async function settle() { + for (let i = 0; i < 64; i++) await Promise.resolve(); +} + +test("public track answering is armed before demand and stops without closing accepted tracks", async () => { + const source = new Broadcast.Producer(); + const answering = source.dynamic(); + const consumer = source.consume(); + const subscribed = consumer.track("video").subscribe(); + const request = await answering.requestedTrack(); + expect(request?.name).toBe("video"); + const track = request?.accept(); + if (!track) throw new Error("expected track"); + const pending = answering.requestedTrack(); + answering.close(); + expect(await pending).toBeUndefined(); + expect(track.closed.peek()).toBeUndefined(); + const absent = consumer.track("other").subscribe(); + expect(absent.closed.peek()).toBeInstanceOf(Error); + absent.close(); + subscribed.close(); + consumer.close(); + source.close(); +}); + +test("a fallback is scoped, never advertised, and its answered requests end with it", async () => { + const origin = new Origin.Producer(); + const allowed = new Path.Patterns([Path.Pattern.subtree(Path.from("room"))]); + const fallback = origin.scope(Path.empty(), allowed).fallback(); + const iterator = fallback.requested(); + const unseen = origin.request(Path.from("elsewhere")); + const wanted = origin.request(Path.from("room/alice")); + const next = await iterator.next(); + expect(next.value?.path).toBe(Path.from("room/alice")); + const source = new Broadcast.Producer(); + next.value?.accept(source); + await settle(); + expect(wanted.active.peek()).toBeDefined(); + expect(unseen.active.peek()).toBeUndefined(); + const announcements = origin.announced(); + let advertised = false; + const event = announcements.next().then((value) => { + advertised = value !== undefined; + }); + await settle(); + expect(advertised).toBe(false); + fallback.close(); + await settle(); + expect(wanted.closed.peek()).toBeInstanceOf(Error); + wanted.close(); + unseen.close(); + announcements.close(); + await event; + source.close(); + origin.close(); + await iterator.return?.(undefined); +}); + +test("an announced route retires a blind answer rather than stitching its request", async () => { + const origin = new Origin.Producer(); + const fallback = origin.fallback(); + const iterator = fallback.requested(); + const wanted = origin.request(Path.from("room")); + const next = await iterator.next(); + const blind = new Broadcast.Producer(); + next.value?.accept(blind); + await settle(); + expect(wanted.active.peek()).toBeDefined(); + const announced = origin.createBroadcast(Path.from("room")); + announced.announce(); + await settle(); + expect(wanted.closed.peek()).toBeInstanceOf(Error); + const fresh = origin.request(Path.from("room")); + expect(fresh.active.peek()).toBeDefined(); + fresh.close(); + wanted.close(); + fallback.close(); + announced.close(); + blind.close(); + origin.close(); + await iterator.return?.(undefined); +}); diff --git a/js/net/src/broadcast.ts b/js/net/src/broadcast.ts index 77a17e4cc2..a8d72cb3bc 100644 --- a/js/net/src/broadcast.ts +++ b/js/net/src/broadcast.ts @@ -63,6 +63,7 @@ class BroadcastState { // publisher that has started pulling them. The counterpart of a live Rust `broadcast::Dynamic`; // without one, a track nobody publishes is `NotFound` rather than a request nobody will answer. served = false; + handlers = 0; // Each name's sequence namespace, shared by every producer of the name, created or requested. sequences = new TrackSequences(); // Live consumer handles sharing this state (see {@link Consumer.clone}). The broadcast @@ -121,19 +122,20 @@ function dequeueRequest(state: BroadcastState): track.Request | undefined { } // The next on-demand track request, or undefined once the broadcast closes. -async function requested(state: BroadcastState): Promise { +async function requested(state: BroadcastState, stopped?: GetPromise): Promise { // Pulling requests is what makes a broadcast serve tracks on demand, and it latches for the // broadcast's lifetime since JS has no drop to mark the handler gone. A subscribe before the first // pull is answered NotFound, so an on-demand publisher starts pulling before it publishes and keeps // pulling until the broadcast closes. - state.served = true; + if (stopped === undefined) state.served = true; for (;;) { + if (stopped !== undefined && stopped.peek() !== undefined) return undefined; const request = dequeueRequest(state); if (request) return request; if (state.closed.peek() !== undefined) return undefined; - await Signal.race(state.requested, state.closed); + await Signal.race(state.requested, state.closed, ...(stopped !== undefined ? [stopped] : [])); } } @@ -349,6 +351,44 @@ export class Demand { } } +let makeDynamic: (state: BroadcastState) => Dynamic; + +/** Answer on-demand track requests and listings for a broadcast. */ +export class Dynamic { + #state: BroadcastState; + #stopped = new Once(); + + private constructor(state: BroadcastState) { + this.#state = state; + state.handlers++; + state.served = true; + } + + static { + makeDynamic = (state) => new Dynamic(state); + } + + /** Wait for the next requested track, or undefined once this handler or its broadcast closes. */ + requestedTrack(): Promise { + return requested(this.#state, this.#stopped); + } + + /** Answer the broadcast's track listings, refusing a second simultaneous lister. */ + lister(): Lister { + return new Lister(this.#state.listing); + } + + /** Stop handling new tracks, rejecting queued requests once no handler remains. */ + close(): void { + if (this.#stopped.peek() !== undefined) return; + this.#stopped.set(null); + if (--this.#state.handlers !== 0) return; + this.#state.served = false; + for (const request of this.#state.requested.peek()) request.reject(new NotFound("no track handler")); + this.#state.requested.set([]); + } +} + /** * The write side of a broadcast. * @@ -402,6 +442,11 @@ export class Producer { return this.#demand; } + /** Handle on-demand track requests before handing consumers the broadcast. */ + dynamic(): Dynamic { + return makeDynamic(this.#state); + } + /** A read handle for this broadcast, named by the path the origin created it at. */ consume(): Consumer { return makeConsumer({ state: this.#state, path: this.#path }); @@ -647,6 +692,14 @@ export class Consumer { }); } + #answering?: Dynamic; + + /** Give a custom session its source's on-demand track requests and listings. */ + protected dynamic(): Dynamic { + this.#answering ??= makeDynamic(this.#state); + return this.#answering; + } + static { makeConsumer = (shared) => new Consumer(shared as never); hooks.stampPath = (target, path) => { diff --git a/js/net/src/connection/forward.ts b/js/net/src/connection/forward.ts index 684482c797..6d09557c1f 100644 --- a/js/net/src/connection/forward.ts +++ b/js/net/src/connection/forward.ts @@ -5,11 +5,10 @@ */ import { Milli, Nano } from "@moq/clock"; -import { type Dispose, race } from "@moq/signals"; import type * as Announce from "../announced.ts"; import { type Route, routesEqual } from "../hop.ts"; import { coveringPrefixes } from "../internal.ts"; -import type { Dynamic, Producer as OriginProducer, RequestSlot } from "../origin.ts"; +import type { Dynamic, Producer as OriginProducer } from "../origin.ts"; import * as Path from "../path.ts"; import { after } from "../util/timer.ts"; import { wireOf } from "../wire.ts"; @@ -295,49 +294,12 @@ async function drive(handle: Dynamic, conn: Established): Promise { * any blind answer, so answering one would only park a handle nothing reads. */ async function serveRequests(conn: Established, origin: OriginProducer): Promise { - const table = wireOf(origin); + const fallback = origin.fallback(); const session = wireOf(conn); - // The withdraws for the answers this session provided, so a dead session only takes - // back its own. Keyed by path but remembering the slot, because a path outlives its - // slot: the last handle closing tears the slot down and a new request installs a fresh - // one, and those two writes coalesce into a single wakeup. Matching on the path alone - // would read the new slot as already answered and leave it unanswered forever. - const answered = new Map(); - - let dead = false; - const closed = conn.closed.then(() => { - dead = true; - }); - - for (;;) { - const map = table.requests.peek(); - if (!map || dead) break; - - for (const [path, slot] of map) { - if (!table.blind(slot)) continue; - if (answered.get(path)?.slot === slot || slot.answer !== undefined) continue; - if (table.routes(path)) continue; - const withdraw = table.answer(path, session.consume(path)); - if (withdraw) answered.set(path, { slot, withdraw }); - } - - // A withdrawn request already released the answer; just forget our claim on the path. - // A replaced slot counts as withdrawn: the answer we hold belongs to the slot that - // went away, not to whatever now occupies the path. - for (const [path, entry] of [...answered]) { - if (map.get(path) === entry.slot && table.blind(entry.slot)) continue; - answered.delete(path); - entry.withdraw(); - } - - // Woken by the table too, not just the requests: a path that stops being routed needs - // the blind answer this loop skipped while it was. - await race([table.changed(), closed]); - } - - // Session gone: withdraw our answers, waking a standby session to provide fresh ones. - for (const { withdraw } of answered.values()) { - withdraw(); + void conn.closed.then(() => fallback.close()); + try { + for await (const request of fallback.requested()) request.accept(session.consume(request.path)); + } finally { + fallback.close(); } - answered.clear(); } diff --git a/js/net/src/goaway-requests.test.ts b/js/net/src/goaway-requests.test.ts index a3501f392a..c7a0440bc8 100644 --- a/js/net/src/goaway-requests.test.ts +++ b/js/net/src/goaway-requests.test.ts @@ -321,13 +321,13 @@ async function routeDeath(kind: "lite" | "ietf", after: boolean): Promise ]); const watching = consume.announced(); + expect((await watching.next())?.kind).toBe("start"); const request = consume.request(Path.from("room")); let routes: ReturnType | undefined; let replacement: Awaited> | undefined; let replacementServer: Awaited> | undefined; try { - expect((await watching.next())?.kind).toBe("start"); await waitUntil(() => request.active.peek() !== undefined); const front = request.active.peek(); if (!front) throw new Error("no route"); diff --git a/js/net/src/ietf/subscriber.ts b/js/net/src/ietf/subscriber.ts index ff46861e50..b1a7fa4e0d 100644 --- a/js/net/src/ietf/subscriber.ts +++ b/js/net/src/ietf/subscriber.ts @@ -39,7 +39,7 @@ import { untilAborted } from "../util/abort.ts"; import { Deadline, requestOptions } from "../util/deadline.ts"; import { abandoned as demandAbandoned, isAbandoned, unwanted } from "../util/demand.ts"; import { LIMITS, type Limits, SlotMap, Slots } from "../util/limits.ts"; -import { overrideBroadcastWire, wireOf } from "../wire.ts"; +import { overrideBroadcastWire } from "../wire.ts"; import type { Session } from "./adapter.ts"; import { DuplicateTrackAlias, RetiredTrackAlias, TrackAliases } from "./aliases.ts"; import * as Cluster from "./cluster.ts"; @@ -730,10 +730,10 @@ export class Subscriber { // moq-transport resolves metadata through SUBSCRIBE and has no one-shot group fetch here. const consumer = new ConsumeBroadcast(); // Registered before anyone can list the broadcast, so no reader syncs on an empty answer. - void this.#refuseListing(wireOf(consumer).lister()); + void this.#refuseListing(consumer.answering.lister()); void (async () => { for (;;) { - const request = await wireOf(consumer).requested(); + const request = await consumer.answering.requestedTrack(); if (!request) break; // Every request names the instance the consume resolved, so the peer refuses // rather than answers from another. @@ -1759,6 +1759,10 @@ export class Subscriber { * group fetch, so `track.Consumer.fetchGroup()` is rejected. */ class ConsumeBroadcast extends broadcast.Consumer { + get answering(): broadcast.Dynamic { + return this.dynamic(); + } + constructor(state?: never) { super(state); overrideBroadcastWire(this, { diff --git a/js/net/src/lite/subscriber.ts b/js/net/src/lite/subscriber.ts index 5e44ff3464..7e2a7f3ed4 100644 --- a/js/net/src/lite/subscriber.ts +++ b/js/net/src/lite/subscriber.ts @@ -36,7 +36,7 @@ import { Deadline, REQUEST_TIMEOUT_MS, requestOptions } from "../util/deadline.t import { abandoned, isAbandoned, unwanted } from "../util/demand.ts"; import { LIMITS, type Limits, SlotMap, Slots } from "../util/limits.ts"; import { Budget } from "../util/yield.ts"; -import { overrideBroadcastWire, wireOf } from "../wire.ts"; +import { overrideBroadcastWire } from "../wire.ts"; import { AnnounceHistory, AnnounceInit, @@ -648,11 +648,11 @@ export class Subscriber { }; const consumer = new ConsumeBroadcast(this, path, consumed); // Registered before anyone can list the broadcast, so no reader syncs without the peer. - void this.#runListing(path, consumed, wireOf(consumer).lister()); + void this.#runListing(path, consumed, consumer.answering.lister()); void (async () => { for (;;) { - const request = await wireOf(consumer).requested(); + const request = await consumer.answering.requestedTrack(); if (!request) break; void this.#runSubscribe(path, consumed, request); } @@ -1848,6 +1848,10 @@ async function untilAbandoned(group: netGroup.Producer, step: Promise): Pr * session. Live subscribes still flow through the inherited requested() queue. */ class ConsumeBroadcast extends broadcast.Consumer { + get answering(): broadcast.Dynamic { + return this.dynamic(); + } + #subscriber: Subscriber; #path: Path.Valid; #consumed: Consumed; diff --git a/js/net/src/origin.ts b/js/net/src/origin.ts index c42bbc876b..e7b977dc81 100644 --- a/js/net/src/origin.ts +++ b/js/net/src/origin.ts @@ -242,7 +242,6 @@ class CoveringRoot { } } -// pending: /quest/m1/public-model/answer.md /** * One requested path: the notify node for everything watching it. * @@ -1292,6 +1291,82 @@ export class Producer implements Table { this.#state.refresh(path); } + /** + * Answer unrouted requests in this handle's scope without advertising a route. + * + * The first answer wins. Closing the handle or routing the path ends its resolved + * requests with an error, since no epoch proves another answer serves the same + * publisher. A standby can answer only unresolved and new requests. + */ + fallback(): Pick { + const server = new ServeState(this.#scope.root, this.#scope.root, false); + const held = new Map(); + const release = this.#expect(); + const forget = (path: Path.Valid) => { + const entry = held.get(path); + if (!entry) return; + held.delete(path); + const pending = server.pending.get(path); + if (pending) server.forget(path, pending); + const front = server.served.get(path); + server.served.delete(path); + entry.withdraw?.(); + front?.close(); + }; + const close = () => { + if (server.closed.peek() !== undefined) return; + server.close(); + for (const path of held.keys()) forget(path); + release(); + }; + const routed = (path: Path.Valid) => + this.#state.local.peek()?.has(path) || this.#state.bestEntry(path) !== undefined; + server.onChange = (path) => { + const entry = held.get(path); + const front = server.served.get(path); + if (!entry || entry.withdraw || !front) return; + entry.withdraw = this.#answer(path, front.clone()); + if (!entry.withdraw) { + server.served.delete(path); + front.close(); + } + }; + server.onReject = (path, err) => { + const entry = held.get(path); + if (entry && this.#state.requests.peek()?.get(path) === entry.slot && !entry.slot.answer && !routed(path)) { + this.#state.drop(path, entry.slot, err); + } + }; + void (async () => { + try { + while (server.closed.peek() === undefined) { + const map = this.#state.requests.peek(); + if (!map) break; + for (const [path, entry] of held) { + if (map.get(path) !== entry.slot || !this.#state.blind(entry.slot) || routed(path)) + forget(path); + } + for (const [path, slot] of map) { + if ( + held.has(path) || + slot.answer || + !this.#scope.matches(path) || + !this.#state.blind(slot) || + routed(path) + ) + continue; + held.set(path, { slot }); + server.enqueue(path); + } + await race([this.#changed(), server.closed]); + } + } finally { + close(); + } + })(); + return { requested: () => requested(server), close }; + } + /** * Advertise `prefix` and serve the requests beneath it. * @@ -1475,12 +1550,15 @@ export class Producer implements Table { } slot.answer = front; this.#state.refresh(path); + // A blind answer has no epoch that could prove a later route serves the same + // publisher, even if that route has already replaced the slot's current view. + const resolved = slot.route.peek()?.front === front; return () => { if (slot.answer === front) { // A request the answer resolved ends with its session: the next session is another // publisher instance, since nothing (no epoch) says it serves the same bytes. - if (slot.route.peek()?.front === front) { + if (resolved || slot.route.peek()?.front === front) { this.#state.drop(path, slot, unroutable()); } else { slot.answer = undefined; @@ -2321,36 +2399,39 @@ export class Dynamic { } /** Requests under this prefix, as they arrive, each to {@link Request.prepare} or reject. */ - async *requested(): AsyncIterableIterator { + requested(): AsyncIterableIterator { const server = this.#entry.server; - if (!server) return; - let current: Request | undefined; - const drop = () => { - current?.reject(unroutable()); - current = undefined; - }; - // Keep the cursor while draining: restarting it would rescan deleted entries. - let queued = server.queue.peek().values(); - try { - for (;;) { - const next = queued.next(); - if (!next.done) { - drop(); - server.queue.mutate((queue) => { - queue.delete(next.value); - }); - current = next.value; - current.publisher = this.#publisher; - yield next.value; - continue; - } - if (server.closed.peek() !== undefined) return; - await Signal.race(server.queue, server.closed); - queued = server.queue.peek().values(); + return server ? requested(server, this.#publisher) : (async function* () {})(); + } +} + +async function* requested(server: ServeState, publisher?: Producer): AsyncIterableIterator { + let current: Request | undefined; + const drop = () => { + current?.reject(unroutable()); + current = undefined; + }; + // Keep the cursor while draining: restarting it would rescan deleted entries. + let queued = server.queue.peek().values(); + try { + for (;;) { + const next = queued.next(); + if (!next.done) { + drop(); + server.queue.mutate((queue) => { + queue.delete(next.value); + }); + current = next.value; + current.publisher = publisher; + yield next.value; + continue; } - } finally { - drop(); + if (server.closed.peek() !== undefined) return; + await Signal.race(server.queue, server.closed); + queued = server.queue.peek().values(); } + } finally { + drop(); } } diff --git a/quest/m1/public-model/answer.md b/quest/m1/public-model/answer.md index 6e702b6f6f..43523404a7 100644 --- a/quest/m1/public-model/answer.md +++ b/quest/m1/public-model/answer.md @@ -10,131 +10,60 @@ reader. ## Plan Rules, carve-outs, and the inventory method are in the -[epic](/quest/m1/public-model/README.md). Rerun the inventory first; the -recommendations below are the planning quest's, and the ones marked -**decision** were settled by the maintainer on 2026-10-16 (#5717). +[epic](/quest/m1/public-model/README.md). The draft implementation binds new +track requests to their answer source's selection period, exposes atomic +unused operations and empty origins, moves source ownership into sessions, +and adds JS broadcast answering and a fallback request stream. Finish the +public contract and inventory before this quest is complete. -Rust items (`rs/moq-net/src/model/`), as they stood at b4eab0db1: +- The fallback must yield a public forwarded-request role that can accept + an answer. The current generic JS `Request.accept` is `@internal`, so it + is erased from published declarations. Making it public would recreate + the application-claim misuse that the required quest below removes. That + quest owns the type split; do not duplicate it here or substitute runtime + checks for the public role boundary. +- Settle the proposed names and ownership shape in the draft PR. The + current Rust proposal is `Producer::answer_source(&Request)`; prefer a + source factory tied to the eventual forwarded-request role if it removes + mismatched origins and application claims by construction. JS proposes + `Producer.fallback()`, with `unrouted()` as an alternative. The fallback + returns only the request stream and close operation, since it owns no + advertised route to update. +- Finish validity for already handed-out but unaccepted track requests. + Source retirement currently stops new lookups and queue handouts, while + ordinary broadcast close leaves a claimed request to its handler. A late + answer must not serve a retired instance. Preserve writes on tracks + accepted before retirement, and add coverage for the unfinished handout. +- Finish migrating test-only users of the JS request-slot wire view onto + the public answerer, then remove that view and the exported `RequestSlot`. + Delete the registered-but-unread `OriginConsumer.broadcasts` view. +- Remove the private Rust `Producer::stats` and `Consumer::stats` accessors + by retaining the supplied session stats at their callers. Fold surviving + hidden-discovery helpers into the public consumer shape after lazy + discovery lands, and remove the private `Consumer::request` wrapper by + using `request_broadcast` from the lite publisher. +- Rerun the inventory after the dependency lands. Keep route ingestion and + advertising with [Public routes](/quest/m1/public-model/routes.md). + Rust interests and snapshots currently have scaffolding, but production + sessions still discover all allowed prefixes; expose only what the lazy + discovery implementation needs, wherever that lands second. +- Keep per-operation validity for uncached FETCH and TRACK-info private + under [the deferred quest](/quest/m1/public-model/deferred.md). Closing a + source to new track requests cannot stop an already accepted epochless + track from fetching a replacement publisher. Do not weaken those checks + while moving the new-track boundary into the model. -- `origin::Producer::create_source` (both subscribers): public under a role - name (for example `answer_source`). It mints a broadcast with the origin's - cache policy and stats that answers a `Dynamic` request without entering - the route table, which `broadcast::Producer::new` cannot do. -- `broadcast::SourceGuard` (both subscribers): not model. Move it to the - sessions; a third party drops or closes its own source. -- **Request validity** (`broadcast::RequestValidity`, `origin::Selection`; - both subscribers). **Decision**: - - ✅ Remove the need. The source minted for a `Dynamic` request enforces - validity itself, so no session can serve a stale instance by forgetting - a check. Revised in review (#5717): once its route is replaced or its - selection period retires, the source refuses new track requests with - `Unroutable`, but tracks it already accepted keep running and finish. - `origin::Dynamic::update` and the subscriber drain paths rely on that, - so producer writes must not start failing. The answering session learns - it separately: the source's request stream (`poll_requested_track`) - ends, so it stops answering and drops the source once its accepted - tracks are done. - - A public validity handle every answerer must poll. -- **Unused races** (`broadcast::Producer::poll_unheld`, `close_unheld`, - `group::Producer::poll_unused`, `abort_unused`, - `track::Request::reject_unused`; both subscribers). **Decision**: - - ✅ Public, as one family matching the public - `track::Producer::abort_unused(self, err) -> Result<(), Self>`: each - answerer has `demand()` for the wait and an atomic `*_unused` that - refuses when a reader returned in the gap. Rename `close_unheld` and - `poll_unheld` to the `unused` vocabulary where their semantics match - `broadcast::Demand`, or document the difference. - - Keep them private. -- `origin::Dynamic::withdrawn` (lite subscriber): public, as the explicit - peer withdrawal that differs from a session simply dropping its handle. -- `origin::interest_prefixes` (both subscribers): remove the need through - Rust lazy discovery ([moq-dev/moq#6165](https://github.com/moq-dev/moq/pull/6165)), which gives Rust - the origin's watched interests the way JS has them; make that public in - both languages there or here, whichever lands second. -- `origin::Consumer::discovery`, `includes_hidden` (IETF publisher): fold - into the public `with_hidden` plus a getter, unless Lazy discovery deletes - them first. `beyond` (topping up a feed): re-inventory after Lazy - discovery; public only if it survives. -- `origin::Consumer::request` (lite publisher): remove the need; it is the - body of the public `request_broadcast`. -- `origin::Producer::empty`, `origin::Consumer::empty` (sessions): public. - A scope with no patterns is not an equivalent: `scope` refuses an empty - intersection with `Unauthorized`, which stays, since failing loud on a - scope that names nothing is the right default. The drivers fill an omitted - session half with an empty producer, and answer an out-of-scope announce - interest with an empty consumer, so a custom session needs the same - handles. Restructuring the drivers to run without a half is the - alternative, but it is a bigger change for no public gain. -- `origin::Producer::stats`, `origin::Consumer::stats` (sessions): remove - the need; the caller keeps the `stats::Session` it passed to the public - `with_stats`. -- Test-only: `origin::Producer::announce`, `AnnounceProducer`, - `ProduceTest`, `origin::Consumer::get_broadcast`. +The fallback still must serve only unrouted, authorized requests, never +advertise, accept the first answer, and error resolved requests when its +handle dies or an announced route replaces the blind answer. Unresolved and +new requests may use a standby; resolved ones never stitch across instances. +Each public call needs mock-time coverage, and the built-in sessions must +use the public answering calls. Public API and wire impact stay explicit in +the PR; these model changes do not change the wire format. -JS items (`js/net/src/`): +## Required -- `wire.ts` `OriginProducer.requests`, `changed`, `blind`, `answer`, - `routes`, `origin.ts` `RequestSlot`, `internal.ts` `coveringPrefixes` - (the blind-answer loop in `connection/forward.ts`). **Decision**: - - ✅ Replace with the Rust shape. A session answers through the public - `Origin.Dynamic` requests and mints a source per request, as Rust - sessions do, so the slot loop and its view go. - - Make the `wire.ts` views public. -- **Blind answers** (the same loop, raised in review of #5717). Every JS - session answers each request no route covers by subscribing on its peer: - always, and as the only path after a peer refuses or resets discovery. - Lazy discovery keeps a plain JS - `request(path)` blind, so blind answering stays in JS and needs a public - shape before the loop goes. Rust has no blind answers (decided 2026-10-20, - maintainer), so no Rust session needs this shape. - **Decision** (settled 2026-10-16, narrowed to JS 2026-10-20): - - ✅ A fallback answerer, `Origin.Producer.fallback()` in JS. The name is still open: propose - alternatives in this quest's PR before shipping it. It returns the same - `Dynamic` request stream, scoped to the handle's allowed paths. It - receives only requests that no announced or served route covers. Once - the table routes a path, the new route takes only unresolved and new - requests for it. A request the fallback already resolved ends with an - error, since a blind answer has no epoch to prove the route serves the - same instance (never stitch). It is never - advertised, since it claims nothing. The first fallback to answer a - request wins. Dropping the handle ends the requests it resolved with an - error and never moves them to another fallback, which may be another - publisher instance (never stitch). A standby answers only requests - still unresolved, and new ones. This is what `serveRequests` in - `connection/forward.ts` does today. - - A `dynamic(prefix, route)` flagged blind and ranked below every - announced route. This reuses one call but puts an unadvertised, - claim-free entry in the route table, where selection and split-horizon - must special-case it. - Delete the JS loop only once the fallback answers the same requests, - including after a downgrade. -- `wire.ts` `OriginProducer.expect` (`connection/reload.ts`, keeps requests - pending while a reconnect is in flight): public if Rust has the same need - across a reconnect, else private to the connection. -- `wire.ts` `OriginProducer.interests`, `attach`, `accepts`: `interests` - is public with Lazy discovery; `attach(discovery)` goes with the - `discovery` option Lazy discovery removes; `accepts` is a check on the - public `allowed()` patterns. -- `wire.ts` `Broadcast.requested` (with its `served` flag) and - `overrideBroadcastWire` (both subscribers). Part of the JS answering - decision above: ✅ public, mirroring Rust's - `broadcast::Producer::dynamic()`: a broadcast producer hands out track - requests on demand, and a track's info and fetches are answered through - its `Track.Request` and `Track.Dynamic`. A subclass of - `Broadcast.Consumer` cannot do either today. The track-request handout - lands here. Fetch and info answering through ranges (`overrideBroadcastWire`) - moved to [the deferred quest](/quest/m1/public-model/deferred.md), which - waits on Model ranges. -- `wire.ts` `OriginConsumer.broadcasts`: registered and never read; delete. -- `announced.ts` `Announce.Producer`: remove the need; a session's route - ingestion goes through the origin. -- `wireOf`, `registerWire`, `trackOf`, `registerTrackConsumer`: stay - package-private plumbing, shrinking as the views above empty out. Delete - `wire.ts` if nothing is left. - -Tests: each public call gets a mock-time test in its language, and the -built-in sessions move onto the public calls, so their existing suites -cover the behavior. +- [Claim request type](/quest/m1/public-model/claim-request-type.md) - split forwarded requests from application claims before publishing a fallback that can accept answers ## Related diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 0c0ba53255..9b74e5ac44 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -2537,7 +2537,13 @@ where Some(requested) => requested.to_owned(), None => continue, }; - let source = crate::model::broadcast::SourceGuard::new(self.origin.create_source(&requested)); + let source = match self.origin.answer_source(&request) { + Ok(source) => crate::session::SourceGuard::new(source), + Err(err) => { + request.reject(err); + continue; + } + }; // The handler exists before the requester sees the source, so a track it asks // for before `run_broadcast` first polls queues instead of failing `NotFound`. let dynamic = source.dynamic(); @@ -2610,7 +2616,7 @@ where async fn run_broadcast( &self, path: Path<'_>, - source: crate::model::broadcast::SourceGuard, + source: crate::session::SourceGuard, mut broadcast: broadcast::Dynamic, route: Option>, ) -> Result<(), Error> { @@ -2722,7 +2728,7 @@ where // Open while the namespace stays announced and authorized, which its own token may // have done in place of the scope. `None` once retracted. route: Option>, - request: track::Request, + mut request: track::Request, ) { // Data streams wait on the alias bound by SUBSCRIBE_OK, so leave the model request // pending until its immutable track metadata is known. @@ -2752,9 +2758,10 @@ where request.reject(Error::Unauthorized); return; } - None if request.reject_unused(Error::Cancel) => return, - // Demand returned in the gap. - None => continue, + None => match request.reject_unused(Error::Cancel) { + Ok(()) => return, + Err(returned) => request = returned, + }, } } @@ -3116,9 +3123,10 @@ where else { break None; }; - if pending.reject_unused(Error::Cancel) { - break None; - } + let pending = match pending.reject_unused(Error::Cancel) { + Ok(()) => break None, + Err(returned) => returned, + }; if let Some(held) = state.subscribes.get_mut(&request_id) { held.pending = Some(pending); } @@ -4137,6 +4145,7 @@ where // Nobody wanting the track any more ends the wait on a peer that may never answer, // and the session ending fails it with the session's reason. let mut demand = request.demand(); + let mut pending = Some(request); let mut closed = self.session.clone(); let response = { let mut response = std::pin::pin!(self.read_track_status_response(&mut stream)); @@ -4163,15 +4172,17 @@ where .await; match res { Some(res) => break Some(res), - None if request.reject_unused(Error::Cancel) => break None, - None => continue, + None => match pending.take().expect("request pending").reject_unused(Error::Cancel) { + Ok(()) => break None, + Err(returned) => pending = Some(returned), + }, } } }; let response = match response { Some(SetupResponse::Response(response)) => response, Some(SetupResponse::Cancelled(err)) => { - request.reject(err); + pending.take().expect("request pending").reject(err); stream.reader.abort(&Error::Cancel); let _ = stream.writer.close().await; return None; @@ -4182,6 +4193,7 @@ where return None; } }; + let request = pending.expect("a received response keeps its request"); let _ = stream.writer.finish(); let ok = match response { // A track that needs a property we do not understand is not forwarded. @@ -9320,7 +9332,7 @@ mod tests { async fn a_minted_source_closes_unheld_or_retracted() { let (subscriber, origin) = cluster_subscriber(crate::Hop::new(1).unwrap()); let run = |route: &kio::Producer<()>| { - let source = crate::model::broadcast::SourceGuard::new(origin.create_source("pool/p")); + let source = crate::session::SourceGuard::new(origin.create_source("pool/p")); let (holder, dynamic) = (source.consume(), source.dynamic()); let (this, route) = (subscriber.clone(), route.consume()); let task = moq_clock::sim::spawn(async move { diff --git a/rs/moq-net/src/lite/subscriber.rs b/rs/moq-net/src/lite/subscriber.rs index 817fb1d9f0..7675582ae5 100644 --- a/rs/moq-net/src/lite/subscriber.rs +++ b/rs/moq-net/src/lite/subscriber.rs @@ -1817,7 +1817,7 @@ struct MintedSource { path: PathOwned, /// The publisher instance the route announced, asked for on every request. epoch: Option, - source: crate::model::broadcast::SourceGuard, + source: crate::session::SourceGuard, /// The source's track handler, registered before its requester could ask for a track. dynamic: crate::broadcast::Dynamic, /// Closes when the route that minted the source goes. @@ -1834,7 +1834,7 @@ struct SourceServe { subscriber: Subscriber, path: PathOwned, epoch: Option, - source: crate::model::broadcast::SourceGuard, + source: crate::session::SourceGuard, dynamic: crate::broadcast::Dynamic, route: kio::Consumer<()>, // A dedicated close-watch handle, since each pending operation needs its own. @@ -3781,7 +3781,7 @@ mod tests { validity, path: Path::new("room").to_owned(), epoch: None, - source: crate::model::broadcast::SourceGuard::new(broadcast), + source: crate::session::SourceGuard::new(broadcast), dynamic, route: route.consume(), }, @@ -5995,7 +5995,13 @@ impl Announced { continue; }; let path = path.to_owned(); - let source = subscriber.origin.create_source(&path); + let source = match subscriber.origin.answer_source(&request) { + Ok(source) => source, + Err(err) => { + request.reject(err); + continue; + } + }; // The handler exists before the requester sees the source, so a track it // asks for before the serve machine first runs queues instead of failing // `NotFound`. Accepted before the push, so the requester holds the source @@ -6011,7 +6017,7 @@ impl Announced { validity, path, epoch: entry.route.epoch.clone(), - source: crate::model::broadcast::SourceGuard::new(source), + source: crate::session::SourceGuard::new(source), dynamic, route: entry.live.consume(), }); @@ -6613,14 +6619,17 @@ impl TrackServeRun { /// stream. Ready once the request is released. fn poll_unused(&mut self, waiter: &kio::Waiter) -> Poll<()> { loop { - let TrackRunState::Info { request, .. } = &self.state else { + let TrackRunState::Info { request, .. } = &mut self.state else { return Poll::Pending; }; let pending = request.as_ref().expect("request pending"); ready!(crate::util::poll_unwanted(&mut pending.demand(), waiter)); - if pending.reject_unused(Error::Cancel) { - self.state = TrackRunState::Done; - return Poll::Ready(()); + match request.take().expect("request pending").reject_unused(Error::Cancel) { + Ok(()) => { + self.state = TrackRunState::Done; + return Poll::Ready(()); + } + Err(returned) => *request = Some(returned), } // Demand returned in the gap and won inside `reject_unused`: poll again so the // unused wait is armed for when it leaves. diff --git a/rs/moq-net/src/model/broadcast.rs b/rs/moq-net/src/model/broadcast.rs index 07e34532d8..194f79790d 100644 --- a/rs/moq-net/src/model/broadcast.rs +++ b/rs/moq-net/src/model/broadcast.rs @@ -133,11 +133,21 @@ struct BroadcastState { // one serves again once the newer route at its path withdraws. finished: bool, + // A served source accepts new tracks only during the selection period that minted it. + validity: RequestValidity, + // Set once a finished broadcast's origin advertises a newer epoch at its path, which // ends its `Finish` at once. superseded: bool, } +fn check_validity(state: &kio::Shared, waiter: &kio::Waiter) -> Result<(), Error> { + // The selection reads the origin, whose route changes can close broadcasts. Never + // hold the broadcast lock while checking it. + let validity = state.read().validity.clone(); + validity.check(waiter) +} + impl BroadcastState { /// Insert a track weak handle into the lookup, returning an error if a live /// track already holds the name. A closed entry under the name is reclaimed. @@ -450,6 +460,11 @@ impl Producer { } } + pub(super) fn with_validity(self, validity: RequestValidity) -> Self { + self.state.lock().validity = validity.start(); + self + } + /// Create a dynamic producer that handles on-demand track requests from consumers. pub fn dynamic(&self) -> Dynamic { Dynamic::new( @@ -530,16 +545,16 @@ impl Producer { } /// `Ready(Ok)` once no [`Consumer`] handle is left, `Ready(Err)` once the broadcast ended. - // pending: /quest/m1/public-model/answer.md - pub(crate) fn poll_unheld(&self, waiter: &kio::Waiter) -> Poll> { + // Unlike Demand, this counts held broadcast handles even when none reads a track. + pub fn poll_unheld(&self, waiter: &kio::Waiter) -> Poll> { self.alive.token.poll_unused(waiter) } /// [`Self::close`], unless a [`Consumer`] holds the broadcast or a track it asked for /// still waits for a handler. Atomic with minting a consumer, so a holder either keeps /// the broadcast or only ever sees it closed. Returns whether the broadcast is closed. - // pending: /quest/m1/public-model/answer.md - pub(crate) fn close_unheld(&self) -> bool { + // Unlike Demand, held broadcast handles prevent this close even while idle. + pub fn close_unheld(&self) -> bool { self.alive.close_unheld() } @@ -720,7 +735,7 @@ impl Drop for Alive { /// Whether a path request still names the source that handed out its track. #[derive(Clone, Default)] #[cfg_attr(not(feature = "async"), allow(dead_code))] -// pending: /quest/m1/public-model/answer.md +// pending: /quest/m1/public-model/deferred.md pub(crate) enum RequestValidity { #[default] Unrestricted, @@ -732,13 +747,13 @@ pub(crate) enum RequestValidity { impl RequestValidity { #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md + // pending: /quest/m1/public-model/deferred.md pub(crate) fn token(route: Option>) -> Self { route.map(Self::Route).unwrap_or_default() } #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md + // pending: /quest/m1/public-model/deferred.md pub(crate) fn for_source(route: &crate::origin::Dynamic, path: &crate::Path) -> Self { route .request_validity(path) @@ -748,7 +763,7 @@ impl RequestValidity { /// Capture one selection period so a pending request never revives after a replacement. #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md + // pending: /quest/m1/public-model/deferred.md pub(crate) fn start(&self) -> Self { match self { Self::Selected(selected) => Self::Period(selected.current(), selected.clone()), @@ -757,7 +772,7 @@ impl RequestValidity { } #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md + // pending: /quest/m1/public-model/deferred.md pub(crate) fn check(&self, waiter: &kio::Waiter) -> Result<(), crate::Error> { match self { Self::Route(route) if route.poll_closed(waiter).is_ready() => Err(crate::Error::Unroutable), @@ -785,37 +800,6 @@ impl Producer { } } -/// A session-owned handle to a source broadcast created via -/// [`crate::origin::Producer::create_broadcast`], closing it on drop even while the -/// session's serve machines still hold its [`Dynamic`]. -/// -/// A peer's retraction and a dead session end the source the same way: a broadcast -/// carries no end cause past this hop. Shared by the lite and IETF subscribers. -#[cfg_attr(not(feature = "async"), allow(dead_code))] -// pending: /quest/m1/public-model/answer.md -pub(crate) struct SourceGuard(Producer); - -impl SourceGuard { - #[cfg_attr(not(feature = "async"), allow(dead_code))] - pub fn new(producer: Producer) -> Self { - Self(producer) - } -} - -impl std::ops::Deref for SourceGuard { - type Target = Producer; - - fn deref(&self) -> &Producer { - &self.0 - } -} - -impl Drop for SourceGuard { - fn drop(&mut self) { - self.0.close(); - } -} - /// Handles on-demand track creation for a broadcast. /// /// When a consumer requests a track that doesn't exist, the dynamic producer @@ -892,6 +876,10 @@ impl Dynamic { /// Returns [`Error::Closed`] once the broadcast has ended, so a serving loop /// knows to stop and release its handle. pub fn poll_requested_track(&mut self, waiter: &kio::Waiter) -> Poll> { + if check_validity(&self.state, waiter).is_err() { + self.alive.close(); + return Poll::Ready(Err(Error::Closed)); + } let mut state = ready!(self.state.poll(waiter, |state| { if state.requests.has_queued() || state.closing { Poll::Ready(()) @@ -1052,6 +1040,7 @@ impl Consumer { /// has ended, and [`Error::NotFound`] when nothing publishes the track and no /// [`Dynamic`] would serve a request for it. pub fn track(&self, name: &str) -> Result { + check_validity(&self.state, &kio::Waiter::noop())?; let current = { let mut state = self.state.lock(); // An ended broadcast serves nothing new, not even a track it still has: @@ -1084,6 +1073,7 @@ impl Consumer { name: &str, subscription: Option<&track::Subscription>, ) -> Result<(kio::Consumer, Option), Error> { + check_validity(&self.state, &kio::Waiter::noop())?; let mut state = loop { let mut state = self.state.lock(); diff --git a/rs/moq-net/src/model/group.rs b/rs/moq-net/src/model/group.rs index a35f014980..4238a603ba 100644 --- a/rs/moq-net/src/model/group.rs +++ b/rs/moq-net/src/model/group.rs @@ -1669,14 +1669,12 @@ impl Producer { self.handle.frame_count() } - /// Abort with `err` only while nothing consumes the group, returning whether it is - /// closed. Consumer creation and the check share a lock, so a reader arriving after - /// [`poll_unused`](Self::poll_unused) keeps the group alive instead of reading the abort. - #[allow(dead_code)] - // The answering API retains these on the writer as well as the session observer. - // pending: /quest/m1/public-model/answer.md - pub(crate) fn abort_unused(&self, err: Error) -> bool { - self.handle.abort_unused(err) + /// Abort an unused group, returning its writer when a reader returned first. + pub fn abort_unused(self, err: Error) -> std::result::Result<(), Self> { + match self.handle.abort_unused(err) { + true => Ok(()), + false => Err(self), + } } /// Whether the group has been aborted (including pool eviction). The track's @@ -1706,14 +1704,6 @@ impl Producer { pub fn demand(&self) -> Demand { self.handle.demand() } - - /// Poll for the group becoming unused (every consumer dropped). - #[allow(dead_code)] - // The answering API retains these on the writer as well as the session observer. - // pending: /quest/m1/public-model/answer.md - pub(crate) fn poll_unused(&self, waiter: &kio::Waiter) -> Poll<()> { - self.handle.poll_unused(waiter) - } } impl Handle { @@ -1844,7 +1834,7 @@ impl Handle { /// closed. Consumer creation and the check share a lock, so a reader arriving after /// [`poll_unused`](Self::poll_unused) keeps the group alive instead of reading the abort. #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md + // private: capability gap pub(crate) fn abort_unused(&self, err: Error) -> bool { match self.state.write_unused() { kio::Unused::Idle(guard) => { @@ -2158,7 +2148,7 @@ impl Handle { /// Poll for the group becoming unused (every consumer dropped). #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md + // private: capability gap pub(crate) fn poll_unused(&self, waiter: &kio::Waiter) -> Poll<()> { self.state.poll_unused(waiter).map(|_| ()) } @@ -3685,17 +3675,14 @@ mod test { let producer = Info { sequence: 0 }.produce(); let consumer = producer.handle.try_consume().expect("open"); - assert!( - !producer.abort_unused(crate::Error::Cancel), - "a reader declines the abort" - ); + let producer = producer + .abort_unused(crate::Error::Cancel) + .expect_err("a reader declines the abort"); drop(consumer); - assert!(producer.abort_unused(crate::Error::Cancel)); - assert!( - producer.handle.try_consume().is_none(), - "an aborted group mints no reader" - ); + let handle = producer.handle(); + assert!(producer.abort_unused(crate::Error::Cancel).is_ok()); + assert!(handle.try_consume().is_none(), "an aborted group mints no reader"); } #[test] diff --git a/rs/moq-net/src/model/origin.rs b/rs/moq-net/src/model/origin.rs index d59a83a945..976210de36 100644 --- a/rs/moq-net/src/model/origin.rs +++ b/rs/moq-net/src/model/origin.rs @@ -1906,8 +1906,7 @@ impl Producer { /// subscriber issues no ANNOUNCE_PLEASE). Used to fill an unset session half /// so both the publisher and subscriber loops still run. #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md - pub(crate) fn empty(hop: Hop) -> Self { + pub fn empty(hop: Hop) -> Self { // No allowed prefixes means no broadcast is ever created, so nothing will // ever be queued on the detached submission handle. let (tasks, _) = TaskSet::new(); @@ -2031,13 +2030,35 @@ impl Producer { Ok(broadcast) } + /// Mint an unannounced source for this origin's forwarded request, bound to its selection. + /// + /// New track requests fail with [`Error::Unroutable`] once that selection retires; + /// accepted tracks keep running. Application claims return [`Error::Unroutable`] + /// and answer through [`Request::prepare`] instead. Another origin or an + /// unauthorized path returns [`Error::Unauthorized`]. + pub fn answer_source(&self, request: &Request) -> Result { + // Application claims publish prepared answers under their own epoch. + if request.publisher.is_some() { + return Err(Error::Unroutable); + } + let broadcast::RequestValidity::Selected(selected) = &request.validity else { + return Err(Error::Unroutable); + }; + if !selected.shared.same_channel(&self.shared) || !self.scope.permits(request.path()) { + return Err(Error::Unauthorized); + } + let relative = request.path().strip_prefix(self.root()).ok_or(Error::Unauthorized)?; + Ok(self.create_source(relative).with_validity(request.validity.clone())) + } + /// Mint a standalone source broadcast for a served-route request: it carries /// this origin's cache policy and ingress attribution, but /// is *not* entered into the route table. Sessions answer /// [`Dynamic`] requests with one of these; the requester already holds /// the request's result channel, so the table never needs to resolve it. #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md + // private: test + // Session fixtures also mint an unrestricted source without a route. pub(crate) fn create_source(&self, path: impl AsPath) -> broadcast::Producer { let path = path.as_path(); let full = self.root.join(&path).to_owned(); @@ -5242,8 +5263,7 @@ impl Dynamic { /// Retire an explicit peer withdrawal, hiding derived paths only once its /// last live advertisement at the prefix is gone. A lost session just drops. #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md - pub(crate) fn withdrawn(self) { + pub fn withdrawn(self) { self.announcement.retract(true); } @@ -5304,7 +5324,13 @@ impl Dynamic { // entry. This mirrors how `poll_requested_track` keeps a served track // discoverable via the weak cache across the same window. let producer = state.requests.get(&path).expect("popped key must be pending").clone(); + drop(state); + let validity = match self.publisher { + Some(_) => Default::default(), + None => broadcast::RequestValidity::for_source(self, &path), + }; Poll::Ready(Ok(Request { + validity, path, producer, home: self.state.clone(), @@ -5416,6 +5442,7 @@ impl ServeState { /// resolves it with a new exact-path broadcast, and [`reject`](Self::reject) resolves it /// with an error. Dropping the request without either rejects it. pub struct Request { + validity: broadcast::RequestValidity, // Absolute path that was requested. path: PathOwned, @@ -5963,8 +5990,7 @@ impl Consumer { /// announce-interest for a prefix outside our scope by announcing nothing, /// rather than tearing the stream down. #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md - pub(crate) fn empty(&self) -> Self { + pub fn empty(&self) -> Self { Self { scope: OriginScope::empty(), ..self.clone() @@ -6994,6 +7020,63 @@ mod tests { use futures::FutureExt; use std::sync::atomic::AtomicBool; + #[moq_clock::test] + async fn answer_source_retires_new_tracks_without_aborting_accepted_tracks() { + let producer = origin(1).produce(); + let route = producer.clone().peer().dynamic_route("room", Route::default()).unwrap(); + let consumer = producer.consume(); + let requesting = consumer.request_broadcast("room/alice", None); + let request = queued(&route).await; + let source = producer.answer_source(&request).unwrap(); + let mut tracks = source.dynamic(); + request.accept(&source); + let _routed = requesting.await.unwrap(); + let front = source.consume(); + let reading = front.track("video").unwrap().subscribe(None); + assert!(reading.poll_ok(&kio::Waiter::noop()).is_pending()); + let track = tracks.assert_request().accept(None); + drop(route); + assert!(matches!(front.track("new"), Err(Error::Unroutable))); + assert!(tracks.poll_requested_track(&kio::Waiter::noop()).is_ready()); + track.finish().unwrap(); + drop(reading); + } + + #[moq_clock::test] + async fn answer_source_refuses_application_claims_and_another_origin() { + let producer = origin(1).produce(); + let claim = producer.dynamic("room").unwrap(); + let _requesting = producer.consume().request_broadcast("room/alice", None); + let request = queued(&claim).await; + assert!(matches!(producer.answer_source(&request), Err(Error::Unroutable))); + drop(request); + drop(claim); + let peer = producer + .clone() + .peer() + .dynamic_route("other", Route::default()) + .unwrap(); + let _requesting = producer.consume().request_broadcast("other/alice", None); + let request = queued(&peer).await; + let unrelated = origin(2).produce(); + assert!(matches!(unrelated.answer_source(&request), Err(Error::Unauthorized))); + } + + #[test] + fn empty_handles_keep_their_identity_and_refuse_every_path() { + let empty = Producer::empty(origin(1)); + assert_eq!(empty.hop(), origin(1)); + assert!(empty.allowed().is_empty()); + assert!(matches!(empty.create_broadcast("room"), Err(Error::Unauthorized))); + let consumer = origin(2).produce().consume().empty(); + assert_eq!(consumer.hop(), origin(2)); + assert!(consumer.allowed().is_empty()); + assert!(matches!( + consumer.request_broadcast("room", None).poll_ok(&kio::Waiter::noop()), + Poll::Ready(Err(Error::Unauthorized)) + )); + } + fn origin(id: u64) -> Hop { Hop::new(id).unwrap() } diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 3b75eb5b41..abdedadd44 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -7360,19 +7360,23 @@ impl Request { self } - /// Reject only while no consumer needs this pending track. Demand and the check - /// share one lock, so demand returning after `Demand::wanted` went to `None` wins the race, and the - /// close under that lock stops a later consumer attaching to a dead request. - #[cfg_attr(not(feature = "async"), allow(dead_code))] - // pending: /quest/m1/public-model/answer.md - pub(crate) fn reject_unused(&self, err: Error) -> bool { - match self.state.write_unused() { + /// Reject an unused request, returning it when a reader returned first. + #[expect( + clippy::result_large_err, + reason = "return the owned request without allocating on an idle check" + )] + pub fn reject_unused(self, err: Error) -> std::result::Result<(), Self> { + let unused = match self.state.write_unused() { kio::Unused::Idle(guard) => { commit_abort(guard, err).abort(); true } kio::Unused::Closed => true, kio::Unused::Used => false, + }; + match unused { + true => Ok(()), + false => Err(self), } } @@ -14062,7 +14066,9 @@ mod test { drop(first.await.unwrap()); let hit = consumer.fetch_group(5, None); - assert!(!group.abort_unused(Error::Cancel), "the pending hit wants the group"); + let _group = group + .abort_unused(Error::Cancel) + .expect_err("the pending hit wants the group"); let mut fetched = hit.await.unwrap(); assert_eq!(&fetched.read_frame().await.unwrap().unwrap().payload[..], b"head"); } @@ -14112,7 +14118,7 @@ mod test { .unwrap(); let group = req.accept(Info::timed()).unwrap(); drop(first.await.unwrap()); - assert!(group.abort_unused(Error::Cancel)); + assert!(group.abort_unused(Error::Cancel).is_ok()); let retry = consumer.fetch_group(5, None); let req = dynamic diff --git a/rs/moq-net/src/session.rs b/rs/moq-net/src/session.rs index 5d217ea7cb..64f3be9e8f 100644 --- a/rs/moq-net/src/session.rs +++ b/rs/moq-net/src/session.rs @@ -6,6 +6,36 @@ use crate::transport::Stats as _; use crate::{Error, SessionError, Version, auth, bandwidth, coding::StreamCodes, goaway}; +/// A session-owned handle to a source broadcast created via +/// [`crate::origin::Producer::answer_source`], closing it on drop even while the +/// session's serve machines still hold its [`Dynamic`]. +/// +/// A peer's retraction and a dead session end the source the same way: a broadcast +/// carries no end cause past this hop. Shared by the lite and IETF subscribers. +#[cfg_attr(not(feature = "async"), allow(dead_code))] +pub(crate) struct SourceGuard(crate::broadcast::Producer); + +impl SourceGuard { + #[cfg_attr(not(feature = "async"), allow(dead_code))] + pub fn new(producer: crate::broadcast::Producer) -> Self { + Self(producer) + } +} + +impl std::ops::Deref for SourceGuard { + type Target = crate::broadcast::Producer; + + fn deref(&self) -> &crate::broadcast::Producer { + &self.0 + } +} + +impl Drop for SourceGuard { + fn drop(&mut self) { + self.0.close(); + } +} + /// How long [`Session::close`] waits for queued data before closing anyway. const CLOSE_TIMEOUT: Duration = Duration::from_secs(1); diff --git a/test/drill/mutations/subscriber-leaks-broadcasts.patch b/test/drill/mutations/subscriber-leaks-broadcasts.patch index 75e14022e9..2865d6d63c 100644 --- a/test/drill/mutations/subscriber-leaks-broadcasts.patch +++ b/test/drill/mutations/subscriber-leaks-broadcasts.patch @@ -34,9 +34,9 @@ diff --git a/rs/moq-net/src/lite/subscriber.rs b/rs/moq-net/src/lite/subscriber. subscriber: Subscriber, path: PathOwned, epoch: Option, -- source: crate::model::broadcast::SourceGuard, +- source: crate::session::SourceGuard, + // MUTATION: never dropped, so a session that ends keeps every source it minted alive. -+ source: std::mem::ManuallyDrop, ++ source: std::mem::ManuallyDrop, dynamic: crate::broadcast::Dynamic, route: kio::Consumer<()>, // A dedicated close-watch handle, since each pending operation needs its own.