From b9bb44d3d1c8871a498b976250b9c4bb51de2735 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 27 Oct 2026 23:42:29 -0700 Subject: [PATCH 1/6] chore(quest): claim quest/m1/api-audit/wanted.md Quest-Claim: c4bd47d367cf460e From 1dc03c22fe72010eca2c88509224adee8f34087d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 28 Oct 2026 00:47:29 -0700 Subject: [PATCH 2/6] feat!: demand reads as wanted everywhere kio, moq-net, @moq/net, moq-ffi, moq-c, and the wrappers name every demand signal wanted: a snapshot wanted() plus wanted_changed() and its poll. bandwidth::Producer gets a demand() handle, track::Fill merges is_used() and next() into wanted(), and @moq/net drops the internal Broadcast.Demand. Co-Authored-By: Claude Opus 5.5 --- doc/lib/c/index.md | 2 +- go/wrapper/README.md | 2 +- go/wrapper/doc.go | 4 +- go/wrapper/moq_test.go | 4 +- go/wrapper/publish.go | 28 ++- js/e2ee/src/e2ee.test.ts | 6 +- js/flate/src/stream/stream.test.ts | 4 +- js/hang/src/container/consumer.test.ts | 4 +- js/json/src/stream/stream.test.ts | 4 +- js/net/bench/broadcasts.ts | 37 +--- js/net/bench/observed.ts | 2 +- js/net/src/broadcast.test.ts | 124 +------------ js/net/src/broadcast.ts | 117 +----------- js/net/src/group.test.ts | 33 +--- js/net/src/group.ts | 34 ++-- js/net/src/ietf/publisher.test.ts | 17 +- js/net/src/ietf/subscriber.ts | 4 +- js/net/src/integration.test.ts | 37 ++-- js/net/src/listing.test.ts | 16 +- js/net/src/listing.ts | 16 +- js/net/src/lite/connection.ts | 2 +- js/net/src/lite/subscriber.ts | 66 +++---- js/net/src/lite/track_stream.test.ts | 70 ++++--- js/net/src/resume.test.ts | 10 +- js/net/src/resume.ts | 4 +- js/net/src/util/observed.test.ts | 24 +-- js/net/src/util/observed.ts | 8 +- py/moq-rs/moq/publish.py | 27 ++- py/moq-rs/tests/test_local.py | 3 +- rs/kio/src/consumer.rs | 6 +- rs/kio/src/lib.rs | 5 +- rs/kio/src/loom.rs | 36 ++-- rs/kio/src/producer.rs | 94 ++++------ rs/kio/src/tests.rs | 34 ++-- rs/kio/src/weak.rs | 91 +++------ rs/moq-archive/src/consumer/track.rs | 3 +- rs/moq-c/src/api.rs | 8 +- rs/moq-c/src/bandwidth.rs | 4 +- rs/moq-c/src/publish.rs | 7 +- rs/moq-c/src/test.rs | 30 +-- rs/moq-ffi/src/bandwidth.rs | 8 +- rs/moq-ffi/src/producer.rs | 45 ++--- rs/moq-ffi/src/session/dial.rs | 17 +- rs/moq-ffi/src/test.rs | 15 +- rs/moq-net/benches/track.rs | 2 +- rs/moq-net/src/ietf/publisher.rs | 2 +- rs/moq-net/src/ietf/subscriber.rs | 18 +- rs/moq-net/src/lite/subscriber.rs | 46 ++--- rs/moq-net/src/model/bandwidth.rs | 76 ++++++-- rs/moq-net/src/model/broadcast.rs | 24 +-- rs/moq-net/src/model/front.rs | 90 ++++----- rs/moq-net/src/model/group.rs | 37 ++-- rs/moq-net/src/model/listing.rs | 32 ++-- rs/moq-net/src/model/origin.rs | 58 +++--- rs/moq-net/src/model/resume.rs | 6 +- rs/moq-net/src/model/track.rs | 175 +++++++++++------- rs/moq-net/src/model/track/fill.rs | 100 ++++++---- rs/moq-net/src/server.rs | 10 +- rs/moq-net/src/session.rs | 13 +- rs/moq-net/tests/integration/fetch_cancel.rs | 6 +- rs/moq-net/tests/loom.rs | 6 +- rs/moq-scene/src/source.rs | 2 +- rs/moq-stats/src/produce.rs | 8 +- rs/moq-tokio/src/connection.rs | 43 +++-- rs/moq-transcode/src/lib.rs | 6 +- rs/moq-transcode/src/rung.rs | 3 +- swift/Sources/Moq/Requests.swift | 28 ++- .../clients/js/src/probe.test.ts | 4 +- 68 files changed, 844 insertions(+), 1063 deletions(-) diff --git a/doc/lib/c/index.md b/doc/lib/c/index.md index 8be91d05c6c..f53b815136c 100644 --- a/doc/lib/c/index.md +++ b/doc/lib/c/index.md @@ -83,7 +83,7 @@ inside. The header is the reference: each function carries a doc comment. - **JSON values.** `moq_consume_json_value` fills `moq_json_value` with the document and its frame's `timestamp_us`, rounded down to a whole microsecond; `timestamp_present` is false on an untimed track. A snapshot consumer's callback fires for each state in order, but a group superseded before it is read is skipped. - **Decoded reads.** `moq_decode_audio` and `moq_decode_video` deliver read ids in order: a frame, a gap where media was lost, or an end where the publisher stopped its encoder. Check `kind` on what `moq_decode_audio_read` or `moq_decode_video_read` writes. Free each id with `moq_decode_audio_read_free` or `moq_decode_video_read_free` promptly: a held video frame holds a decoder buffer, and the decoder stalls. - **Stats.** `moq_session_stats()` fills `moq_connection_stats` with `rtt_us`, `estimated_send_rate_bps`, and the byte and packet counters (`bytes_sent`, `bytes_received`, `bytes_lost`, `packets_sent`, `packets_received`, `packets_lost`). Each has a `_valid` flag, false when the transport does not report it, which is not the same as zero. `moq_session_snapshot()` adds the negotiated `protocol`. A failed dial or dropped connection reaches the session's `on_state` with its `failure` code, and `moq_error()` gives the reason during that callback. Hold the handle returned by `moq_session_recv_bandwidth()` to request receive estimates. `moq_bandwidth_consumer_peek()` reads the estimate and `moq_bandwidth_consumer_changed()` waits for a change. `moq_bandwidth_consumer_close()` releases demand; the last consumer stops PROBE. Stats snapshots do not request it. -- **Demand.** A watcher from `moq_publish_track_demand()` (or the media, video, and audio variants) calls back `MOQ_DEMAND_USED` or `MOQ_DEMAND_UNUSED`, again whenever the wanted group moves. `moq_publish_demand_wanted()` reads that group: start a new one once it passes your largest. Set `refresh` in `moq_track_info` to have a joining subscriber ask for a new group once the newest has grown past `refresh` times its first frame. +- **Demand.** A watcher from `moq_publish_track_demand()` (or the media, video, and audio variants) calls back `MOQ_DEMAND_WANTED` or `MOQ_DEMAND_UNWANTED`, again whenever the wanted group moves. `moq_publish_demand_wanted()` reads that group: start a new one once it passes your largest. Set `refresh` in `moq_track_info` to have a joining subscriber ask for a new group once the newest has grown past `refresh` times its first frame. - **Disabled renditions.** Audio and video convenience decoders reject a catalog configuration with `enabled: false`. Select an enabled rendition from the latest catalog before opening a decoder. Among the video renditions you can decode, prefer the highest `preference`; `0` is the default and `-1` marks a compatibility transcode. ## Reference diff --git a/go/wrapper/README.md b/go/wrapper/README.md index 034eea4ddf4..8579b938ebb 100644 --- a/go/wrapper/README.md +++ b/go/wrapper/README.md @@ -140,7 +140,7 @@ the session closed, i.e. not a real failure) and `moq.IsAuthError(err)` (HTTP Every call that can block takes a `context.Context` and aborts cleanly when it is cancelled, leaving nothing running in the background. A one-shot call (a subscribe, -a fetch, `RequestBroadcast`, `Resolve`, `WantedChanged`, `Used`/`Unused`, `Server.Accept`) aborts on +a fetch, `RequestBroadcast`, `Resolve`, `WantedChanged`, `Unwanted`, `Server.Accept`) aborts on its own and leaves the object usable; a stream read cancels the stream it reads. See the package doc for details. diff --git a/go/wrapper/doc.go b/go/wrapper/doc.go index 4728018d1a0..5008240a2c8 100644 --- a/go/wrapper/doc.go +++ b/go/wrapper/doc.go @@ -24,8 +24,8 @@ // owner. // // What a cancel tears down depends on the call. A one-shot call (a subscribe, a -// fetch, RequestBroadcast, Resolve, a TrackDemand's WantedChanged, a -// Fill's Unused, Server.Accept) aborts on its own and leaves the +// fetch, RequestBroadcast, Resolve, a TrackDemand's or Fill's WantedChanged, a +// Fill's Unwanted, Server.Accept) aborts on its own and leaves the // object it was made on usable, so the same // broadcast, producer, or server takes the next call. A stream read (any Next, // RecvGroup, ReadFrame, or the iterators over them) instead cancels the stream diff --git a/go/wrapper/moq_test.go b/go/wrapper/moq_test.go index 9f7358e239e..91a1985df4d 100644 --- a/go/wrapper/moq_test.go +++ b/go/wrapper/moq_test.go @@ -587,8 +587,8 @@ func TestFetchGroupAndServeDynamicMiss(t *testing.T) { if fill.StartGroup() != 7 || fill.EndGroup() != 8 || fill.Priority() != 11 { t.Fatalf("unexpected fill: start=%d end=%d priority=%d", fill.StartGroup(), fill.EndGroup(), fill.Priority()) } - if next := fill.Next(); next == nil || *next != 7 || !fill.IsUsed() { - t.Fatalf("unexpected fill: next=%v used=%v", next, fill.IsUsed()) + if wanted := fill.Wanted(); wanted == nil || *wanted != 7 { + t.Fatalf("unexpected fill: wanted=%v", wanted) } if !fill.IsWanted(7) || fill.IsWanted(8) { t.Fatalf("unexpected wants: 7=%v 8=%v", fill.IsWanted(7), fill.IsWanted(8)) diff --git a/go/wrapper/publish.go b/go/wrapper/publish.go index 9d7722985c9..0583c621a6c 100644 --- a/go/wrapper/publish.go +++ b/go/wrapper/publish.go @@ -331,11 +331,11 @@ func (d *TrackRequests) Cancel() { // Fill is a run of groups readers miss, for a TrackRequests to serve. // -// Accept each group you have and DropGroups the rest; Next names what is still +// Accept each group you have and DropGroups the rest; Wanted names what is still // owed, in the fill's order. Releasing the fill names whatever is left as dropped // while a reader still wants it; once nobody does, it is withdrawn, so stop -// serving it when Unused returns. Stop serving a single group when Unwanted -// returns for it, and move on to Next. +// serving it once Wanted is nil. Stop serving a single group when Unwanted +// returns for it, and move on to Wanted. type Fill struct { inner *ffi.MoqFill } @@ -371,9 +371,15 @@ func (f *Fill) Order() Order { return f.inner.Order() } -// Next is the next group still owed in the fill's order, or nil once every group is answered. -func (f *Fill) Next() *uint64 { - return f.inner.Next() +// Wanted is the next group still owed in the fill's order, or nil once every +// group is answered or no reader waits on the fill. +func (f *Fill) Wanted() *uint64 { + return f.inner.Wanted() +} + +// WantedChanged blocks until Wanted changes and returns it. The first call returns at once. +func (f *Fill) WantedChanged(ctx context.Context) (*uint64, error) { + return f.inner.WantedChanged(ctx) } // Accept inserts group sequence and returns a producer for it. The first group @@ -391,16 +397,6 @@ func (f *Fill) DropGroups(start, end uint64) { f.inner.DropGroups(start, end) } -// IsUsed reports whether a reader still waits on this fill. -func (f *Fill) IsUsed() bool { - return f.inner.IsUsed() -} - -// Unused blocks until no reader waits on this fill. -func (f *Fill) Unused(ctx context.Context) error { - return f.inner.Unused(ctx) -} - // IsWanted reports whether a reader still waits for group sequence from this fill: // false once every reader took it or narrowed past it, or nobody waits on the fill. func (f *Fill) IsWanted(sequence uint64) bool { diff --git a/js/e2ee/src/e2ee.test.ts b/js/e2ee/src/e2ee.test.ts index 9d620bf96a3..49b88769bf1 100644 --- a/js/e2ee/src/e2ee.test.ts +++ b/js/e2ee/src/e2ee.test.ts @@ -429,7 +429,7 @@ test("grouped failure wakes pending reads and releases demand", async () => { const good = await recvGroup(p.consumer); expect(await readText(good)).toBe("ok"); const forged = await recvGroup(p.consumer); - expect(badDemand.used.peek()).toBe(true); + expect(badDemand.wanted.peek()).toBe(true); // Sibling frame, next group, and datagram reads have nothing to return until the failure. const frame = good.readFrame(); @@ -442,8 +442,8 @@ test("grouped failure wakes pending reads and releases demand", async () => { expect(await failure(datagram)).toBe("authentication"); expect(demand.wanted.peek()).toBeUndefined(); - expect(badDemand.used.peek()).toBe(false); - expect(open.demand().used.peek()).toBe(false); + expect(badDemand.wanted.peek()).toBe(false); + expect(open.demand().wanted.peek()).toBe(false); expect(p.net.closed.peek()).toBeUndefined(); }); diff --git a/js/flate/src/stream/stream.test.ts b/js/flate/src/stream/stream.test.ts index b4999ff9993..8907a62bc2a 100644 --- a/js/flate/src/stream/stream.test.ts +++ b/js/flate/src/stream/stream.test.ts @@ -118,8 +118,8 @@ test("a second group is reported while the first is still open", async () => { // Both mirrors are released. The read that lost the race would otherwise stay registered on the // first group, keeping this consumer's subscription reachable after the caller drops it. - expect(first.demand().used.peek()).toBe(false); - expect(second.demand().used.peek()).toBe(false); + expect(first.demand().wanted.peek()).toBe(false); + expect(second.demand().wanted.peek()).toBe(false); // Sticky: a later read must not report the rest of the first group as a whole log. first.writeFrame({ payload: payloads(3)[2], timestamp: Time.Timestamp.now() }); diff --git a/js/hang/src/container/consumer.test.ts b/js/hang/src/container/consumer.test.ts index c3f21d7b4d4..a230c993b74 100644 --- a/js/hang/src/container/consumer.test.ts +++ b/js/hang/src/container/consumer.test.ts @@ -728,10 +728,10 @@ test("Consumer releases its open groups when an error ends it", async () => { const later = track.createGroup(1); later.writeFrame({ payload: new Uint8Array([0x01]), timestamp: Time.Timestamp.now() }); await open; - expect(later.demand().used.peek()).toBe(true); + expect(later.demand().wanted.peek()).toBe(true); await expect(consumer.next()).rejects.toThrow("malformed"); - expect(later.demand().used.peek()).toBe(false); + expect(later.demand().wanted.peek()).toBe(false); consumer.close(); }); diff --git a/js/json/src/stream/stream.test.ts b/js/json/src/stream/stream.test.ts index 48a825e7ce4..f862a40849a 100644 --- a/js/json/src/stream/stream.test.ts +++ b/js/json/src/stream/stream.test.ts @@ -91,8 +91,8 @@ test("a second group is reported while the first is still open", async () => { // Both mirrors are released. The read that lost the race would otherwise stay registered on the // first group, keeping this consumer's subscription reachable after the caller drops it. - expect(first.demand().used.peek()).toBe(false); - expect(second.demand().used.peek()).toBe(false); + expect(first.demand().wanted.peek()).toBe(false); + expect(second.demand().wanted.peek()).toBe(false); // Sticky: a later read must not report the rest of the first group as a whole log. first.writeFrame({ payload: encode({ n: 2 }), timestamp: Time.Timestamp.now() }); diff --git a/js/net/bench/broadcasts.ts b/js/net/bench/broadcasts.ts index 9e6825289b4..5fd9e294333 100644 --- a/js/net/bench/broadcasts.ts +++ b/js/net/bench/broadcasts.ts @@ -1,4 +1,4 @@ -/** Sweep observers against routes for one route update, and against tracks for one demand edge. */ +/** Sweep observers against routes for one route update, and against tracks for one listing change. */ import { Signal } from "@moq/signals"; import type { Grant } from "../src/auth.ts"; @@ -72,41 +72,6 @@ for (const scope of ["unscoped", "distinct-scopes", "producer-scopes", "rooted-p } if (checksum === 0) throw new Error("benchmark did no work"); -// Sweep both axes while only one track's demand changes. -const demandUpdates = 1024; -console.log("tracks,observers,demand_update_us"); -for (const trackCount of [8, 32, 128]) { - for (const observerCount of [1, 8, 32]) { - const broadcast = new BroadcastProducer(); - const tracks = Array.from({ length: trackCount }, (_, index) => broadcast.createTrack(`track-${index}`)); - const demand = broadcast.demand(); - let notifications = 0; - const disposes = Array.from({ length: observerCount }, () => - demand.used.subscribe(() => { - notifications++; - }), - ); - const start = performance.now(); - for (let index = 0; index < demandUpdates; index++) { - const subscriber = tracks[0].subscribe(); - await Promise.resolve(); - await Promise.resolve(); - if (!demand.used.peek()) throw new Error("missing demand"); - subscriber.close(); - await demand.unused(); - await Promise.resolve(); - } - if (notifications !== demandUpdates * 2 * observerCount) throw new Error(`lost demand edge: ${notifications}`); - console.log( - `${trackCount},${observerCount},${(((performance.now() - start) * 1000) / demandUpdates).toFixed(1)}`, - ); - for (const dispose of disposes) dispose(); - for (const track of tracks) track.finish(); - for (const track of tracks) track.close(); - broadcast.close(); - } -} - // One created and removed track reaches every listing, however many tracks the broadcast holds. const listed = 256; console.log("tracks,listings,create_remove_and_read_us"); diff --git a/js/net/bench/observed.ts b/js/net/bench/observed.ts index 27adbdbf015..104b44ce6a4 100644 --- a/js/net/bench/observed.ts +++ b/js/net/bench/observed.ts @@ -29,7 +29,7 @@ for (const mode of ["eager", "observed"] as const) ); }); await settle(); - const held = values.filter((value) => value.used.peek()).length; + const held = values.filter((value) => value.wanted.peek()).length; const samples: number[] = []; for (let round = 0; round < 6; round++) { const before = updates; diff --git a/js/net/src/broadcast.test.ts b/js/net/src/broadcast.test.ts index 821de20676e..ee48f21b303 100644 --- a/js/net/src/broadcast.test.ts +++ b/js/net/src/broadcast.test.ts @@ -125,7 +125,7 @@ test("createTrack answers a queued request", async () => { // No handler ever sees the request, and the lookup that opened it leaves the track open. expect(await pendingRequest(broadcast)).toBeUndefined(); subscriber.close(); - await broadcast.demand().unused(); + await unwanted(producer.demand()); expect(producer.closed.peek()).toBeUndefined(); // Later subscriptions read the created track, and the name stays taken. @@ -183,11 +183,11 @@ test("publishing-side subscriptions coalesce onto one request", async () => { expect(await a.readString()).toBe("hello"); expect(await b.readString()).toBe("hello"); await info; - // The lookup leaves the shared producer to its subscribers, and stops counting as demand. + // The lookup leaves the shared producer to its subscribers. expect(producer.closed.peek()).toBeUndefined(); a.close(); b.close(); - await broadcast.demand().unused(); + await unwanted(producer.demand()); expect(producer.closed.peek()).toBeUndefined(); producer.finish(); @@ -195,9 +195,8 @@ test("publishing-side subscriptions coalesce onto one request", async () => { broadcast.close(); }); -test("an info lookup joining a subscription's request stays demand after the subscriber leaves", async () => { +test("an info lookup joining a subscription's request is answered after the subscriber leaves", async () => { const broadcast = new BroadcastProducer(); - const demand = broadcast.demand(); const pulled = wireOf(broadcast).requested(); const subscriber = broadcast.track("media").subscribe(); const info = broadcast.track("media").info(); @@ -208,11 +207,9 @@ test("an info lookup joining a subscription's request stays demand after the sub subscriber.close(); await new Promise((resolve) => setTimeout(resolve, 0)); expect(request.demand().wanted.peek()).toBeUndefined(); - expect(demand.used.peek()).toBe(true); - request.accept({ timescale: Timescale.MILLI }); - await info; - await demand.unused(); + request.accept({ timescale: Timescale.MILLI, priority: 2 }); + expect((await info).priority).toBe(2); broadcast.close(); }); @@ -235,7 +232,6 @@ test("an info lookup releases a request nobody subscribed to", async () => { test("held info lookups keep a request until the last one lets go", async () => { const broadcast = new BroadcastProducer(); - const demand = broadcast.demand(); const pulled = wireOf(broadcast).requested(); const first = new AbortController(); const opened = wireOf(broadcast).resolveTrackInfo("media", first.signal); @@ -249,27 +245,22 @@ test("held info lookups keep a request until the last one lets go", async () => // The lookup that opened the request lets go first, under the one still holding it. first.abort(); expect(producer.closed.peek()).toBeUndefined(); - expect(demand.used.peek()).toBe(true); second.abort(); expect(producer.closed.peek()).toBeDefined(); - expect(demand.used.peek()).toBe(false); broadcast.close(); }); -test("an abandoned info lookup stops counting as demand, and its request goes once answered", async () => { +test("an abandoned info lookup's request goes once answered", async () => { const broadcast = new BroadcastProducer(); - const demand = broadcast.demand(); const pulled = wireOf(broadcast).requested(); const hold = new AbortController(); const info = wireOf(broadcast).resolveTrackInfo("media", hold.signal); const request = await pulled; if (!request) throw new Error("expected request"); - expect(demand.used.peek()).toBe(true); hold.abort(); await expect(info).rejects.toThrow(); - expect(demand.used.peek()).toBe(false); // The handler still gets an open track, and the answer lets it go. const producer = request.accept({ timescale: Timescale.MILLI }); @@ -278,45 +269,19 @@ test("an abandoned info lookup stops counting as demand, and its request goes on broadcast.close(); }); -test("a held info lookup on an inserted track counts as demand until it lets go", async () => { +test("a held info lookup on an inserted track leaves it open when it lets go", async () => { const broadcast = new BroadcastProducer(); - const demand = broadcast.demand(); const track = broadcast.createTrack("media", { timescale: Timescale.MILLI }); const hold = new AbortController(); await wireOf(broadcast).resolveTrackInfo("media", hold.signal); - expect(demand.used.peek()).toBe(true); hold.abort(); - expect(demand.used.peek()).toBe(false); expect(track.closed.peek()).toBeUndefined(); broadcast.close(); }); -test("a held info lookup ending after removeTrack leaves demand alone", async () => { - const broadcast = new BroadcastProducer(); - const demand = broadcast.demand(); - const track = broadcast.createTrack("media", { timescale: Timescale.MILLI }); - const subscriber = track.subscribe(); - const hold = new AbortController(); - await wireOf(broadcast).resolveTrackInfo("media", hold.signal); - expect(demand.used.peek()).toBe(true); - - broadcast.removeTrack("media"); - expect(demand.used.peek()).toBe(false); - hold.abort(); - expect(demand.used.peek()).toBe(false); - subscriber.close(); - await Promise.resolve(); - expect(demand.used.peek()).toBe(false); - - track.finish(); - track.close(); - broadcast.close(); -}); - test("removeTrack leaves a track that only a request serves", async () => { const broadcast = new BroadcastProducer(); - const demand = broadcast.demand(); const pulled = wireOf(broadcast).requested(); const subscriber = broadcast.track("media").subscribe(); const request = await pulled; @@ -324,7 +289,6 @@ test("removeTrack leaves a track that only a request serves", async () => { const producer = request.accept(); broadcast.removeTrack("media"); - expect(demand.used.peek()).toBe(true); expect(producer.closed.peek()).toBeUndefined(); subscriber.close(); @@ -713,59 +677,11 @@ test("aborting a fetch rejects with the signal's reason", async () => { broadcast.close(); }); -test("broadcast demand watches static and pending tracks rather than broadcast handles", async () => { +test("a pending track info query is answered by its request, and closing rejects it", async () => { const broadcast = new BroadcastProducer(); - const demand = broadcast.demand(); - const consumer = broadcast.consume(); - expect(demand.used.peek()).toBe(false); - const video = broadcast.createTrack("video", { timescale: Timescale.MILLI }); - const first = video.subscribe(); - await Promise.resolve(); - expect(demand.used.peek()).toBe(true); - const pulled = wireOf(broadcast).requested(); - const second = consumer.track("audio").subscribe(); - const request = await pulled; - if (!request) throw new Error("expected request"); - const requested = request.demand(); - expect(requested.wanted.peek()).toBeDefined(); - first.close(); - await Promise.resolve(); - expect(demand.used.peek()).toBe(true); - second.close(); - await unwanted(requested); - await demand.unused(); - expect(demand.used.peek()).toBe(false); - request.reject(); - video.finish(); - video.close(); - broadcast.close(); - expect(await demand.closed).toBeNull(); - consumer.close(); -}); - -test("broadcast closure clears active demand and subscriptions", async () => { - const broadcast = new BroadcastProducer(); - const track = broadcast.createTrack("video", { timescale: Timescale.MILLI }); - const subscriber = track.subscribe(); - const demand = broadcast.demand(); - await Promise.resolve(); - expect(demand.used.peek()).toBe(true); - broadcast.close(); - expect(demand.used.peek()).toBe(false); - // Like Rust, a closed broadcast rejects rather than reporting it unused. - await expect(demand.unused()).rejects.toThrow("broadcast is closed"); - subscriber.close(); - track.finish(); - track.close(); -}); - -test("a pending track info query counts as broadcast demand", async () => { - const broadcast = new BroadcastProducer(); - const demand = broadcast.demand(); const pending = wireOf(broadcast).requested(); const info = wireOf(broadcast).resolveTrackInfo("video"); - expect(demand.used.peek()).toBe(true); const request = await pending; if (!request) throw new Error("expected request"); @@ -773,30 +689,8 @@ test("a pending track info query counts as broadcast demand", async () => { expect(request.demand().wanted.peek()).toBeUndefined(); request.accept({ timescale: Timescale.MILLI, priority: 2 }); expect((await info).priority).toBe(2); - await demand.unused(); - expect(demand.used.peek()).toBe(false); - // Closing the broadcast ends a query's demand at once. const rejected = wireOf(broadcast).resolveTrackInfo("audio"); - expect(demand.used.peek()).toBe(true); broadcast.close(); - expect(demand.used.peek()).toBe(false); await expect(rejected).rejects.toThrow(); }); - -test("removeTrack stops counting the removed track's demand at once", async () => { - const broadcast = new BroadcastProducer(); - const demand = broadcast.demand(); - const video = broadcast.createTrack("video", { timescale: Timescale.MILLI }); - const subscriber = video.subscribe(); - await Promise.resolve(); - expect(demand.used.peek()).toBe(true); - - broadcast.removeTrack("video"); - expect(demand.used.peek()).toBe(false); - - subscriber.close(); - video.finish(); - video.close(); - broadcast.close(); -}); diff --git a/js/net/src/broadcast.ts b/js/net/src/broadcast.ts index e0fc0807a80..39c7f16a3ad 100644 --- a/js/net/src/broadcast.ts +++ b/js/net/src/broadcast.ts @@ -3,7 +3,7 @@ * * @module */ -import { type Dispose, type GetPromise, type Getter, Once, Signal } from "@moq/signals"; +import { type GetPromise, Once, Signal } from "@moq/signals"; import type * as Epoch from "./epoch.ts"; import { NotFound } from "./error.ts"; import type { Consumer as GroupConsumer } from "./group.ts"; @@ -30,19 +30,11 @@ export interface Announcer { let attachAnnouncer: (producer: Producer, announcer: Announcer) => void; let stampProducer: (producer: Producer, path: Path.Valid) => void; -// Info lookups pending or held on a track. Each is demand, as a pending track request is in Rust, -// though nobody subscribes. +// Info lookups pending or held on a track. class Lookups { pending = 0; // Whether a lookup opened the request, so the last lookup to end closes it unless subscribed. opened = false; - // Re-evaluates the track's demand at once; set by watchDemand. - changed = () => {}; - - add(delta: number): void { - this.pending += delta; - this.changed(); - } } /** Reactive backing state shared by broadcast producers and consumers. */ @@ -67,54 +59,10 @@ class BroadcastState { // A wire layer's broadcast, which has no producer: it ends once no consumer holds it, as a // Rust session retires a source nothing holds. A produced broadcast ends only on its close. unheld = false; - used = new Signal(false); - active = 0; - // The demand watcher of each open track, disposed when it closes or leaves the broadcast. - demands = new Map(); // Run synchronously once the broadcast closes, before `closed` notifies its observers. closers = new Set<() => void>(); } -// Each track updates the aggregate on an edge, so a demand change touches only its track. -// A track counts as demand while any `lookups` are pending, subscribed or not. -function watchDemand(state: BroadcastState, producer: track.Producer, lookups?: Lookups): void { - if (state.demands.has(producer)) return; - const demand = producer.demand(); - // A closed track is never demand, and its close already fired, so nothing would dispose a watcher. - if (hooks.demandReleased(demand).peek() !== undefined) return; - let active = false; - const update = () => { - const used = - state.closed.peek() === undefined && - hooks.demandReleased(demand).peek() === undefined && - ((lookups?.pending ?? 0) > 0 || demand.wanted.peek() !== undefined); - if (active === used) return; - state.active += used ? 1 : -1; - active = used; - state.used.set(state.active > 0); - }; - const disposeUsed = demand.wanted.subscribe(update); - if (lookups) lookups.changed = update; - const disposeClosed = hooks.demandReleased(demand).subscribe(() => { - if (hooks.demandReleased(demand).peek() === undefined) return; - cleanup(); - }); - const cleanup = () => { - disposeUsed(); - disposeClosed(); - // A removed track keeps its lookups, so a hold ending later must not count it again. - if (lookups?.changed === update) lookups.changed = () => {}; - if (active) { - state.active--; - active = false; - state.used.set(state.active > 0); - } - state.demands.delete(producer); - }; - state.demands.set(producer, cleanup); - update(); -} - function dequeueRequest(state: BroadcastState): track.Request | undefined { const requested = state.requested.peek(); requested.sort((a, b) => a.priority - b.priority); @@ -148,7 +96,6 @@ function closeState(state: BroadcastState) { if (state.closed.peek() !== undefined) return; state.closed.set(null); state.listing.close(); - for (const cleanup of state.demands.values()) cleanup(); for (const request of state.pending) request.reject(); state.requested.mutate((requests) => { requests.length = 0; @@ -199,7 +146,6 @@ function logical(state: BroadcastState, name: string): { producer: track.Produce const lookups = new Lookups(); state.lookups.set(producer, lookups); - watchDemand(state, producer, lookups); state.requests.set(name, producer); void hooks.trackReleased(producer).then(() => { if (state.requests.get(name) === producer) state.requests.delete(name); @@ -216,8 +162,7 @@ function subscribe(state: BroadcastState, name: string, options: track.Subscript return logical(state, name).producer.subscribe(options); } -// A pending lookup is demand, and so is a held one until `hold` aborts, which also abandons it before -// the answer. A request a lookup opened is let go once it is answered and nobody holds or subscribes +// A lookup is held until `hold` aborts, which also abandons it before the answer. A request a lookup opened is let go once it is answered and nobody holds or subscribes // to it; closing it sooner would hand its handler a closed producer. async function resolveTrackInfo(state: BroadcastState, name: string, hold?: AbortSignal): Promise { hold?.throwIfAborted(); @@ -225,7 +170,7 @@ async function resolveTrackInfo(state: BroadcastState, name: string, hold?: Abor const lookups = state.lookups.get(producer); if (!lookups) return producer.info(); if (requested) lookups.opened = true; - lookups.add(1); + lookups.pending++; const info = producer.info(); let answered = false; @@ -243,7 +188,7 @@ async function resolveTrackInfo(state: BroadcastState, name: string, hold?: Abor ); const end = () => { ended = true; - lookups.add(-1); + lookups.pending--; if (answered) release(); }; @@ -311,7 +256,6 @@ function takeQueued(state: BroadcastState, name: string): track.Request | undefi function adopt(state: BroadcastState, producer: track.Producer, info: track.Info): void { const lookups = state.lookups.get(producer) ?? new Lookups(); state.lookups.set(producer, lookups); - watchDemand(state, producer, lookups); state.tracks.set(producer.name, producer); state.listing.listOwn(producer.name, producer, info); @@ -323,42 +267,6 @@ function adopt(state: BroadcastState, producer: track.Producer, info: track.Info }); } -let makeDemand: (state: BroadcastState) => Demand; - -/** - * A watch-only view of demand for any track in a broadcast. - * - * @internal Tests only; nothing outside the package consumes it. - */ -export class Demand { - #state: BroadcastState; - private constructor(state: BroadcastState) { - this.#state = state; - } - static { - makeDemand = (state) => new Demand(state); - } - - /** Whether any track currently has subscribers, or a track info query is pending. */ - get used(): Getter { - return this.#state.used; - } - - /** Wait until every track becomes unused. Throws once the broadcast closes. */ - async unused(): Promise { - for (;;) { - if (this.#state.closed.peek() !== undefined) throw new Error("broadcast is closed"); - if (!this.#state.used.peek()) return; - await Signal.race(this.#state.used, this.#state.closed); - } - } - - /** The broadcast's clean close. */ - get closed(): GetPromise { - return this.#state.closed; - } -} - /** * The write side of a broadcast. * @@ -366,7 +274,6 @@ export class Demand { */ export class Producer { #state = new BroadcastState(); - #demand = makeDemand(this.#state); #announcer?: Announcer; #path = Path.empty(); @@ -406,15 +313,6 @@ export class Producer { return this.#state.closed; } - /** - * Watch demand for the broadcast's tracks. - * - * @internal Tests only; nothing outside the package consumes it. - */ - demand(): Demand { - return this.#demand; - } - /** 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 }); @@ -458,14 +356,13 @@ export class Producer { } /** - * Remove a statically inserted track, including a finished cached one: stop counting it - * toward the broadcast's demand and listing it at once. + * Remove a statically inserted track, including a finished cached one, and stop listing it + * at once. */ removeTrack(name: string): void { const track = this.#state.tracks.get(name); if (!track) return; this.#state.tracks.delete(name); - this.#state.demands.get(track)?.(); this.#state.listing.unlistOwn(name, track); } diff --git a/js/net/src/group.test.ts b/js/net/src/group.test.ts index 663fcb40d89..4b51b9e1381 100644 --- a/js/net/src/group.test.ts +++ b/js/net/src/group.test.ts @@ -82,39 +82,26 @@ test("zero group PTS keeps its scale and untimed groups stay unstamped", () => { } }); -test("used reflects mirror demand and unused resolves when the last reader leaves", async () => { +test("wanted reflects mirror demand until the last reader leaves", async () => { const producer = new Producer(0); // No mirror readers: no demand. - expect(producer.demand().used.peek()).toBe(false); + expect(producer.demand().wanted.peek()).toBe(false); const a = producer.mirror(); const b = producer.mirror(); - expect(producer.demand().used.peek()).toBe(true); + expect(producer.demand().wanted.peek()).toBe(true); - // Closing one of two keeps demand, so unused() stays pending. + // Closing one of two keeps demand. a.close(); - expect(producer.demand().used.peek()).toBe(true); + expect(producer.demand().wanted.peek()).toBe(true); - // Closing the last reader drops demand; unused() resolves. Fetch coalescing awaits this to - // cancel a download that everyone has abandoned (a group may never end on its own). + // Closing the last reader drops demand. Fetch coalescing watches this to cancel a download + // that everyone has abandoned (a group may never end on its own). b.close(); - await producer.demand().unused(); - expect(producer.demand().used.peek()).toBe(false); -}); - -test("unused rejects once the group closes, even while read", async () => { - const producer = new Producer(0); - const reader = producer.mirror(); - const pending = producer.demand().unused(); - const abort = new Error("boom"); - producer.abort(abort); - await expect(pending).rejects.toBe(abort); - - const finished = new Producer(1); - finished.finish(); - await expect(finished.demand().unused()).rejects.toThrow("group is closed"); - reader.close(); + const wanted = producer.demand().wanted; + while (wanted.peek()) await wanted.changed(); + expect(wanted.peek()).toBe(false); }); function pair(sequence: number) { diff --git a/js/net/src/group.ts b/js/net/src/group.ts index 155034bb8f9..fdddb370f5f 100644 --- a/js/net/src/group.ts +++ b/js/net/src/group.ts @@ -117,7 +117,7 @@ function appendFrame(state: GroupState, frame: Frame) { for (const fn of state.progress) fn(state.total.peek()); } -let makeDemand: (sequence: number, used: Getter, state: GroupState) => Demand; +let makeDemand: (sequence: number, wanted: Getter, state: GroupState) => Demand; /** * A watch-only view of a group's mirror readers, including coalesced fetches. @@ -127,32 +127,22 @@ let makeDemand: (sequence: number, used: Getter, state: GroupState) => export class Demand { /** The group sequence this handle watches. */ readonly sequence: number; - #used: Getter; + #wanted: Getter; #state: GroupState; - private constructor(sequence: number, used: Getter, state: GroupState) { + private constructor(sequence: number, wanted: Getter, state: GroupState) { this.sequence = sequence; - this.#used = used; + this.#wanted = wanted; this.#state = state; } static { - makeDemand = (sequence, used, state) => new Demand(sequence, used, state); + makeDemand = (sequence, wanted, state) => new Demand(sequence, wanted, state); } /** Whether any mirror reader is attached. */ - get used(): Getter { - return this.#used; - } - - /** Wait until all mirror readers leave. Throws once the group closes: its abort error, or a clean-close error. */ - async unused(): Promise { - for (;;) { - const closed = this.#state.closed.peek(); - if (closed !== undefined) throw closed ?? new Error("group is closed"); - if (!this.#used.peek()) return; - await Signal.race(this.#used, this.#state.closed); - } + get wanted(): Getter { + return this.#wanted; } /** The group's clean close or abort error. */ @@ -173,10 +163,10 @@ export class Producer { #state: GroupState; #mirrors?: Set; - // Whether any mirror reader is attached (see {@link Demand.used}). Fetch coalescing watches it to + // Whether any mirror reader is attached (see {@link Demand.wanted}). Fetch coalescing watches it to // cancel an abandoned download; a group can stay open indefinitely (a catalog or JSON stream), // so this is what stops a reader-less fetch instead of the stream ending on its own. - #used = new Signal(false); + #wanted = new Signal(false); #demand: Demand; // Exclusive cap on the frame count, set when the track ends inside this group: a write @@ -194,7 +184,7 @@ export class Producer { constructor(sequence: number) { this.#state = new GroupState(sequence); this.sequence = sequence; - this.#demand = makeDemand(sequence, this.#used, this.#state); + this.#demand = makeDemand(sequence, this.#wanted, this.#state); } static { @@ -320,14 +310,14 @@ export class Producer { this.#mirrors ??= new Set(); this.#mirrors.add(dst); - this.#used.set(true); + this.#wanted.set(true); // Track this mirror's close eagerly (not just lazily on the next write) so demand drops the // moment the last reader leaves, letting an abandoned fetch cancel even with no more frames. const dispose = dst.closed.subscribe((c) => { if (c === undefined) return; this.#mirrors?.delete(dst); - this.#used.set((this.#mirrors?.size ?? 0) > 0); + this.#wanted.set((this.#mirrors?.size ?? 0) > 0); dispose(); }); diff --git a/js/net/src/ietf/publisher.test.ts b/js/net/src/ietf/publisher.test.ts index a9fe499c6c9..0a84d8c55b5 100644 --- a/js/net/src/ietf/publisher.test.ts +++ b/js/net/src/ietf/publisher.test.ts @@ -14,6 +14,7 @@ import * as Path from "../path.ts"; import { Reader, Stream, Writer } from "../stream.ts"; import { Timescale, Timestamp } from "../time.ts"; import type { Producer as TrackProducer } from "../track.ts"; +import { unwanted } from "../util/demand.ts"; import { wireOf } from "../wire.ts"; import type * as Cluster from "./cluster.ts"; import { ObjectDatagram } from "./datagram.ts"; @@ -417,12 +418,14 @@ test("a cancelled TRACK_STATUS releases its demand on every draft", async () => const served = new BroadcastProducer(); const trackRequest = wireOf(served).requested(); hooks.acceptRequest(request, served); - expect((await trackRequest)?.name).toBe("video"); - expect(served.demand().used.peek()).toBe(true); + const track = await trackRequest; + expect(track?.name).toBe("video"); late.cancel(); await looking; expect(late.written).toEqual([]); - await served.demand().unused(); + // The withdrawn lookup no longer holds the track, so the answer lets it go. + const producer = track?.accept({ timescale: Timescale.MILLI }); + await producer?.closed; await requests.return?.(); served.close(); @@ -467,12 +470,14 @@ test("a SUBSCRIBE withdrawn during setup releases its demand on every draft", as const served = new BroadcastProducer(); const trackRequest = wireOf(served).requested(); hooks.acceptRequest(request, served); - expect((await trackRequest)?.name).toBe("video"); - expect(served.demand().used.peek()).toBe(true); + const track = await trackRequest; + if (!track) throw new Error("no request for video"); + expect(track.name).toBe("video"); + expect(track.demand().wanted.peek()).toBeDefined(); late.withdraw(); await subscribing; expect(late.written).toEqual([]); - await served.demand().unused(); + await unwanted(track.demand()); await requests.return?.(); served.close(); diff --git a/js/net/src/ietf/subscriber.ts b/js/net/src/ietf/subscriber.ts index 9e7aa82e3ba..41d7cb97190 100644 --- a/js/net/src/ietf/subscriber.ts +++ b/js/net/src/ietf/subscriber.ts @@ -703,9 +703,9 @@ export class Subscriber { async #refuseListing(lister: Lister): Promise { try { for (;;) { - while (!lister.used.peek()) { + while (!lister.wanted.peek()) { if (lister.closed.peek() !== undefined) return; - await race([Signal.race(lister.used, lister.closed)]); + await race([Signal.race(lister.wanted, lister.closed)]); } lister.abort(new Error("track listing requires moq-lite-07 or newer")); } diff --git a/js/net/src/integration.test.ts b/js/net/src/integration.test.ts index 7d65dcf01e0..20ebd7d57c5 100644 --- a/js/net/src/integration.test.ts +++ b/js/net/src/integration.test.ts @@ -29,6 +29,7 @@ import * as Path from "./path.ts"; import { Stream } from "./stream.ts"; import { Timescale, Timestamp } from "./time.ts"; import type { Position, Producer as TrackProducer } from "./track.ts"; +import { unwanted } from "./util/demand.ts"; import { withTimeout } from "./util/timeout.ts"; import { wireOf } from "./wire.ts"; @@ -457,16 +458,25 @@ test.each([Lite.ALPN_05, Lite.ALPN_06, Lite.ALPN_07_WIP])( ]); const broadcast = publish(origin, Path.from("test")); - const demand = broadcast.demand(); + // Whether anyone subscribes to the served track, recorded on each change. const edges: boolean[] = []; - const dispose = demand.used.subscribe((used) => edges.push(used)); + let producer: TrackProducer | undefined; + let dispose = () => {}; let requests = 0; const serving = (async () => { for (;;) { const request = await wireOf(broadcast).requested(); if (!request) return; requests++; - request.accept({ timescale: Timescale.MILLI }).writeFrame(textFrame("hello")); + producer = request.accept({ timescale: Timescale.MILLI }); + const wanted = producer.demand().wanted; + const edge = () => { + const now = wanted.peek() !== undefined; + if (edges.at(-1) !== now) edges.push(now); + }; + edge(); + dispose = wanted.subscribe(edge); + producer.writeFrame(textFrame("hello")); } })(); @@ -474,12 +484,17 @@ test.each([Lite.ALPN_05, Lite.ALPN_06, Lite.ALPN_07_WIP])( const track = remote.track("video").subscribe().ordered(); expect(await track.readString()).toBe("hello"); await sleep(50); - expect(edges).toEqual([true]); + // One request, wanted from its subscription on and never let go in between. + if (!producer) throw new Error("the track was never answered"); + const served = producer; expect(requests).toBe(1); + expect(edges.filter((edge) => edge)).toEqual([true]); + expect(edges.at(-1)).toBe(true); track.close(); - await withTimeout(demand.unused(), Nano.fromMilli(Milli(1000)), "demand outlived the reader"); - expect(edges).toEqual([true, false]); + await withTimeout(unwanted(served.demand()), Nano.fromMilli(Milli(1000)), "demand outlived the reader"); + expect(edges.at(-1)).toBe(false); + expect(edges.filter((edge) => edge)).toEqual([true]); dispose(); remote.close(); @@ -1449,12 +1464,12 @@ test("integration: lite fetch teardown when the reader abandons an open group", expect(await fetched.readString()).toBe("hello"); // The publisher is now serving the still-open group. - await waitUntil(() => group.demand().used.peek()); + await waitUntil(() => group.demand().wanted.peek()); // Abandoning the fetch cancels the FETCH stream, so the publisher stops serving instead of // pumping an open group to a reader that left. fetched.close(); - await waitUntil(() => !group.demand().used.peek()); + await waitUntil(() => !group.demand().wanted.peek()); group.finish(); broadcast.close(); @@ -1557,17 +1572,17 @@ test("integration: lite coalesced fetch stays until every reader abandons the op const f2 = await remote.track("video").fetchGroup(group.sequence); expect(await f1.readString()).toBe("hello"); expect(await f2.readString()).toBe("hello"); - await waitUntil(() => group.demand().used.peek()); + await waitUntil(() => group.demand().wanted.peek()); // Closing one coalesced reader keeps the shared FETCH flowing for the other. f1.close(); group.writeFrame(textFrame("more")); expect(await f2.readString()).toBe("more"); - expect(group.demand().used.peek()).toBe(true); + expect(group.demand().wanted.peek()).toBe(true); // The last abandon cancels the FETCH. f2.close(); - await waitUntil(() => !group.demand().used.peek()); + await waitUntil(() => !group.demand().wanted.peek()); group.finish(); broadcast.close(); diff --git a/js/net/src/listing.test.ts b/js/net/src/listing.test.ts index a74c0f92f77..9eedb173533 100644 --- a/js/net/src/listing.test.ts +++ b/js/net/src/listing.test.ts @@ -289,10 +289,10 @@ test("readers wait for the lister to sync", async () => { const broadcast = new BroadcastProducer(); broadcast.createTrack("own"); const lister = wireOf(broadcast).lister(); - expect(lister.used.peek()).toBe(false); + expect(lister.wanted.peek()).toBe(false); const tracks = list(broadcast); - expect(lister.used.peek()).toBe(true); + expect(lister.wanted.peek()).toBe(true); expect(await tracks.drain()).toEqual(["+own"]); lister.add("up", { priority: 0 }); expect(() => lister.add("up", { priority: 0 })).toThrow("duplicate"); @@ -304,7 +304,7 @@ test("readers wait for the lister to sync", async () => { expect(await tracks.drain()).toEqual(["-up"]); tracks.tracks.close(); - expect(lister.used.peek()).toBe(false); + expect(lister.wanted.peek()).toBe(false); }); test("a name listed by both is reported once", async () => { @@ -328,16 +328,16 @@ test("an aborted answer fails its readers and a later reader asks again", async lister.add("up", { priority: 0 }); lister.abort(new NotFound("broadcast")); expect(await tracks.drain()).toEqual([`error: ${StreamCode.NotFound}`]); - expect(lister.used.peek()).toBe(false); + expect(lister.wanted.peek()).toBe(false); const again = list(broadcast); - expect(lister.used.peek()).toBe(true); + expect(lister.wanted.peek()).toBe(true); expect(await again.drain()).toEqual([]); lister.synced(); expect(await again.drain()).toEqual(["synced"]); // The failed reader leaving releases nothing. tracks.tracks.close(); - expect(lister.used.peek()).toBe(true); + expect(lister.wanted.peek()).toBe(true); }); test("a closed lister withdraws its answer and syncs", async () => { @@ -370,7 +370,7 @@ test("an owner lists each broadcast once", async () => { expect(await newer.drain()).toEqual(["+b"]); newer.tracks.close(); stranger.tracks.close(); - expect(wireOf(broadcast).lister().used.peek()).toBe(false); + expect(wireOf(broadcast).lister().wanted.peek()).toBe(false); }); test("a broadcast has one lister", () => { @@ -390,7 +390,7 @@ test("an ended answer ends its readers and a later reader asks again", async () expect(await tracks.drain()).toEqual(["+up", "synced"]); lister.end(); expect(await tracks.drain()).toEqual(["end"]); - expect(lister.used.peek()).toBe(false); + expect(lister.wanted.peek()).toBe(false); const again = list(broadcast); expect(await again.drain()).toEqual([]); diff --git a/js/net/src/listing.ts b/js/net/src/listing.ts index 05f612d0533..1a02c95837c 100644 --- a/js/net/src/listing.ts +++ b/js/net/src/listing.ts @@ -173,7 +173,7 @@ export class Listing { #lister = false; /** Whether the lister's answer has synced. */ #answered = false; - #used = new Signal(false); + #wanted = new Signal(false); #closed: GetPromise; constructor(closed: GetPromise) { @@ -181,8 +181,8 @@ export class Listing { } /** Whether anyone lists the broadcast. */ - get used(): Getter { - return this.#used; + get wanted(): Getter { + return this.#wanted; } /** Settles once the broadcast ended. */ @@ -195,7 +195,7 @@ export class Listing { } #counted() { - this.#used.set(this.#readers.size > 0); + this.#wanted.set(this.#readers.size > 0); } /** Re-evaluate whether the listing has synced, telling readers when it changed. */ @@ -416,8 +416,8 @@ export class Tracks { * session lists its peer's. A broadcast has at most one. * * While it exists, readers wait for it to {@link synced} before reporting `synced`. Answer - * while {@link used}: list what the publisher behind it holds and keep it current, then - * {@link clear} once unused. {@link close} withdraws what it listed. + * while {@link wanted}: list what the publisher behind it holds and keep it current, then + * {@link clear} once unwanted. {@link close} withdraws what it listed. */ export class Lister { #listing: Listing; @@ -430,8 +430,8 @@ export class Lister { } /** Whether anyone lists the broadcast. */ - get used(): Getter { - return this.#listing.used; + get wanted(): Getter { + return this.#listing.wanted; } /** Settles once the broadcast ended, as every listing of it does. */ diff --git a/js/net/src/lite/connection.ts b/js/net/src/lite/connection.ts index f1842f4871a..7c38655eea6 100644 --- a/js/net/src/lite/connection.ts +++ b/js/net/src/lite/connection.ts @@ -281,7 +281,7 @@ export class Connection implements Established { } const probes = new Effect((effect) => { - if (!effect.get(this.#probe.used)) return; + if (!effect.get(this.#probe.wanted)) return; effect.spawn(() => this.#subscriber.runProbe(effect.abort)); }); diff --git a/js/net/src/lite/subscriber.ts b/js/net/src/lite/subscriber.ts index 7b3debc7861..0683f19ed3d 100644 --- a/js/net/src/lite/subscriber.ts +++ b/js/net/src/lite/subscriber.ts @@ -690,8 +690,8 @@ export class Subscriber { try { for (;;) { if (lister.closed.peek() !== undefined || this.#closed.signal.aborted) return; - if (!lister.used.peek()) { - await untilAborted(race([Signal.race(lister.used, lister.closed)]), this.#closed.signal).catch( + if (!lister.wanted.peek()) { + await untilAborted(race([Signal.race(lister.wanted, lister.closed)]), this.#closed.signal).catch( () => {}, ); continue; @@ -727,14 +727,14 @@ export class Subscriber { ?.ready(broadcast, "subscribe", { signal: setup, until: async () => { - while (lister.used.peek() && lister.closed.peek() === undefined) { - await Signal.race(lister.used, lister.closed); + while (lister.wanted.peek() && lister.closed.peek() === undefined) { + await Signal.race(lister.wanted, lister.closed); } }, }) .catch(() => false); // The readers may have left while the wait settled, before the unused watch below is armed. - if (ready === false || setup.aborted || !lister.used.peek()) return; + if (ready === false || setup.aborted || !lister.wanted.peek()) return; // Armed before the first check and held to the end, so a shrink is never missed. const refused = unauthorized(broadcast); @@ -755,8 +755,8 @@ export class Subscriber { const listed = new Set(); const unused = new AbortController(); const stop = AbortSignal.any([unused.signal, revoked.signal, this.#closed.signal]); - const disposeUsed = lister.used.subscribe((used) => { - if (!used) unused.abort(new StreamError(StreamCode.Cancel, { message: "listing unused" })); + const disposeWanted = lister.wanted.subscribe((wanted) => { + if (!wanted) unused.abort(new StreamError(StreamCode.Cancel, { message: "listing unused" })); }); let stream: Stream | undefined; try { @@ -826,7 +826,7 @@ export class Subscriber { lister.abort(e); } finally { disposeGrant?.(); - disposeUsed(); + disposeWanted(); this.#announces.release(listed.size); } } @@ -1283,10 +1283,10 @@ export class Subscriber { while ( (await this.#auth?.ready(broadcast, "subscribe", { signal: consumed.valid, - until: () => group.demand().unused(), + until: () => groupUnwanted(group), })) === false ) { - if (group.closed.peek() === undefined && group.demand().used.peek()) continue; + if (group.closed.peek() === undefined && group.demand().wanted.peek()) continue; throw new StreamError(StreamCode.Cancel, { message: "cancel" }); } // Arm after admission, before checking: another unanswered token may still @@ -1382,17 +1382,11 @@ export class Subscriber { // Serve until the stream FINs, the group closes, or every reader leaves. A group can // stay open indefinitely (a catalog or JSON stream), so an abandoned fetch is stopped by - // demand, not by the stream ending. `unused` is watched across frames as one stable + // demand, not by the stream ending. `idling` is watched across frames as one stable // promise; the check is level-triggered, so a coalesced fetch that arrives before we // cancel re-arms and resumes. const idle: unique symbol = Symbol("idle"); - let unused = group - .demand() - .unused() - .then( - (): typeof idle => idle, - (): typeof idle => idle, - ); + let idling = groupUnwanted(group).then((): typeof idle => idle); // A decode consumes its frame whenever it lands, so one outstanding across a re-arm is // kept and awaited again rather than abandoned with its frame. let pending: Promise | undefined; @@ -1401,17 +1395,11 @@ export class Subscriber { let frame = pending === undefined ? stream.reader.tryDecode(decode) : undefined; if (!frame) { pending ??= stream.reader.decodeMaybe(decode); - const next = await race([pending, group.closed, unused]); + const next = await race([pending, group.closed, idling]); if (next === idle) { if (group.isClosed) break; - if (group.demand().used.peek()) { - unused = group - .demand() - .unused() - .then( - (): typeof idle => idle, - (): typeof idle => idle, - ); + if (group.demand().wanted.peek()) { + idling = groupUnwanted(group).then((): typeof idle => idle); continue; } // Abandoned mid-group: the truncated group must never end clean. @@ -1876,26 +1864,22 @@ async function untilClosed(group: netGroup.Producer, step: Promise): Promi return value as T; } +// Resolve once no reader wants `group`, or it closed. +async function groupUnwanted(group: netGroup.Producer): Promise { + const demand = group.demand(); + while (demand.wanted.peek() && demand.closed.peek() === undefined) { + await Signal.race(demand.wanted, demand.closed); + } +} + // Like untilClosed, but also cancels once every reader has left. Demand is level-triggered, so a // caller that coalesces onto the group before the check re-arms it. async function untilAbandoned(group: netGroup.Producer, step: Promise): Promise { const idle: unique symbol = Symbol("idle"); for (;;) { - const value = await untilClosed( - group, - race([ - step, - group - .demand() - .unused() - .then( - (): typeof idle => idle, - (): typeof idle => idle, - ), - ]), - ); + const value = await untilClosed(group, race([step, groupUnwanted(group).then((): typeof idle => idle)])); if (value !== idle) return value as T; - if (!group.demand().used.peek()) { + if (!group.demand().wanted.peek()) { // Close here rather than where the error lands, so no fetch coalesces onto the group // in between only to fail with it. const err = new StreamError(StreamCode.Cancel, { message: "cancel" }); diff --git a/js/net/src/lite/track_stream.test.ts b/js/net/src/lite/track_stream.test.ts index 97bdb20ac3f..353239fd2dc 100644 --- a/js/net/src/lite/track_stream.test.ts +++ b/js/net/src/lite/track_stream.test.ts @@ -10,6 +10,7 @@ import { Producer as OriginProducer } from "../origin.ts"; import * as Path from "../path.ts"; import { Stream } from "../stream.ts"; import { Timescale } from "../time.ts"; +import type { Producer as TrackProducer } from "../track.ts"; import { wireOf } from "../wire.ts"; import { Fetch } from "./fetch.ts"; import { Publisher } from "./publisher.ts"; @@ -121,12 +122,12 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const broadcastPublication = origin.createBroadcast(Path.from("room")); const broadcast = broadcastPublication.broadcast; broadcastPublication.announce(); - const demand = broadcast.demand(); + let producer: TrackProducer | undefined; const serving = (async () => { for (;;) { const request = await wireOf(broadcast).requested(); if (!request) return; - request.accept({ timescale: Timescale.MILLI }); + producer = request.accept({ timescale: Timescale.MILLI }); } })(); @@ -137,11 +138,12 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver await TrackInfo.decode(client.reader, version); expect(await client.reader.done()).toBe(true); - expect(await settlesWithin(demand.unused(), 50)).toBe(false); + if (!producer) throw new Error("the track was never answered"); + expect(await settlesWithin(Promise.resolve(producer.closed), 50)).toBe(false); if (close === "FIN") client.writer.close(); else client.writer.reset(new Error("gone")); - expect(await settlesWithin(demand.unused(), 1000)).toBe(true); + expect(await settlesWithin(Promise.resolve(producer.closed), 1000)).toBe(true); publisher.close(); broadcast.close(); @@ -159,12 +161,12 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const broadcastPublication = origin.createBroadcast(Path.from("room")); const broadcast = broadcastPublication.broadcast; broadcastPublication.announce(); - const demand = broadcast.demand(); + let producer: TrackProducer | undefined; const serving = (async () => { for (;;) { const request = await wireOf(broadcast).requested(); if (!request) return; - request.accept({ timescale: Timescale.MILLI }); + producer = request.accept({ timescale: Timescale.MILLI }); } })(); @@ -186,11 +188,12 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver void publisher.runTrackInfo(new TrackMessage(Path.from("room"), "video"), server); await writing; - expect(demand.used.peek()).toBe(true); + if (!producer) throw new Error("the track was never answered"); + expect(producer.closed.peek()).toBeUndefined(); if (close === "FIN") client.writer.close(); else client.writer.reset(new Error("gone")); - expect(await settlesWithin(demand.unused(), 1000)).toBe(true); + expect(await settlesWithin(Promise.resolve(producer.closed), 1000)).toBe(true); publisher.close(); broadcast.close(); @@ -206,16 +209,15 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const broadcastPublication = origin.createBroadcast(Path.from("room")); const broadcast = broadcastPublication.broadcast; broadcastPublication.announce(); - const demand = broadcast.demand(); - let requests = 0; + const producers: TrackProducer[] = []; const serving = (async () => { for (;;) { const request = await wireOf(broadcast).requested(); if (!request) return; - requests++; - request.accept({ timescale: Timescale.MILLI }); + producers.push(request.accept({ timescale: Timescale.MILLI })); } })(); + const released = (index: number) => Promise.resolve(producers[index].closed); const open = async () => { const client = await Stream.open(pair.client, { version }); @@ -228,13 +230,13 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const second = await open(); await TrackInfo.decode(first.reader, version); await TrackInfo.decode(second.reader, version); - expect(requests).toBe(1); + expect(producers.length).toBe(1); first.writer.close(); - expect(await settlesWithin(demand.unused(), 50)).toBe(false); + expect(await settlesWithin(released(0), 50)).toBe(false); second.writer.close(); - expect(await settlesWithin(demand.unused(), 1000)).toBe(true); + expect(await settlesWithin(released(0), 1000)).toBe(true); publisher.close(); broadcast.close(); @@ -249,16 +251,15 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const broadcastPublication = origin.createBroadcast(Path.from("room")); const broadcast = broadcastPublication.broadcast; broadcastPublication.announce(); - const demand = broadcast.demand(); - let requests = 0; + const producers: TrackProducer[] = []; const serving = (async () => { for (;;) { const request = await wireOf(broadcast).requested(); if (!request) return; - requests++; - request.accept({ timescale: Timescale.MILLI }); + producers.push(request.accept({ timescale: Timescale.MILLI })); } })(); + const released = (index: number) => Promise.resolve(producers[index].closed); const open = async () => { const client = await Stream.open(pair.client, { version }); @@ -270,15 +271,15 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const first = await open(); await TrackInfo.decode(first.reader, version); first.writer.close(); - expect(await settlesWithin(demand.unused(), 1000)).toBe(true); + expect(await settlesWithin(released(0), 1000)).toBe(true); const second = await open(); await TrackInfo.decode(second.reader, version); - expect(requests).toBe(2); - expect(await settlesWithin(demand.unused(), 50)).toBe(false); + expect(producers.length).toBe(2); + expect(await settlesWithin(released(1), 50)).toBe(false); second.writer.close(); - expect(await settlesWithin(demand.unused(), 1000)).toBe(true); + expect(await settlesWithin(released(1), 1000)).toBe(true); publisher.close(); broadcast.close(); @@ -309,8 +310,7 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const broadcastPublication = origin.createBroadcast(Path.from("room")); const broadcast = broadcastPublication.broadcast; broadcastPublication.announce(); - const demand = broadcast.demand(); - // Answered by hand, after every requester left. + // Answered by hand, once every requester has left. const requested = wireOf(broadcast).requested(); const first = await requester(pair, publisher, kind); @@ -319,14 +319,10 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const second = await requester(pair, publisher, kind); first.writer.reset(new Error("gone")); - expect(await settlesWithin(demand.unused(), 50)).toBe(false); - second.writer.reset(new Error("gone")); - expect(await settlesWithin(demand.unused(), 1000)).toBe(true); // The handler still gets an open track, and the answer lets it go. const producer = request.accept({ timescale: Timescale.MILLI }); - expect(producer.closed.peek()).toBeUndefined(); expect(await settlesWithin(Promise.resolve(producer.closed), 1000)).toBe(true); publisher.close(); @@ -342,18 +338,17 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const broadcastPublication = origin.createBroadcast(Path.from("room")); const broadcast = broadcastPublication.broadcast; broadcastPublication.announce(); - const demand = broadcast.demand(); const requested = wireOf(broadcast).requested(); await requester(pair, publisher, kind); const request = await requested; if (!request) throw new Error("the publisher never asked for the track"); - expect(demand.used.peek()).toBe(true); pair.server.close(); - expect(await settlesWithin(demand.unused(), 1000)).toBe(true); + // The handler still gets an open track, and the answer lets it go. + const producer = request.accept({ timescale: Timescale.MILLI }); + expect(await settlesWithin(Promise.resolve(producer.closed), 1000)).toBe(true); - request.reject(); publisher.close(); broadcast.close(); origin.close(); @@ -366,7 +361,6 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const broadcastPublication = origin.createBroadcast(Path.from("room")); const broadcast = broadcastPublication.broadcast; broadcastPublication.announce(); - const demand = broadcast.demand(); // Answered by hand, after the first TRACK stream has left. const requested = wireOf(broadcast).requested(); @@ -374,18 +368,16 @@ describe.each([Version.DRAFT_05, Version.DRAFT_06, Version.DRAFT_07])("%s", (ver const request = await requested; if (!request) throw new Error("the publisher never asked for the track"); first.writer.reset(new Error("gone")); - expect(await settlesWithin(demand.unused(), 1000)).toBe(true); // The next TRACK stream joins the request still waiting on the application. const second = await requester(pair, publisher, "TRACK"); - expect(demand.used.peek()).toBe(true); expect(await settlesWithin(wireOf(broadcast).requested(), 50)).toBe(false); - request.accept({ timescale: Timescale.MILLI }); + const producer = request.accept({ timescale: Timescale.MILLI }); await TrackInfo.decode(second.reader, version); - expect(await settlesWithin(demand.unused(), 50)).toBe(false); + expect(await settlesWithin(Promise.resolve(producer.closed), 50)).toBe(false); second.writer.close(); - expect(await settlesWithin(demand.unused(), 1000)).toBe(true); + expect(await settlesWithin(Promise.resolve(producer.closed), 1000)).toBe(true); publisher.close(); broadcast.close(); diff --git a/js/net/src/resume.test.ts b/js/net/src/resume.test.ts index aa9b7df0a2b..bfc1da6bdc5 100644 --- a/js/net/src/resume.test.ts +++ b/js/net/src/resume.test.ts @@ -1740,14 +1740,18 @@ test("a route superseded while its properties are checked stops being asked", as const sub = front.consumer.track("seq").subscribe(); await sub.info(); // b never answers what the track is, so its splice waits on it. - void wireOf(b).requested(); + const pulled = wireOf(b).requested(); front.serve(b.consume()); await settle(); - expect(b.demand().used.peek()).toBe(true); + const request = await pulled; + if (!request) throw new Error("b was never asked"); front.serve(c.consume()); await settle(); - expect(b.demand().used.peek()).toBe(false); + // Nobody asks b any more, so its late answer is let go. + const late = request.accept(); + await settle(); + expect(late.closed.peek()).toBeDefined(); sub.close(); front.consumer.close(); diff --git a/js/net/src/resume.ts b/js/net/src/resume.ts index 440e8a43b00..313ad6beb28 100644 --- a/js/net/src/resume.ts +++ b/js/net/src/resume.ts @@ -1488,7 +1488,7 @@ export class Front { next?: Promise<{ event?: TracksEvent; err?: Error }>; } | undefined; - const wait = () => race([Signal.race(lister.used, this.#changed), closed]); + const wait = () => race([Signal.race(lister.wanted, this.#changed), closed]); const stop = () => { listing?.reader?.close(); listing = undefined; @@ -1512,7 +1512,7 @@ export class Front { try { for (;;) { if (closed.peek() !== undefined) return; - if (!lister.used.peek()) { + if (!lister.wanted.peek()) { if (listing) { stop(); lister.clear(); diff --git a/js/net/src/util/observed.test.ts b/js/net/src/util/observed.test.ts index 58e03b49ca7..8dcad772018 100644 --- a/js/net/src/util/observed.test.ts +++ b/js/net/src/util/observed.test.ts @@ -9,20 +9,20 @@ const flush = async () => { test("peeking holds no demand and only the last observer releases it", () => { const value = new Observed(1); expect(value.peek()).toBe(1); - expect(value.used.peek()).toBe(false); + expect(value.wanted.peek()).toBe(false); const first = value.subscribe(() => {}); const second = value.subscribe(() => {}); first(); first(); - expect(value.used.peek()).toBe(true); + expect(value.wanted.peek()).toBe(true); second(); - expect(value.used.peek()).toBe(false); + expect(value.wanted.peek()).toBe(false); }); test("reactive one-shot observers preserve demand across a value update", async () => { const value = new Observed(1); const demand: boolean[] = []; - const stop = value.used.subscribe((used) => demand.push(used)); + const stop = value.wanted.subscribe((wanted) => demand.push(wanted)); const effect = new Effect((effect) => { effect.get(value); }); @@ -42,20 +42,20 @@ test("forwarded demand switches sources and releases its old source", async () = const source = new Signal | undefined>(first); const follow = new Follow(source); expect(follow.peek()).toBe(1); - expect(first.used.peek()).toBe(false); + expect(first.wanted.peek()).toBe(false); const seen: (number | undefined)[] = []; const stop = follow.subscribe((value) => seen.push(value)); - expect(first.used.peek()).toBe(true); + expect(first.wanted.peek()).toBe(true); source.set(second); await flush(); - expect(first.used.peek()).toBe(false); - expect(second.used.peek()).toBe(true); + expect(first.wanted.peek()).toBe(false); + expect(second.wanted.peek()).toBe(true); second.set(3); await flush(); expect(seen).toEqual([2, 3]); stop(); stop(); - expect(second.used.peek()).toBe(false); + expect(second.wanted.peek()).toBe(false); }); test("one-shot forwarding re-arms once per change without releasing demand", async () => { @@ -72,7 +72,7 @@ test("one-shot forwarding re-arms once per change without releasing demand", asy value.set(i); await flush(); expect(seen).toHaveLength(i); - expect(value.used.peek()).toBe(i < 3); + expect(value.wanted.peek()).toBe(i < 3); } expect(seen).toEqual([1, 2, 3]); }); @@ -89,7 +89,7 @@ test("independent observers may share a callback", async () => { value.set(1); await flush(); expect(calls).toBe(1); - expect(value.used.peek()).toBe(true); + expect(value.wanted.peek()).toBe(true); second(); - expect(value.used.peek()).toBe(false); + expect(value.wanted.peek()).toBe(false); }); diff --git a/js/net/src/util/observed.ts b/js/net/src/util/observed.ts index 798078797d1..c035ed0f049 100644 --- a/js/net/src/util/observed.ts +++ b/js/net/src/util/observed.ts @@ -4,16 +4,16 @@ type Listener = (value: T) => void; /** A package-private signal whose listeners hold demand for its producer. */ export class Observed extends Signal { - readonly used = new Signal(false); + readonly wanted = new Signal(false); #observers = 0; #observe(): Dispose { - if (++this.#observers === 1) this.used.set(true); + if (++this.#observers === 1) this.wanted.set(true); let active = true; return () => { if (!active) return; active = false; - if (--this.#observers === 0) this.used.set(false); + if (--this.#observers === 0) this.wanted.set(false); }; } @@ -32,7 +32,7 @@ export class Observed extends Signal { if (!fn) return new Promise((resolve) => this.changed(resolve)); const release = this.#observe(); const dispose = super.changed((value) => { - // Schedule a reactive reader's next run before publishing unused, so its + // Schedule a reactive reader's next run before publishing unwanted, so its // replacement listener coalesces away the temporary gap in demand. try { fn(value); diff --git a/py/moq-rs/moq/publish.py b/py/moq-rs/moq/publish.py index 3cb61709077..7ad51a25f87 100644 --- a/py/moq-rs/moq/publish.py +++ b/py/moq-rs/moq/publish.py @@ -194,10 +194,10 @@ class Fill: """A run of groups readers miss, for a :class:`TrackRequests` to serve. Call :meth:`accept` for each group you have and :meth:`drop_groups` for the rest; - :meth:`next` names what is still owed, in the fill's order. Dropping the fill names + :meth:`wanted` names what is still owed, in the fill's order. Dropping the fill names whatever is left as dropped while a reader still wants it; once nobody does, it is - withdrawn, so stop serving it when :meth:`unused` returns. Stop serving a single group - when :meth:`unwanted` returns for it, and move on to :meth:`next`. + withdrawn, so stop serving it once :meth:`wanted` is ``None``. Stop serving a single group + when :meth:`unwanted` returns for it, and move on to :meth:`wanted`. """ def __init__(self, inner: MoqFill) -> None: @@ -233,9 +233,16 @@ def order(self) -> MoqOrder: """Which end to fill first.""" return self._inner.order() - def next(self) -> int | None: - """The next group still owed in the fill's order, ``None`` once every group is answered.""" - return self._inner.next() + def wanted(self) -> int | None: + """The next group still owed in the fill's order. + + ``None`` once every group is answered or no reader waits on the fill. + """ + return self._inner.wanted() + + async def wanted_changed(self) -> int | None: + """Wait for ``wanted`` to change and return it. The first call returns at once.""" + return await self._inner.wanted_changed() def accept(self, sequence: int) -> GroupProducer: """Insert group ``sequence`` and return a producer for it. @@ -248,14 +255,6 @@ def drop_groups(self, start: int, end: int) -> None: """Name groups ``start`` up to the exclusive ``end`` as never to be delivered.""" self._inner.drop_groups(start, end) - def is_used(self) -> bool: - """Whether a reader still waits on this fill.""" - return self._inner.is_used() - - async def unused(self) -> None: - """Wait until no reader waits on this fill.""" - await self._inner.unused() - def is_wanted(self, sequence: int) -> bool: """Whether a reader still waits for group ``sequence`` from this fill. diff --git a/py/moq-rs/tests/test_local.py b/py/moq-rs/tests/test_local.py index 4aaf944f467..94922fbe1b8 100644 --- a/py/moq-rs/tests/test_local.py +++ b/py/moq-rs/tests/test_local.py @@ -485,8 +485,7 @@ async def test_fetch_group_and_serve_dynamic_miss(): fill = await asyncio.wait_for(dynamic.requested_fill(), timeout=5.0) assert (fill.start_group, fill.start_frame, fill.end_group) == (7, 0, 8) assert fill.priority == 11 - assert fill.next() == 7 - assert fill.is_used() + assert fill.wanted() == 7 assert fill.is_wanted(7) assert not fill.is_wanted(8) diff --git a/rs/kio/src/consumer.rs b/rs/kio/src/consumer.rs index 9b2347332f6..44af36a6a68 100644 --- a/rs/kio/src/consumer.rs +++ b/rs/kio/src/consumer.rs @@ -7,7 +7,7 @@ use crate::{Closed, Counts, State, lock::*, producer::Ref, sync::Arc, waiter::*, /// Consumers have read-only access to the shared value and are notified when /// a producer modifies it. Cloning a consumer increments the consumer reference /// count. When the last consumer is dropped, the consumer-count waiters -/// (e.g. [`Producer::unused`](crate::Producer::unused)) are notified. +/// (e.g. [`Producer::wanted_changed`](crate::Producer::wanted_changed)) are notified. #[derive(Debug)] pub struct Consumer { pub(crate) state: Lock>, @@ -36,7 +36,7 @@ fn mint(state: &Lock>, counts: &Arc, only_open: bool) -> Opt return None; } - // Wake `used()` waiters when the first consumer appears, after the lock is + // Wake `wanted_changed()` waiters when the first consumer appears, after the lock is // released. match counts.consumers.fetch_add(1, Ordering::AcqRel) { 0 => Some(guard.waiters_consumer.take()), @@ -165,7 +165,7 @@ impl Drop for Consumer { return; } - // We were the last consumer, so wake the `unused()` waiters. The value + // We were the last consumer, so wake the `wanted_changed()` waiters. The value // and closed waiters don't care about the consumer count, so leave // them alone. let mut waiters = { diff --git a/rs/kio/src/lib.rs b/rs/kio/src/lib.rs index 8b8ab652c88..ca9f55eae92 100644 --- a/rs/kio/src/lib.rs +++ b/rs/kio/src/lib.rs @@ -52,7 +52,7 @@ mod tests; pub use consumer::Consumer; pub use lock::{Lock, LockGuard}; pub use pending::Pending; -pub use producer::{Mut, Producer, Ref, Unused}; +pub use producer::{Mut, Producer, Ref, Unwanted}; pub use queue::{PushError, Queue}; pub use send::MaybeSend; pub use shared::Shared; @@ -134,8 +134,7 @@ pub(crate) struct State { pub waiters_value: waiter::WaiterList, /// Closure (`closed`). Woken only when the channel closes. pub waiters_closed: waiter::WaiterList, - /// Consumer-count changes (`used`/`unused`). `used`/`unused` are used - /// sequentially in practice, so they share one list. + /// Consumer-count changes (`wanted_changed`), in either direction. pub waiters_consumer: waiter::WaiterList, pub closed: bool, } diff --git a/rs/kio/src/loom.rs b/rs/kio/src/loom.rs index 3d89efb01af..ea2e8791b20 100644 --- a/rs/kio/src/loom.rs +++ b/rs/kio/src/loom.rs @@ -131,59 +131,59 @@ fn weak_downgrade_upgrade_races_the_last_drop() { }); } -/// The first consumer appearing must wake a producer parked on `used()`, however the +/// The first consumer appearing must wake a producer parked on `wanted_changed()`, however the /// `fetch_add` interleaves with the waiter registration. #[test] -fn first_consumer_wakes_used() { +fn first_consumer_wakes_wanted_changed() { loom::model(|| { let producer = Producer::new(0u32); let second = producer.clone(); - // The consumer rides back on the join handle so it outlives the `used()` poll. + // The consumer rides back on the join handle so it outlives the `wanted_changed()` poll. let maker = thread::spawn(move || second.consume()); - assert_eq!(block_on(producer.used()), Ok(()), "the new consumer was missed"); + assert_eq!(block_on(producer.wanted_changed(false)), Ok(true), "the new consumer was missed"); drop(maker.join().unwrap()); }); } /// The mirror image: the last consumer leaving must wake a producer parked on -/// `unused()`. +/// `wanted_changed()`. #[test] -fn last_consumer_wakes_unused() { +fn last_consumer_wakes_wanted_changed() { loom::model(|| { let producer = Producer::new(0u32); let consumer = producer.consume(); let dropper = thread::spawn(move || drop(consumer)); - assert_eq!(block_on(producer.unused()), Ok(()), "the last drop was missed"); + assert_eq!(block_on(producer.wanted_changed(true)), Ok(false), "the last drop was missed"); dropper.join().unwrap(); }); } -/// A consumer created and dropped while `unused()` is parked must not leave it +/// A consumer created and dropped while `wanted_changed()` is parked must not leave it /// parked on a stale count. #[test] -fn consumer_churn_resolves_unused() { +fn consumer_churn_resolves_unwanted() { loom::model(|| { let producer = Producer::new(0u32); let second = producer.clone(); let churn = thread::spawn(move || drop(second.consume())); - assert_eq!(block_on(producer.unused()), Ok(()), "unused() stalled on churn"); + assert_eq!(block_on(producer.wanted_changed(true)), Ok(false), "wanted_changed() stalled on churn"); churn.join().unwrap(); }); } -/// The invariant `write_unused` exists for: a consumer and a teardown racing must +/// The invariant `write_unwanted` exists for: a consumer and a teardown racing must /// never both win. Either the consumer is minted and the teardown is declined, or /// the teardown closes the channel and the weak handle mints nothing. /// /// Without the count moving under the state lock, the "closed and minted" corner /// is reachable: the bump lands after the teardown read zero, and the consumer is -/// left holding a channel that was torn down for being unused. +/// left holding a channel that was torn down for being unwanted. #[test] fn a_teardown_and_a_consumer_never_both_win() { loom::model(|| { @@ -192,13 +192,13 @@ fn a_teardown_and_a_consumer_never_both_win() { let consumer = thread::spawn(move || weak.try_consume()); - let committed = match producer.write_unused() { - crate::Unused::Idle(guard) => { + let committed = match producer.write_unwanted() { + crate::Unwanted::Idle(guard) => { guard.close(); true } - crate::Unused::Used => false, - crate::Unused::Closed => unreachable!("nothing else closes this channel"), + crate::Unwanted::Wanted => false, + crate::Unwanted::Closed => unreachable!("nothing else closes this channel"), }; let consumer = consumer.join().unwrap(); @@ -326,9 +326,9 @@ fn consumer_creation_cannot_cross_an_idle_guard() { let weak = producer.weak(); let minting = thread::spawn(move || weak.consume()); drop(existing); - if let crate::Unused::Idle(guard) = producer.write_unused() { + if let crate::Unwanted::Idle(guard) = producer.write_unwanted() { thread::yield_now(); - assert!(!producer.is_used(), "consumer appeared while idle guard was held"); + assert!(!producer.wanted(), "consumer appeared while idle guard was held"); guard.close(); } drop(minting.join().unwrap()); diff --git a/rs/kio/src/producer.rs b/rs/kio/src/producer.rs index 5328d59a80a..58c90cf9ba7 100644 --- a/rs/kio/src/producer.rs +++ b/rs/kio/src/producer.rs @@ -66,30 +66,30 @@ impl Producer { /// Acquire mutable access only while nothing is consuming the channel. /// /// The count is read under the lock [`consume`](Self::consume) takes when - /// demand is zero, so the returned guard keeps the channel unused until it - /// drops. That is what a teardown decided by [`poll_unused`](Self::poll_unused) + /// demand is zero, so the returned guard keeps the channel unwanted until it + /// drops. That is what a teardown decided by [`poll_wanted_changed`](Self::poll_wanted_changed) /// needs: the poll is a level snapshot, so committing on it directly can cancel /// a consumer that appeared in the gap. Commit through this instead, and take - /// [`Unused::Used`] as "a consumer got there first, keep going". + /// [`Unwanted::Wanted`] as "a consumer got there first, keep going". /// /// Pair it with [`ProducerWeak::try_consume`], which refuses to mint a consumer for /// a closed channel: between the two, a consumer either exists in time to /// decline the teardown or is never handed out at all. - pub fn write_unused(&self) -> Unused<'_, T> { + pub fn write_unwanted(&self) -> Unwanted<'_, T> { let state = self.state.lock(); if state.closed { - return Unused::Closed; + return Unwanted::Closed; } // Relaxed is enough: a zero-to-one increment happens under this lock. // Further increments cannot pass through zero. A decrement runs outside // it, so this may still read a consumer that is going away, which only - // declines a teardown that the next `unused` wake will offer again. + // declines a teardown that the next `wanted_changed` wake will offer again. if self.counts.consumers.load(Ordering::Relaxed) > 0 { - return Unused::Used; + return Unwanted::Wanted; } - Unused::Idle(Mut::new(state)) + Unwanted::Idle(Mut::new(state)) } /// Close the channel, notifying all consumers. @@ -204,71 +204,41 @@ impl Producer { Poll::Pending } - /// Wait until all consumers have been dropped. + /// Whether any consumer handle exists right now. /// - /// Returns `Ok(())` when no consumers remain, or [`Closed`] if the channel closes first. - pub async fn unused(&self) -> Result<(), Closed> { - crate::wait(move |waiter| self.poll_unused(waiter)).await - } - - /// Poll-based variant of [`Self::unused`]: `Ready(Ok(()))` when no consumers - /// remain, [`Closed`] if the channel closed first, else `Pending`. - #[cfg_attr(feature = "_sim", track_caller)] - pub fn poll_unused(&self, waiter: &Waiter) -> Poll> { - let mut state = self.state.lock(); - if state.closed { - return crate::coop::ready(Err(Closed)); - } - - if self.counts.consumers.load(Ordering::Relaxed) == 0 { - return crate::coop::ready(Ok(())); - } - - waiter.register(&mut state.waiters_consumer); - - // Re-check after registration to avoid TOCTOU race where the last - // consumer drops between the initial check and waiter registration. - if self.counts.consumers.load(Ordering::Relaxed) == 0 { - return crate::coop::ready(Ok(())); - } - - Poll::Pending - } - - /// Whether any consumer handle currently exists. - /// - /// A point-in-time snapshot with no registration; use [`Self::poll_used`] / - /// [`Self::poll_unused`] to wait for the edge instead. - pub fn is_used(&self) -> bool { + /// A point-in-time snapshot with no registration; use + /// [`Self::poll_wanted_changed`] to wait for it to change instead. + pub fn wanted(&self) -> bool { self.counts.consumers.load(Ordering::Relaxed) > 0 } - /// Wait until at least one consumer exists. - /// - /// Returns `Ok(())` when a consumer is created, or [`Closed`] if the channel closes first. - pub async fn used(&self) -> Result<(), Closed> { - crate::wait(move |waiter| self.poll_used(waiter)).await + /// Wait until [`Self::wanted`] differs from `last`, returning the new value, or + /// [`Closed`] if the channel closes first. + pub async fn wanted_changed(&self, last: bool) -> Result { + crate::wait(move |waiter| self.poll_wanted_changed(waiter, last)).await } - /// Poll-based variant of [`Self::used`]: `Ready(Ok(()))` once a consumer - /// exists, [`Closed`] if the channel closed first, else `Pending`. + /// Poll for [`Self::wanted`] to differ from `last`: `Ready(Ok)` with the new value once + /// it does, [`Closed`] if the channel closed first, else `Pending`. #[cfg_attr(feature = "_sim", track_caller)] - pub fn poll_used(&self, waiter: &Waiter) -> Poll> { + pub fn poll_wanted_changed(&self, waiter: &Waiter, last: bool) -> Poll> { + // Closure is checked first, so a closed channel reports `Closed` from every handle + // rather than a consumer count that only dropped because it's dead. let mut state = self.state.lock(); if state.closed { return crate::coop::ready(Err(Closed)); } - if self.counts.consumers.load(Ordering::Relaxed) > 0 { - return crate::coop::ready(Ok(())); + if self.wanted() != last { + return crate::coop::ready(Ok(!last)); } waiter.register(&mut state.waiters_consumer); - // Re-check after registration to avoid TOCTOU race where a consumer - // is created between the initial check and waiter registration. - if self.counts.consumers.load(Ordering::Relaxed) > 0 { - return crate::coop::ready(Ok(())); + // Re-check after registration to avoid a TOCTOU race where the count moves + // between the initial check and waiter registration. + if self.wanted() != last { + return crate::coop::ready(Ok(!last)); } Poll::Pending @@ -333,7 +303,7 @@ impl Drop for Producer { } // We were the last producer, so close. Every waiter reacts to closure - // (value/closed resolve, `used`/`unused` resolve to `None`), so drain + // (value/closed resolve, `wanted_changed` resolves to `Closed`), so drain // every list and wake them once the lock is released. state.closed = true; state.take_close_waiters() @@ -345,16 +315,16 @@ impl Drop for Producer { } } -/// What [`Producer::write_unused`] found. +/// What [`Producer::write_unwanted`] found. #[must_use] #[derive(Debug)] -pub enum Unused<'a, T> { +pub enum Unwanted<'a, T> { /// Nothing is consuming the channel: write access, held under the lock a new /// consumer has to take, so the write lands before one can exist. Idle(Mut<'a, T>), - /// A consumer exists, so whatever was decided while the channel looked unused + /// A consumer exists, so whatever was decided while the channel looked unwanted /// is stale. - Used, + Wanted, /// The channel is already closed, so there is nothing left to write. Closed, } diff --git a/rs/kio/src/tests.rs b/rs/kio/src/tests.rs index ac2de8ed6e3..abaf1212620 100644 --- a/rs/kio/src/tests.rs +++ b/rs/kio/src/tests.rs @@ -1,7 +1,7 @@ //! Regression tests for the value/closed/consumer waiter split. //! //! Each non-close event must wake only the waiters that care about it: a value -//! modification wakes neither `closed()` nor `used`/`unused`, and consumer +//! modification wakes neither `closed()` nor `wanted_changed`, and consumer //! create/drop wakes neither value nor `closed()` waiters. We drive the public //! async futures by hand with a counting waker so we can assert that a waiter //! was *not* notified. @@ -55,20 +55,20 @@ fn value_modification_does_not_wake_consumer() { let (waker, w) = counting(); let mut cx = Context::from_waker(&w); - let mut fut = Box::pin(producer.unused()); + let mut fut = Box::pin(producer.wanted_changed(true)); assert!(matches!(fut.as_mut().poll(&mut cx), Poll::Pending)); - // `unused()` only cares about the consumer count, so value churn shouldn't wake it. + // `wanted_changed()` only cares about the consumer count, so value churn shouldn't wake it. for i in 1..=100 { *producer.write().ok().expect("open") = i; } - assert_eq!(waker.count(), 0, "value modification spuriously woke unused()"); + assert_eq!(waker.count(), 0, "value modification spuriously woke wanted_changed()"); assert!(matches!(fut.as_mut().poll(&mut cx), Poll::Pending)); // Dropping the last consumer is what should wake it. drop(consumer); - assert!(waker.count() >= 1, "dropping the last consumer did not wake unused()"); - assert!(matches!(fut.as_mut().poll(&mut cx), Poll::Ready(Ok(())))); + assert!(waker.count() >= 1, "dropping the last consumer did not wake wanted_changed()"); + assert!(matches!(fut.as_mut().poll(&mut cx), Poll::Ready(Ok(false)))); } #[test] @@ -146,12 +146,12 @@ fn close_wakes_value_closed_and_consumer() { let mut value_fut = Box::pin(producer.wait(|m| if **m == 99 { Poll::Ready(()) } else { Poll::Pending })); let mut closed_fut = Box::pin(producer.closed()); - let mut unused_fut = Box::pin(producer.unused()); + let mut unused_fut = Box::pin(producer.wanted_changed(true)); assert!(matches!(value_fut.as_mut().poll(&mut value_cx), Poll::Pending)); assert!(matches!(closed_fut.as_mut().poll(&mut closed_cx), Poll::Pending)); assert!(matches!(unused_fut.as_mut().poll(&mut unused_cx), Poll::Pending)); - // Closing resolves all three: value/unused with `Err`, closed with `()`. + // Closing resolves all three: value/wanted_changed with `Err`, closed with `()`. producer.close().ok().expect("open"); assert!(value_waker.count() >= 1, "close did not wake the value waiter"); assert!(closed_waker.count() >= 1, "close did not wake the closed waiter"); @@ -164,27 +164,27 @@ fn close_wakes_value_closed_and_consumer() { assert!(matches!(unused_fut.as_mut().poll(&mut unused_cx), Poll::Ready(Err(_)))); } -/// The three states `write_unused` distinguishes, since a teardown branches on all +/// The three states `write_unwanted` distinguishes, since a teardown branches on all /// of them: commit, keep going, already gone. #[test] -fn write_unused_reports_used_closed_and_idle() { +fn write_unwanted_reports_wanted_closed_and_idle() { let producer = Producer::new(0u32); let consumer = producer.consume(); assert!( - matches!(producer.write_unused(), crate::Unused::Used), + matches!(producer.write_unwanted(), crate::Unwanted::Wanted), "a live consumer must decline the write" ); drop(consumer); - let crate::Unused::Idle(mut guard) = producer.write_unused() else { + let crate::Unwanted::Idle(mut guard) = producer.write_unwanted() else { panic!("the last consumer left, so the write is owed"); }; *guard = 1; guard.close(); assert!( - matches!(producer.write_unused(), crate::Unused::Closed), + matches!(producer.write_unwanted(), crate::Unwanted::Closed), "a closed channel has nothing left to write" ); } @@ -199,16 +199,16 @@ fn a_consumer_minted_before_the_guard_declines_it() { let weak = producer.weak(); // The wake a teardown acts on. - assert!(matches!(producer.write_unused(), crate::Unused::Idle(_))); + assert!(matches!(producer.write_unwanted(), crate::Unwanted::Idle(_))); // Demand returns in the gap between that wake and the commit. let consumer = weak.try_consume().expect("the channel is open"); - assert!(matches!(producer.write_unused(), crate::Unused::Used)); + assert!(matches!(producer.write_unwanted(), crate::Unwanted::Wanted)); // Once the teardown does commit, the weak handle mints nothing more. drop(consumer); - let crate::Unused::Idle(guard) = producer.write_unused() else { - panic!("unused again"); + let crate::Unwanted::Idle(guard) = producer.write_unwanted() else { + panic!("unwanted again"); }; guard.close(); assert!(weak.try_consume().is_none(), "a closed channel minted a consumer"); diff --git a/rs/kio/src/weak.rs b/rs/kio/src/weak.rs index cff7c3ce90b..a2764b3a9fd 100644 --- a/rs/kio/src/weak.rs +++ b/rs/kio/src/weak.rs @@ -130,7 +130,7 @@ impl ProducerWeak { /// Create a consumer only if the channel is open. /// /// The closed check and count increment share the lock held by - /// [`Producer::write_unused`], so an idle teardown either sees this consumer + /// [`Producer::write_unwanted`], so an idle teardown either sees this consumer /// or closes the channel before this returns `None`. pub fn try_consume(&self) -> Option> { crate::consumer::consume_open(&self.state, &self.counts) @@ -165,73 +165,35 @@ impl ProducerWeak { Poll::Pending } - /// Wait until all consumers have been dropped. + /// Whether any consumer handle exists right now. /// - /// Returns `Ok(())` when no consumers remain, or [`Closed`] if the channel closes first. - pub async fn unused(&self) -> Result<(), Closed> { - crate::wait(move |waiter| self.poll_unused(waiter)).await - } - - /// Poll-based variant of [`Self::unused`]: `Ready(Ok(()))` when no consumers - /// remain, [`Closed`] if the channel closed first, else `Pending`. - #[cfg_attr(feature = "_sim", track_caller)] - pub fn poll_unused(&self, waiter: &Waiter) -> Poll> { - // Closure is checked first, matching `Producer::poll_unused`: a closed channel - // with no consumers resolves `Closed` from either handle. - let mut state = self.state.lock(); - if state.closed { - return crate::coop::ready(Err(Closed)); - } - - if self.counts.consumers.load(Ordering::Relaxed) == 0 { - return crate::coop::ready(Ok(())); - } - - waiter.register(&mut state.waiters_consumer); - - // Re-check after registration to avoid TOCTOU race where the last - // consumer drops between the initial check and waiter registration. - if self.counts.consumers.load(Ordering::Relaxed) == 0 { - return crate::coop::ready(Ok(())); - } - - Poll::Pending - } - - /// Whether any consumer handle currently exists. - /// - /// A point-in-time snapshot with no registration; use [`Self::poll_used`] / - /// [`Self::poll_unused`] to wait for the edge instead. - pub fn is_used(&self) -> bool { + /// A point-in-time snapshot with no registration; use + /// [`Self::poll_wanted_changed`] to wait for it to change instead. + pub fn wanted(&self) -> bool { self.counts.consumers.load(Ordering::Relaxed) > 0 } - /// Wait until at least one consumer exists. - /// - /// Returns `Ok(())` when a consumer is created, or [`Closed`] if the channel closes first. - pub async fn used(&self) -> Result<(), Closed> { - crate::wait(move |waiter| self.poll_used(waiter)).await - } - - /// Poll-based variant of [`Self::used`]: `Ready(Ok(()))` once a consumer - /// exists, [`Closed`] if the channel closed first, else `Pending`. + /// Poll for [`Self::wanted`] to differ from `last`: `Ready(Ok)` with the new value once + /// it does, [`Closed`] if the channel closed first, else `Pending`. #[cfg_attr(feature = "_sim", track_caller)] - pub fn poll_used(&self, waiter: &Waiter) -> Poll> { - // Closure is checked first, matching `Producer::poll_used`. + pub fn poll_wanted_changed(&self, waiter: &Waiter, last: bool) -> Poll> { + // Closure is checked first, so a closed channel reports `Closed` from every handle + // rather than a consumer count that only dropped because it's dead. let mut state = self.state.lock(); if state.closed { return crate::coop::ready(Err(Closed)); } - if self.counts.consumers.load(Ordering::Relaxed) > 0 { - return crate::coop::ready(Ok(())); + if self.wanted() != last { + return crate::coop::ready(Ok(!last)); } waiter.register(&mut state.waiters_consumer); - // Re-check after registration to avoid TOCTOU race. - if self.counts.consumers.load(Ordering::Relaxed) > 0 { - return crate::coop::ready(Ok(())); + // Re-check after registration to avoid a TOCTOU race where the count moves + // between the initial check and waiter registration. + if self.wanted() != last { + return crate::coop::ready(Ok(!last)); } Poll::Pending @@ -278,7 +240,7 @@ impl ConsumerWeak { /// Poll the shared state with a closure, exactly as [`Consumer::poll`] does. /// /// Available here so a watcher can register for changes without joining the - /// consumer count, which is what [`Producer::unused`](crate::Producer::unused) + /// consumer count, which is what [`Producer::wanted_changed`](crate::Producer::wanted_changed) /// keys off. #[cfg_attr(feature = "_sim", track_caller)] pub fn poll(&self, waiter: &Waiter, f: F) -> Poll>> @@ -354,11 +316,10 @@ mod test { // No consumers were ever created, and the channel is now closed. producer.close().ok().expect("open"); - assert_eq!(producer.unused().await, Err(Closed)); - assert_eq!(weak.unused().await, Err(Closed)); - - assert_eq!(producer.used().await, Err(Closed)); - assert_eq!(weak.used().await, Err(Closed)); + for last in [false, true] { + assert_eq!(producer.wanted_changed(last).await, Err(Closed)); + assert_eq!(crate::wait(|w| weak.poll_wanted_changed(w, last)).await, Err(Closed)); + } } /// While the channel is open the two handles still agree on the consumer count. @@ -367,15 +328,15 @@ mod test { let producer = Producer::new(0u32); let weak = producer.weak(); - assert_eq!(producer.unused().await, Ok(())); - assert_eq!(weak.unused().await, Ok(())); + assert!(!producer.wanted()); + assert!(!weak.wanted()); let consumer = producer.consume(); - assert_eq!(producer.used().await, Ok(())); - assert_eq!(weak.used().await, Ok(())); + assert_eq!(producer.wanted_changed(false).await, Ok(true)); + assert_eq!(crate::wait(|w| weak.poll_wanted_changed(w, false)).await, Ok(true)); drop(consumer); - assert_eq!(weak.unused().await, Ok(())); + assert_eq!(crate::wait(|w| weak.poll_wanted_changed(w, true)).await, Ok(false)); } /// A state holding a handle back to itself is still deallocated once the last real diff --git a/rs/moq-archive/src/consumer/track.rs b/rs/moq-archive/src/consumer/track.rs index 914afc5b5dd..5a7f13509c0 100644 --- a/rs/moq-archive/src/consumer/track.rs +++ b/rs/moq-archive/src/consumer/track.rs @@ -165,8 +165,7 @@ impl Live { /// answered or nobody waits on it. pub(super) async fn serve(shared: Arc>, index: Arc>, fill: Fill) { let mut last = None; - while fill.is_used() - && let Some(sequence) = fill.next() + while let Some(sequence) = fill.wanted() && last != Some(sequence) { last = Some(sequence); diff --git a/rs/moq-c/src/api.rs b/rs/moq-c/src/api.rs index cbb48b75c08..4621a748b79 100644 --- a/rs/moq-c/src/api.rs +++ b/rs/moq-c/src/api.rs @@ -582,10 +582,10 @@ impl TryFrom<&moq_track_info> for moq_net::track::Info { #[expect(non_camel_case_types, reason = "C ABI type names match moq.h")] #[derive(Clone, Copy, Debug)] pub enum moq_demand { - /// At least one subscriber is active. Repeats when the wanted group moves. - MOQ_DEMAND_USED = 1, - /// No subscriber is active. - MOQ_DEMAND_UNUSED = 2, + /// At least one subscriber wants the track. Repeats when the wanted group moves. + MOQ_DEMAND_WANTED = 1, + /// No subscriber wants the track. + MOQ_DEMAND_UNWANTED = 2, } /// Subscriber-side raw track delivery preferences. diff --git a/rs/moq-c/src/bandwidth.rs b/rs/moq-c/src/bandwidth.rs index 36ad3239642..96e0695c1c2 100644 --- a/rs/moq-c/src/bandwidth.rs +++ b/rs/moq-c/src/bandwidth.rs @@ -280,7 +280,7 @@ mod consumer_tests { .unwrap(); let (sender, mut receiver) = tokio::sync::mpsc::unbounded_channel(); let user_data = || Box::into_raw(Box::new(sender.clone())) as *mut std::ffi::c_void; - assert!(producer.is_used()); + assert!(producer.demand().wanted()); assert_eq!( unsafe { moq_bandwidth_consumer_changed(id.into(), user_data(), Some(changed)) }, 0 @@ -299,7 +299,7 @@ mod consumer_tests { 0 ); assert_eq!(moq_bandwidth_consumer_close(id.into()), 0); - assert!(!producer.is_used(), "closing releases demand before the callback runs"); + assert!(!producer.demand().wanted(), "closing releases demand before the callback runs"); assert!(receiver.recv().await.unwrap() < 0); } } diff --git a/rs/moq-c/src/publish.rs b/rs/moq-c/src/publish.rs index 1f1c4c227d1..a13f493189a 100644 --- a/rs/moq-c/src/publish.rs +++ b/rs/moq-c/src/publish.rs @@ -800,8 +800,8 @@ impl Publish { } }; let state = match next { - Ok(Some(_)) => moq_demand::MOQ_DEMAND_USED, - Ok(None) => moq_demand::MOQ_DEMAND_UNUSED, + Ok(Some(_)) => moq_demand::MOQ_DEMAND_WANTED, + Ok(None) => moq_demand::MOQ_DEMAND_UNWANTED, Err(moq_net::Error::Dropped) => return Ok(()), Err(err) => return Err(err.into()), }; @@ -922,8 +922,7 @@ impl Publish { /// reads it any more, until every group is answered or nobody waits on the fill. async fn serve_fill(callback: OnStatus, fill: Arc) { let mut last = None; - while fill.is_used() - && let Some(sequence) = fill.next() + while let Some(sequence) = fill.wanted() && last != Some(sequence) { last = Some(sequence); diff --git a/rs/moq-c/src/test.rs b/rs/moq-c/src/test.rs index 1f86b714a76..1268624992b 100644 --- a/rs/moq-c/src/test.rs +++ b/rs/moq-c/src/test.rs @@ -2889,21 +2889,21 @@ fn track_demand_follows_subscribers() { // Nobody is subscribed yet; the watcher seeds with the current state. let demand_cb = Callback::new(); let watcher = id(unsafe { moq_publish_track_demand(track, Some(channel_callback), demand_cb.ptr) }); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNWANTED as i32); let consume = consume_local(broadcast); let frame_cb = Callback::new(); let consumer = consume_track(consume, name, &frame_cb); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_USED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_WANTED as i32); assert_eq!(moq_consume_track_cancel(consumer), 0); assert_eq!(frame_cb.recv_terminal(), 0); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNWANTED as i32); // A watcher registered after the last subscriber left is told so first, not left waiting. let late_cb = Callback::new(); let late = id(unsafe { moq_publish_track_demand(track, Some(channel_callback), late_cb.ptr) }); - assert_eq!(late_cb.recv(), moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(late_cb.recv(), moq_demand::MOQ_DEMAND_UNWANTED as i32); assert_eq!(moq_publish_demand_cancel(late), 0); assert_eq!(late_cb.recv_terminal(), 0); assert!(moq_publish_demand_cancel(late) < 0, "double-close should fail"); @@ -2964,14 +2964,14 @@ fn track_demand_reports_the_wanted_group() { let demand_cb = Callback::new(); let watcher = id(unsafe { moq_publish_track_demand(track, Some(channel_callback), demand_cb.ptr) }); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNWANTED as i32); assert_eq!(demand_wanted(watcher), None); // The first subscriber of a track with no group wants group 0. let consume = consume_local(broadcast); let first_cb = Callback::new(); let first = consume_track(consume, name, &first_cb); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_USED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_WANTED as i32); assert_eq!(demand_wanted(watcher), Some(0)); // A delta after the first frame puts the group past a `refresh` of 0. @@ -2986,12 +2986,12 @@ fn track_demand_reports_the_wanted_group() { // A joiner asks for the next group, and the callback repeats USED to say so. let second_cb = Callback::new(); let second = consume_track(consume, name, &second_cb); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_USED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_WANTED as i32); assert_eq!(demand_wanted(watcher), Some(1)); cancel_track(first, &first_cb); cancel_track(second, &second_cb); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNWANTED as i32); assert_eq!(demand_wanted(watcher), None); assert_eq!(moq_publish_track_finish(track), 0); @@ -3029,7 +3029,7 @@ fn track_demand_reports_current_state_before_close() { crate::ffi::RUNTIME .block_on(crate::publish::Publish::run_demand(on_demand, demand, closed)) .unwrap(); - assert_eq!(cb.recv(), moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(cb.recv(), moq_demand::MOQ_DEMAND_UNWANTED as i32); assert!( cb.rx.recv_timeout(Duration::from_millis(50)).is_err(), "the terminal is the spawner's to deliver" @@ -3050,7 +3050,7 @@ fn track_demand_reports_an_abort() { let demand_cb = ProtocolCallback::new(); let _watcher = id(unsafe { moq_publish_track_demand(track, Some(protocol_callback), demand_cb.ptr) }); - assert_eq!(demand_cb.recv().0, moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(demand_cb.recv().0, moq_demand::MOQ_DEMAND_UNWANTED as i32); assert_eq!(moq_publish_track_abort(track, 7), 0); let (code, protocol) = demand_cb.recv(); @@ -3091,7 +3091,7 @@ fn media_demand_refuses_a_container() { ); let _watcher = id(unsafe { moq_publish_media_demand(media, Some(channel_callback), cb.ptr) }); - assert_eq!(cb.recv(), moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(cb.recv(), moq_demand::MOQ_DEMAND_UNWANTED as i32); assert_eq!(moq_publish_media_finish(media), 0); assert_eq!(moq_publish_container_finish(container), 0); assert_eq!(moq_publish_close(broadcast), 0); @@ -3133,7 +3133,7 @@ fn dynamic_serves_track_requests() { ); let demand_cb = Callback::new(); let _watcher = id(unsafe { moq_publish_track_demand(track, Some(channel_callback), demand_cb.ptr) }); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_USED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_WANTED as i32); let payload = b"hello dynamic track"; assert_eq!( @@ -3195,7 +3195,7 @@ fn dynamic_serves_track_requests() { assert_eq!(moq_consume_track_cancel(consumer), 0); assert_eq!(frame_cb.recv_terminal(), 0); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNWANTED as i32); assert_eq!(moq_publish_track_finish(track), 0); assert_eq!(moq_consume_close(consume), 0); assert_eq!(moq_publish_close(broadcast), 0); @@ -3228,7 +3228,7 @@ fn dynamic_track_request_publishes_media() { // The media handle is the one moq_publish_audio returns: frames and demand work unchanged. let demand_cb = Callback::new(); let _watcher = id(unsafe { moq_publish_media_demand(media, Some(channel_callback), demand_cb.ptr) }); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_USED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_WANTED as i32); let payload = b"opus frame"; assert_eq!( @@ -3240,7 +3240,7 @@ fn dynamic_track_request_publishes_media() { assert_eq!(moq_consume_track_cancel(consumer), 0); assert_eq!(frame_cb.recv_terminal(), 0); - assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNUSED as i32); + assert_eq!(demand_cb.recv(), moq_demand::MOQ_DEMAND_UNWANTED as i32); assert_eq!(moq_publish_media_finish(media), 0); assert_eq!(moq_publish_close(broadcast), 0); assert_eq!(demand_cb.recv_terminal(), 0); diff --git a/rs/moq-ffi/src/bandwidth.rs b/rs/moq-ffi/src/bandwidth.rs index bc0d22e1f61..8cb77c3a3de 100644 --- a/rs/moq-ffi/src/bandwidth.rs +++ b/rs/moq-ffi/src/bandwidth.rs @@ -309,7 +309,7 @@ mod consumer_tests { tokio::time::pause(); let producer = moq_net::bandwidth::Producer::new(); let consumer = MoqBandwidthConsumer::new(producer.consume()); - assert!(producer.is_used()); + assert!(producer.demand().wanted()); let mut changed = std::pin::pin!(consumer.rate_changed()); assert!( changed @@ -328,7 +328,7 @@ mod consumer_tests { .is_pending() ); consumer.cancel(); - assert!(!producer.is_used()); + assert!(!producer.demand().wanted()); assert_eq!(consumer.rate(), None); assert!(matches!(changed.await, Err(MoqError::Cancelled))); } @@ -337,9 +337,9 @@ mod consumer_tests { fn dropping_the_owned_consumer_releases_demand() { let producer = moq_net::bandwidth::Producer::new(); let consumer = MoqBandwidthConsumer::new(producer.consume()); - assert!(producer.is_used()); + assert!(producer.demand().wanted()); drop(consumer); - assert!(!producer.is_used()); + assert!(!producer.demand().wanted()); } #[tokio::test] diff --git a/rs/moq-ffi/src/producer.rs b/rs/moq-ffi/src/producer.rs index d622bdc1925..8197596b5f2 100644 --- a/rs/moq-ffi/src/producer.rs +++ b/rs/moq-ffi/src/producer.rs @@ -256,7 +256,7 @@ impl RequestsProducer { impl TrackDynamicProducer { async fn requested_fill(&mut self) -> Result, MoqError> { let fill = self.inner.requested_fill().await?; - Ok(Arc::new(MoqFill { inner: Arc::new(fill) })) + Ok(Arc::new(MoqFill::new(fill))) } } @@ -484,13 +484,22 @@ impl MoqTrackRequests { /// A run of groups readers miss, mirroring [`moq_net::track::Fill`]. /// /// Call [`accept`](Self::accept) for each group you have and [`drop_groups`](Self::drop_groups) -/// for the rest; [`next`](Self::next) names what is still owed, in the fill's order. Releasing +/// for the rest; [`wanted`](Self::wanted) names what is still owed, in the fill's order. Releasing /// it names whatever is left as dropped while a reader still wants it; once nobody does, it is -/// withdrawn, so stop serving it when [`unused`](Self::unused) resolves. Stop serving a single +/// withdrawn, so stop serving it once [`wanted`](Self::wanted) is null. Stop serving a single /// group when [`unwanted`](Self::unwanted) resolves for it, and move on to the next. #[derive(uniffi::Object)] pub struct MoqFill { - inner: Arc, + inner: moq_net::track::Fill, + // The watcher `wanted_changed` advances, kept apart so concurrent waits queue in turn. + changes: Arc>, +} + +impl MoqFill { + fn new(inner: moq_net::track::Fill) -> Self { + let changes = Arc::new(tokio::sync::Mutex::new(inner.clone())); + Self { inner, changes } + } } #[uniffi::export] @@ -526,9 +535,16 @@ impl MoqFill { self.inner.order().into() } - /// The next group still owed in the fill's order, or null once every group is answered. - pub fn next(&self) -> Option { - self.inner.next() + /// The next group still owed in the fill's order, or null once every group is answered or + /// no reader waits on the fill. + pub fn wanted(&self) -> Option { + self.inner.wanted() + } + + /// Wait for `wanted` to change and return the new value. The first call returns at once. + pub async fn wanted_changed(&self) -> Result, MoqError> { + let changes = self.changes.clone(); + crate::ffi::detached(async move { Ok::<_, MoqError>(changes.lock().await.wanted_changed().await) }).await } /// Insert group `sequence` and return a producer for filling it. The first group starts @@ -548,21 +564,6 @@ impl MoqFill { self.inner.drop_groups(start..end); } - /// Whether a reader still waits on this fill. - pub fn is_used(&self) -> bool { - self.inner.is_used() - } - - /// Wait until no reader waits on this fill. - pub async fn unused(&self) -> Result<(), MoqError> { - let fill = self.inner.clone(); - crate::ffi::detached(async move { - fill.unused().await; - Ok::<_, MoqError>(()) - }) - .await - } - /// Whether a reader still waits for group `sequence` from this fill: false once every /// reader took it or narrowed past it, or nobody waits on the fill. pub fn is_wanted(&self, sequence: u64) -> bool { diff --git a/rs/moq-ffi/src/session/dial.rs b/rs/moq-ffi/src/session/dial.rs index 10efb740cb0..89a74e0360a 100644 --- a/rs/moq-ffi/src/session/dial.rs +++ b/rs/moq-ffi/src/session/dial.rs @@ -276,9 +276,14 @@ async fn forward_send(source: Option, out: &moq_ne /// Forward the receive estimate only while someone holds `out`, since holding the /// session's consumer is what requests PROBE. async fn forward_recv(session: &moq_net::Session, out: &moq_net::bandwidth::Producer) { + let mut demand = out.demand(); loop { - if out.used().await.is_err() { - return; + loop { + match demand.wanted_changed().await { + Ok(true) => break, + Ok(false) => {} + Err(_) => return, + } } let Some(mut source) = session.recv_bandwidth() else { return; @@ -286,9 +291,11 @@ async fn forward_recv(session: &moq_net::Session, out: &moq_net::bandwidth::Prod loop { tokio::select! { biased; - unused = out.unused() => { - if unused.is_err() { - return; + wanted = demand.wanted_changed() => { + match wanted { + Ok(true) => continue, + Ok(false) => {} + Err(_) => return, } let _ = out.set(None); break; diff --git a/rs/moq-ffi/src/test.rs b/rs/moq-ffi/src/test.rs index a2a5974cbdf..8b9dd8a9c16 100644 --- a/rs/moq-ffi/src/test.rs +++ b/rs/moq-ffi/src/test.rs @@ -2599,7 +2599,7 @@ async fn dynamic_track_serves_fetch_miss_and_priority() { } #[tokio::test] -async fn fill_goes_unused_when_the_fetcher_leaves() { +async fn fill_goes_unwanted_when_the_fetcher_leaves() { let broadcast = MoqBroadcastProducer::new().unwrap(); let track = broadcast.publish_track("events".into(), None).unwrap(); let dynamic = track.requests().unwrap(); @@ -2610,19 +2610,20 @@ async fn fill_goes_unused_when_the_fetcher_leaves() { .await .expect("timed out waiting for fill") .unwrap(); - assert!(fill.is_used()); + assert_eq!(fill.wanted_changed().await.unwrap(), Some(5)); assert!(fill.is_wanted(5)); assert!(!fill.is_wanted(6), "outside the fill"); fetch.abort(); - tokio::time::timeout(TIMEOUT, fill.unused()) + let wanted = tokio::time::timeout(TIMEOUT, fill.wanted_changed()) .await - .expect("timed out waiting for the abandoned fetch to become unused") + .expect("timed out waiting for the abandoned fetch to become unwanted") .unwrap(); - assert!(!fill.is_used()); + assert_eq!(wanted, None); + assert_eq!(fill.wanted(), None); tokio::time::timeout(TIMEOUT, fill.unwanted(5)) .await - .expect("an unused fill wants none of its groups") + .expect("an unwanted fill wants none of its groups") .unwrap(); assert!(!fill.is_wanted(5)); } @@ -2640,7 +2641,7 @@ async fn dynamic_track_rejects_fetch_miss() { .expect("timed out waiting for fill") .unwrap(); fill.drop_groups(5, 6); - assert_eq!(fill.next(), None, "the dropped group is answered"); + assert_eq!(fill.wanted(), None, "the dropped group is answered"); let result = tokio::time::timeout(TIMEOUT, fetch) .await diff --git a/rs/moq-net/benches/track.rs b/rs/moq-net/benches/track.rs index a5d55b421f6..3e363a7a560 100644 --- a/rs/moq-net/benches/track.rs +++ b/rs/moq-net/benches/track.rs @@ -1901,7 +1901,7 @@ impl RangeFills { "everything is a miss" ); while let Poll::Ready(Ok(fill)) = self.dynamic.poll_requested_fill(&waiter) { - while let Some(sequence) = fill.next() { + while let Some(sequence) = fill.wanted() { let mut group = fill.accept(sequence, None).unwrap(); group .write_frame(Timestamp::from_millis(sequence).unwrap(), Bytes::from_static(b"x")) diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index f5d8396da80..ae066a58fde 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1678,7 +1678,7 @@ impl Publisher { async fn released(&self, credit: &kio::Producer<()>) { let mut session = self.session.clone(); kio::wait(|waiter| { - if session.poll_closed(&mut waiter.context()).is_ready() || credit.poll_unused(waiter).is_ready() { + if session.poll_closed(&mut waiter.context()).is_ready() || credit.poll_wanted_changed(waiter, true).is_ready() { return Poll::Ready(()); } Poll::Pending diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index 54cebefe420..117ea632261 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -2811,12 +2811,12 @@ impl Subscriber { loop { match listening { None => { - if !matches!(lister.poll_used(waiter), Poll::Ready(Ok(()))) { + if !matches!(lister.poll_wanted(waiter), Poll::Ready(Ok(()))) { return; } *listening = Some(self.listen(path.to_owned(), lister.clone())); } - Some(_) if lister.poll_unused(waiter).is_ready() => { + Some(_) if lister.poll_unwanted(waiter).is_ready() => { *listening = None; lister.clear(); } @@ -2870,7 +2870,7 @@ impl Subscriber { request.reject(Error::Unauthorized); return; } - None if request.reject_unused(Error::Cancel) => return, + None if request.reject_unwanted(Error::Cancel) => return, // Demand returned in the gap. None => continue, } @@ -3222,7 +3222,7 @@ impl Subscriber { else { break None; }; - if pending.reject_unused(Error::Cancel) { + if pending.reject_unwanted(Error::Cancel) { break None; } if let Some(held) = state.subscribes.get_mut(&request_id) { @@ -3918,7 +3918,7 @@ impl Subscriber { group_fetches.push(fetch); } Step::Subscribe => break true, - Step::Expired => match idle.track.abort_unused(Error::Cancel) { + Step::Expired => match idle.track.abort_unwanted(Error::Cancel) { Ok(()) => { tracing::debug!(broadcast = %crate::path::Rooted(self.origin.root(), broadcast_path), track = %track_name, "track released (idle)"); self.state.lock().lingering.remove(&id); @@ -4311,7 +4311,7 @@ impl Subscriber { .await; match res { Some(res) => break Some(res), - None if request.reject_unused(Error::Cancel) => break None, + None if request.reject_unwanted(Error::Cancel) => break None, None => continue, } } @@ -5277,9 +5277,7 @@ impl Subscriber { async fn serve_fill(self, broadcast: Path, name: String, fill: track::Fill, timescale: Option) { let fill = std::sync::Arc::new(fill); let mut last = None; - while fill.is_used() - && let Some(run) = fill.next_run() - { + while let Some(run) = fill.next_run() { let run = match fill.live_group() { Some(live) if run.contains(&live) => { let mut pieces = [run.start..live, live..live + 1, live + 1..run.end] @@ -5554,7 +5552,7 @@ impl Subscriber { slot.poll(waiter, |state| match &**state { GroupFetch::Done => Poll::Ready(()), GroupFetch::Receiving(Some(group)) - if unwanted && group.poll_unused(waiter).is_ready() && group.abort_unused(Error::Cancel) => + if unwanted && group.poll_unwanted(waiter).is_ready() && group.abort_unwanted(Error::Cancel) => { abandoned = true; Poll::Ready(()) diff --git a/rs/moq-net/src/lite/subscriber.rs b/rs/moq-net/src/lite/subscriber.rs index 3e02d35be12..48ab57b55db 100644 --- a/rs/moq-net/src/lite/subscriber.rs +++ b/rs/moq-net/src/lite/subscriber.rs @@ -1460,18 +1460,18 @@ impl RecvBandwidth { } BandwidthState::WaitUsed => { let bandwidth = self.subscriber.recv_bandwidth.as_ref().expect("gated above"); - match ready!(bandwidth.poll_used(waiter)) { - Ok(()) => self.state = BandwidthState::Probing(ProbeStream::new(&self.subscriber)), + match ready!(bandwidth.poll_wanted_changed(waiter, false)) { + Ok(_) => self.state = BandwidthState::Probing(ProbeStream::new(&self.subscriber)), Err(_) => return Poll::Ready(Ok(())), } } BandwidthState::Probing(probe) => { let bandwidth = self.subscriber.recv_bandwidth.as_ref().expect("gated above"); - match bandwidth.poll_unused(waiter) { + match bandwidth.poll_wanted_changed(waiter, true) { // Loop back: a new consumer may arrive later. Dropping the probe // machine resets its stream, and clearing the estimate keeps that // consumer from reading a stale rate before the new stream reports. - Poll::Ready(Ok(())) => { + Poll::Ready(Ok(_)) => { let _ = bandwidth.set(None); self.state = BandwidthState::WaitUsed; continue; @@ -2011,7 +2011,7 @@ impl SourceServe { loop { match &mut listing { None => { - if !matches!(self.lister.poll_used(waiter), Poll::Ready(Ok(()))) { + if !matches!(self.lister.poll_wanted(waiter), Poll::Ready(Ok(()))) { break; } if !self.subscriber.version.has_tracks() { @@ -2034,7 +2034,7 @@ impl SourceServe { unanswered: Some(self.dynamic.validity().start()), }); } - Some(_) if self.lister.poll_unused(waiter).is_ready() => { + Some(_) if self.lister.poll_unwanted(waiter).is_ready() => { // Dropping the stream resets it. listing = None; self.lister.clear(); @@ -2652,7 +2652,7 @@ mod tests { let tail = active.tail.read(); let mut fold: Option = None; for (req, feed) in std::mem::take(&mut self.held) { - if req.poll_unused(waiter).is_ready() { + if req.poll_unwanted(waiter).is_ready() { continue; } let sequence = req.sequence(); @@ -7163,18 +7163,18 @@ impl TrackServeRun { /// waiting on a token or a peer that may never answer, which would otherwise hold /// this task, its TRACK stream, and the track for good. Dropping the fetch resets the /// stream. Ready once the request is released. - fn poll_unused(&mut self, waiter: &kio::Waiter) -> Poll<()> { + fn poll_unwanted(&mut self, waiter: &kio::Waiter) -> Poll<()> { loop { let TrackRunState::Info { request, .. } = &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) { + if pending.reject_unwanted(Error::Cancel) { self.state = TrackRunState::Done; return Poll::Ready(()); } - // Demand returned in the gap and won inside `reject_unused`: poll again so the + // Demand returned in the gap and won inside `reject_unwanted`: poll again so the // unused wait is armed for when it leaves. } } @@ -7185,7 +7185,7 @@ impl kio::Task for TrackServeRun { fn poll(&mut self, waiter: &kio::Waiter) -> Poll<()> { // Checked before the auth wait too, so an abandoned request never parks on a token. - if self.poll_unused(waiter).is_ready() { + if self.poll_unwanted(waiter).is_ready() { return Poll::Ready(()); } ready!(self.gate.poll_ready(waiter)); @@ -7243,7 +7243,7 @@ impl kio::Task for TrackServeRun { }; match teardown { - ServeEnd::Idle => match serve_loop.serving.abort_unused(Error::Cancel) { + ServeEnd::Idle => match serve_loop.serving.abort_unwanted(Error::Cancel) { Ok(()) => { tracing::debug!(broadcast = %self.serve.subscriber.log_path(&self.serve.path), track = %self.serve.name, "track released (idle)"); } @@ -7568,20 +7568,20 @@ impl FeedFetches { fn hold(&mut self, fill: track::Fill, fresh: bool) { let mut next = Some((fill, fresh)); while let Some((fill, fresh)) = next.take() { - let Some(sequence) = fill.next() else { + let Some(sequence) = fill.wanted() else { continue; }; // A replacement attempt may arrive before the poll that would see the held one's // readers leave this group: that one moves on to the next group it owes, if any. if let Some(previous) = self.held.remove(&sequence) - && previous.fill.is_used() + && previous.fill.wanted().is_some() { next = Some((previous.fill, false)); } if fresh { let moving = usize::from(next.is_some()); if self.held.len() + moving >= self.max { - self.held.retain(|_, held| held.fill.is_used()); + self.held.retain(|_, held| held.fill.wanted().is_some()); } if self.held.len() + moving >= self.max { fill.refuse(Error::TooManyRequests); @@ -7651,7 +7651,7 @@ impl FeedFetches { { self.polled += 1; } - if held.fill.poll_unused(&held.waiter).is_ready() { + if held.fill.poll_withdrawn(&held.waiter).is_ready() { unused.push(held.fill); continue; } @@ -7686,7 +7686,7 @@ impl FeedFetches { // Answered before the bulk give-up, which knows no reason, so a drop's own error // reaches the fetch. if let Some((sequence, err)) = decided - && fill.next() == Some(sequence) + && fill.wanted() == Some(sequence) { fill.group(sequence).reject(err); } @@ -8162,10 +8162,10 @@ impl kio::Task for FillServeRun { None => None, }; // Withdrawn: nobody waits on what is left, so it records nothing. - if self.fill.poll_unused(waiter).is_ready() { + if self.fill.poll_withdrawn(waiter).is_ready() { return Poll::Ready(()); } - let Some(sequence) = self.fill.next() else { + let Some(sequence) = self.fill.wanted() else { return Poll::Ready(()); }; // Every run answers its group, so the same one coming back is a bug; ending the @@ -8312,7 +8312,7 @@ impl kio::Task for FetchServeRun { // A fetch nobody waits on any more is cancelled upstream, so the publisher stops // serving it (and a relay there releases its own FETCH). Ingest has its own check. if let Some(request) = self.state.request() - && request.poll_unused(waiter).is_ready() + && request.poll_unwanted(waiter).is_ready() { tracing::debug!(track = %self.serve.name, group = self.group, "fetch abandoned"); if let FetchRunState::Send { stream, .. } | FetchRunState::Answer { stream, .. } = @@ -8497,12 +8497,12 @@ impl kio::Task for FetchServeRun { // wants the rest (no reader left to pick it up, none reading it), // cancel upstream and abort the truncated group, never caching it // as complete. The abort is atomic with a new reader arriving. - if readers.poll_unused(waiter).is_pending() - || producer.handle().poll_unused(waiter).is_pending() + if readers.poll_unwanted(waiter).is_pending() + || producer.handle().poll_unwanted(waiter).is_pending() { return Poll::Pending; } - if !producer.handle().abort_unused(Error::Cancel) { + if !producer.handle().abort_unwanted(Error::Cancel) { continue; } tracing::debug!(track = %self.serve.name, group = self.group, "fetch abandoned"); diff --git a/rs/moq-net/src/model/bandwidth.rs b/rs/moq-net/src/model/bandwidth.rs index ecafcde5693..b9098ffcc7b 100644 --- a/rs/moq-net/src/model/bandwidth.rs +++ b/rs/moq-net/src/model/bandwidth.rs @@ -2,6 +2,7 @@ //! //! A [Producer] is used to set the current estimated bitrate, notifying consumers. //! A [Consumer] can read the current estimate and wait for changes. +//! A [Demand] watches whether anyone reads it, so a producer estimates only on demand. //! //! One estimate covers a whole connection, so senders sharing one divide it with //! an [Allocator] rather than each targeting the whole thing. How a sender then @@ -114,31 +115,70 @@ impl Producer { Weak(self.state.weak()) } - /// Block until there are no active consumers. - #[cfg(feature = "async")] - pub async fn unused(&self) -> Result<()> { - kio::wait(|waiter| self.poll_unused(waiter)).await + /// A watch-only handle to whether anyone reads the estimate. + pub fn demand(&self) -> Demand { + Demand { + state: self.state.weak(), + prev: None, + } } - /// Poll until there are no active consumers. Errors if the channel closes first. - pub fn poll_unused(&self, waiter: &kio::Waiter) -> Poll> { - self.state.poll_unused(waiter).map_err(|_| Error::Dropped) + /// Poll for consumer presence to differ from `last`, with the new value once it does. + /// Errors once the channel closes. + pub(crate) fn poll_wanted_changed(&self, waiter: &kio::Waiter, last: bool) -> Poll> { + self.state + .poll_wanted_changed(waiter, last) + .map_err(|_| Error::Dropped) } +} + +/// A cloneable, watch-only handle to whether anyone reads a bandwidth estimate. +/// +/// Holding it is not demand and keeps nothing open. Once every [`Producer`] is gone, its +/// waits end with [`Error::Dropped`]. Each clone watches on its own, so its first +/// [`poll_wanted_changed`](Self::poll_wanted_changed) reports the current value. +pub struct Demand { + state: kio::ProducerWeak, + // The value `poll_wanted_changed` last returned, or `None` before it returned any. + prev: Option, +} - /// Whether at least one active consumer exists right now. - pub fn is_used(&self) -> bool { - self.state.is_used() +impl Clone for Demand { + fn clone(&self) -> Self { + Self { + state: self.state.clone(), + prev: None, + } + } +} + +impl Demand { + /// Whether the estimate is still produced and a consumer reads it right now. + pub fn wanted(&self) -> bool { + !self.state.is_closed() && self.state.wanted() } - /// Block until there is at least one active consumer. + /// Return [`wanted`](Self::wanted) once it differs from what this handle last returned: + /// immediately on the first call, then on each change. Errors once every producer is gone. #[cfg(feature = "async")] - pub async fn used(&self) -> Result<()> { - kio::wait(|waiter| self.poll_used(waiter)).await + pub async fn wanted_changed(&mut self) -> Result { + kio::wait(|waiter| self.poll_wanted_changed(waiter)).await } - /// Poll until at least one active consumer exists. Errors if the channel closes first. - pub fn poll_used(&self, waiter: &kio::Waiter) -> Poll> { - self.state.poll_used(waiter).map_err(|_| Error::Dropped) + /// Poll for [`wanted`](Self::wanted) to differ from what this handle last returned. + pub fn poll_wanted_changed(&mut self, waiter: &kio::Waiter) -> Poll> { + loop { + if self.state.is_closed() { + return Poll::Ready(Err(Error::Dropped)); + } + let wanted = self.state.wanted(); + if self.prev != Some(wanted) { + self.prev = Some(wanted); + return Poll::Ready(Ok(wanted)); + } + // Closing fails the wait, which errors above. + let _ = std::task::ready!(self.state.poll_wanted_changed(waiter, wanted)); + } } } @@ -557,7 +597,7 @@ impl Share { // then left, needs that to know when to resume. .filter(|(want, demand)| match want.id == self.id { true => !demand.is_closed(), - false => demand.is_used(), + false => demand.is_wanted(), }) .map(|(want, _)| want) .collect(); @@ -1205,7 +1245,7 @@ mod tests { retired.update(bps(4_000_000)); assert_eq!(sibling.rate(), Some(bps(1_000_000))); assert_eq!(target.poll_changed(&waiter), Poll::Ready(bps(12_000_000))); - assert!(encoder.demand().is_used()); + assert!(encoder.demand().is_wanted()); } /// A reader parked on `changed` has to be woken by its own reservation moving, not diff --git a/rs/moq-net/src/model/broadcast.rs b/rs/moq-net/src/model/broadcast.rs index 123d8f294c3..3b661156895 100644 --- a/rs/moq-net/src/model/broadcast.rs +++ b/rs/moq-net/src/model/broadcast.rs @@ -443,17 +443,17 @@ impl Producer { /// Whether any [`Consumer`] handle exists, which is not itself demand for a track. pub(super) fn is_held(&self) -> bool { - self.alive.token.is_used() + self.alive.token.wanted() } /// `Ready(Ok)` once a [`Consumer`] handle exists, `Ready(Err)` once the broadcast ended. pub(super) fn poll_held(&self, waiter: &kio::Waiter) -> Poll> { - self.alive.token.poll_used(waiter) + self.alive.token.poll_wanted_changed(waiter, false).map_ok(drop) } /// `Ready(Ok)` once no [`Consumer`] handle is left, `Ready(Err)` once the broadcast ended. pub(crate) fn poll_unheld(&self, waiter: &kio::Waiter) -> Poll> { - self.alive.token.poll_unused(waiter) + self.alive.token.poll_wanted_changed(waiter, true).map_ok(drop) } /// [`Self::close`], unless a [`Consumer`] holds the broadcast or a track it asked for @@ -533,10 +533,10 @@ impl Alive { } // Held under the lock a consumer's first mint takes, so none appears until the // close below lands. - let token = match self.token.write_unused() { - kio::Unused::Idle(token) => token, - kio::Unused::Used => return false, - kio::Unused::Closed => return true, + let token = match self.token.write_unwanted() { + kio::Unwanted::Idle(token) => token, + kio::Unwanted::Wanted => return false, + kio::Unwanted::Closed => return true, }; state.closing = true; state.reject_unserved(Error::Unroutable); @@ -1966,7 +1966,7 @@ mod test { // Demand returns in the gap before it commits. let viewer = consumer.track("video").unwrap().subscribe(None); let track = track - .abort_unused(Error::Cancel) + .abort_unwanted(Error::Cancel) .expect_err("viewer keeps the track alive"); // So the viewer holds a live track, not a cancelled one. @@ -1979,7 +1979,7 @@ mod test { // Once the viewer really leaves, the same teardown commits, and the lookup // re-requests the track instead of resolving the closed one. drop(subscriber); - assert!(track.abort_unused(Error::Cancel).is_ok()); + assert!(track.abort_unwanted(Error::Cancel).is_ok()); assert!(matches!(consumer.track("video"), Err(Error::NotFound))); producer.close(); @@ -1991,10 +1991,10 @@ mod test { let consumer = producer.consume(); let track = producer.create_track("video", crate::track::Info::timed()).unwrap(); let _viewer = consumer.track("video").unwrap().subscribe(None); - assert!(track.demand().is_used()); + assert!(track.demand().is_wanted()); track.clone().abort(Error::Cancel).unwrap(); - assert!(!track.demand().is_used()); - assert!(track.abort_unused(Error::Cancel).is_ok()); + assert!(!track.demand().is_wanted()); + assert!(track.abort_unwanted(Error::Cancel).is_ok()); producer.close(); } } diff --git a/rs/moq-net/src/model/front.rs b/rs/moq-net/src/model/front.rs index b027627a282..f87ebf6f844 100644 --- a/rs/moq-net/src/model/front.rs +++ b/rs/moq-net/src/model/front.rs @@ -71,7 +71,7 @@ pub(super) enum Event { /// A source closed: it will never serve again. SourceClosed { source: u64 }, /// The front's broadcast handed out a new logical track to serve. It has no - /// reader until [`Event::Used`] says so. + /// reader until [`Event::Wanted`] says so. TrackAssigned { track: Arc, now: Instant }, /// A source answered a track query: its copy's metadata, or a refusal. /// `closing` is whether the source has begun closing, in which case a @@ -93,13 +93,13 @@ pub(super) enum Event { delivered: bool, }, /// A reader arrived on the track. - Used { track: Arc }, + Wanted { track: Arc }, /// The last reader left the track. - Unused { track: Arc, now: Instant }, + Unwanted { track: Arc, now: Instant }, /// The armed deadline passed. Deadline { now: Instant }, /// The driver let go of a track the machine asked it to [`Action::Forget`]. - /// Not fed when a reader arrived first: [`Event::Used`] follows instead. + /// Not fed when a reader arrived first: [`Event::Wanted`] follows instead. Forgotten { track: Arc }, /// A consumer took hold of the front's broadcast: a request joined it, or the /// driver declined an [`Action::Retire`] because one did. @@ -177,7 +177,7 @@ struct Track { /// The most recent refusal, reported when every source has refused. refusal: Option, /// Whether anyone reads the track: a copy is only spliced in for a reader. - used: bool, + wanted: bool, /// A detached source whose copy still feeds the track until the serving source's /// replaces it: a route being beaten keeps serving until then, and keeps serving if /// the newcomer refuses the track. @@ -277,8 +277,8 @@ impl Front { Event::TrackAssigned { track, .. } | Event::TrackInfo { track, .. } | Event::TrackEnded { track, .. } - | Event::Used { track } - | Event::Unused { track, .. } + | Event::Wanted { track } + | Event::Unwanted { track, .. } | Event::Forgotten { track } => Some(track.clone()), _ => None, }; @@ -298,7 +298,7 @@ impl Front { state: TrackState::Parked { since: now }, refused: HashSet::new(), refusal: None, - used: false, + wanted: false, ended: false, draining: None, }, @@ -317,8 +317,8 @@ impl Front { result, delivered, } => self.track_ended(track, source, closing, result, delivered, &mut actions), - Event::Used { track } => self.used(track, &mut actions), - Event::Unused { track, now } => self.unused(track, now, &mut actions), + Event::Wanted { track } => self.wanted(track, &mut actions), + Event::Unwanted { track, now } => self.unwanted(track, now, &mut actions), Event::Deadline { now } => self.deadline(now, &mut actions), Event::Forgotten { track } => { self.tracks.remove(&track); @@ -435,7 +435,7 @@ impl Front { } // A read track is never parked, so idle is the only state to re-query. for (name, track) in &mut self.tracks { - if track.used && !track.ended && track.state == TrackState::Idle { + if track.wanted && !track.ended && track.state == TrackState::Idle { track.state = TrackState::Querying { source }; actions.push(Action::Query { track: name.clone(), @@ -609,7 +609,7 @@ impl Front { return; }; let track = self.tracks.get_mut(&name).expect("dispatching a known track"); - if !track.used || track.ended || track.state != TrackState::Idle { + if !track.wanted || track.ended || track.state != TrackState::Idle { return; } if track.refused.contains(&source) { @@ -625,22 +625,22 @@ impl Front { actions.push(Action::Query { track: name, source }); } - fn used(&mut self, name: Arc, actions: &mut Vec) { + fn wanted(&mut self, name: Arc, actions: &mut Vec) { let Some(track) = self.tracks.get_mut(&name) else { return; }; - track.used = true; + track.wanted = true; if let TrackState::Parked { .. } = track.state { track.state = TrackState::Idle; } self.redispatch(name, actions); } - fn unused(&mut self, name: Arc, now: Instant, actions: &mut Vec) { + fn unwanted(&mut self, name: Arc, now: Instant, actions: &mut Vec) { let Some(track) = self.tracks.get_mut(&name) else { return; }; - track.used = false; + track.wanted = false; // Nothing reads it, so a copy it still drains goes with the park. track.draining = None; match track.state { @@ -784,7 +784,7 @@ mod tests { &[Action::Arm { at: Some(t0 + LINGER) }], ); assert_actions( - front.step(Event::Used { track: name("video") }), + front.step(Event::Wanted { track: name("video") }), &[ Action::Query { track: name("video"), @@ -1020,7 +1020,7 @@ mod tests { track: name("audio"), now: Instant::now(), }); - front.step(Event::Used { track: name("audio") }); + front.step(Event::Wanted { track: name("audio") }); assert_actions( front.step(Event::TrackInfo { track: name("audio"), @@ -1043,7 +1043,7 @@ mod tests { track: name("audio"), now: Instant::now(), }); - front.step(Event::Used { track: name("audio") }); + front.step(Event::Wanted { track: name("audio") }); assert_actions( front.step(Event::TrackInfo { track: name("audio"), @@ -1113,7 +1113,7 @@ mod tests { err, }], ); - assert_actions(front.step(Event::Used { track: name("video") }), &[]); + assert_actions(front.step(Event::Wanted { track: name("video") }), &[]); } } } @@ -1145,7 +1145,7 @@ mod tests { }], ); // A returning reader is not re-spliced either. - assert_actions(front.step(Event::Used { track: name("video") }), &[]); + assert_actions(front.step(Event::Wanted { track: name("video") }), &[]); } } @@ -1241,7 +1241,7 @@ mod tests { let mut front = serving(remote(1), 100); let t0 = Instant::now(); assert_actions( - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: t0, }), @@ -1267,12 +1267,12 @@ mod tests { } /// A reader that looked the track up before the driver could forget it keeps - /// it: the driver feeds `Used` instead of `Forgotten`, and the track re-splices. + /// it: the driver feeds `Wanted` instead of `Forgotten`, and the track re-splices. #[test] fn a_reader_racing_the_forget_keeps_the_track() { let mut front = serving(remote(1), 100); let t0 = Instant::now(); - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: t0, }); @@ -1281,7 +1281,7 @@ mod tests { &[Action::Forget { track: name("video") }], ); assert_actions( - front.step(Event::Used { track: name("video") }), + front.step(Event::Wanted { track: name("video") }), &[Action::Query { track: name("video"), source: 100, @@ -1295,7 +1295,7 @@ mod tests { fn an_unread_front_retires_after_the_linger() { let mut front = serving(remote(1), 100); let t0 = Instant::now(); - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: t0, }); @@ -1315,7 +1315,7 @@ mod tests { #[test] fn a_held_front_stays_without_tracks() { let mut front = serving(local(1), 100); - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: Instant::now(), }); @@ -1342,7 +1342,7 @@ mod tests { ); let mut front = serving(local(1), 100); - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: Instant::now(), }); @@ -1374,7 +1374,7 @@ mod tests { #[test] fn a_request_racing_the_retire_keeps_the_front() { let mut front = serving(local(1), 100); - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: Instant::now(), }); @@ -1403,7 +1403,7 @@ mod tests { ); let t0 = Instant::now(); assert_actions( - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: t0, }), @@ -1411,7 +1411,7 @@ mod tests { ); // A returning reader reads what it holds: no query, and the linger waits. assert_actions( - front.step(Event::Used { track: name("video") }), + front.step(Event::Wanted { track: name("video") }), &[Action::Arm { at: None }], ); // A replacement source does not re-splice it either. @@ -1427,7 +1427,7 @@ mod tests { &[Action::Detach { source: 100 }], ); let t1 = t0 + LINGER; - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: t1, }); @@ -1453,7 +1453,7 @@ mod tests { }); let t0 = Instant::now(); assert_actions( - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: t0, }), @@ -1472,7 +1472,7 @@ mod tests { fn an_unread_local_track_is_forgotten_at_once() { let mut front = serving(local(1), 100); assert_actions( - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: Instant::now(), }), @@ -1484,12 +1484,12 @@ mod tests { fn a_returning_reader_cancels_the_linger() { let mut front = serving(remote(1), 100); let t0 = Instant::now(); - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: t0, }); assert_actions( - front.step(Event::Used { track: name("video") }), + front.step(Event::Wanted { track: name("video") }), &[ Action::Query { track: name("video"), @@ -1504,7 +1504,7 @@ mod tests { fn returning_reader_preserves_other_tracks_deadlines() { let mut front = serving(remote(1), 100); let now = Instant::now(); - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now, }); @@ -1513,7 +1513,7 @@ mod tests { now: now + LINGER / 2, }); assert_actions( - front.step(Event::Used { track: name("video") }), + front.step(Event::Wanted { track: name("video") }), &[ Action::Query { track: name("video"), @@ -1603,8 +1603,8 @@ mod tests { Event::TrackAssigned { track, now } } else { match rng.random_range(0..4) { - 0 => Event::Used { track }, - 1 => Event::Unused { track, now }, + 0 => Event::Wanted { track }, + 1 => Event::Unwanted { track, now }, 2 => { now += LINGER / 4; Event::Deadline { now } @@ -1664,8 +1664,8 @@ mod tests { track: name("v"), now: t0, }, - Event::Used { track: name("v") }, - Event::Unused { + Event::Wanted { track: name("v") }, + Event::Unwanted { track: name("v"), now: t0, }, @@ -1816,7 +1816,7 @@ mod tests { }); let t0 = Instant::now(); assert_actions( - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: t0, }), @@ -1833,7 +1833,7 @@ mod tests { route: 3, result: Ok(300), }); - front.step(Event::Used { track: name("video") }); + front.step(Event::Wanted { track: name("video") }); let actions = front.step(Event::TrackInfo { track: name("video"), source: 300, @@ -1862,7 +1862,7 @@ mod tests { }); let t0 = Instant::now(); assert_actions( - front.step(Event::Unused { + front.step(Event::Unwanted { track: name("video"), now: t0, }), diff --git a/rs/moq-net/src/model/group.rs b/rs/moq-net/src/model/group.rs index aaaa927dec5..165f7578879 100644 --- a/rs/moq-net/src/model/group.rs +++ b/rs/moq-net/src/model/group.rs @@ -2179,16 +2179,16 @@ impl Handle { /// 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. + /// [`poll_unwanted`](Self::poll_unwanted) keeps the group alive instead of reading the abort. #[cfg_attr(not(feature = "async"), allow(dead_code, reason = "async-only"))] - pub(crate) fn abort_unused(&self, err: Error) -> bool { - match self.state.write_unused() { - kio::Unused::Idle(guard) => { + pub(crate) fn abort_unwanted(&self, err: Error) -> bool { + match self.state.write_unwanted() { + kio::Unwanted::Idle(guard) => { self.commit_abort(guard, err); true } - kio::Unused::Closed => true, - kio::Unused::Used => false, + kio::Unwanted::Closed => true, + kio::Unwanted::Wanted => false, } } @@ -2482,7 +2482,7 @@ impl Handle { } /// Create a consumer, or `None` once the group is aborted. Paired with - /// [`abort_unused`](Self::abort_unused): the closed check and the count share its + /// [`abort_unwanted`](Self::abort_unwanted): the closed check and the count share its /// lock, so a consumer either exists in time to decline the abort or is never made. pub(super) fn try_consume(&self) -> Option { self.state.weak().try_consume().map(|state| self.consumer(state)) @@ -2553,10 +2553,10 @@ impl Handle { } } - /// Poll for the group becoming unused (every consumer dropped). + /// Poll for the group becoming unwanted (every consumer dropped). #[cfg_attr(not(feature = "async"), allow(dead_code, reason = "async-only"))] - pub(crate) fn poll_unused(&self, waiter: &kio::Waiter) -> Poll<()> { - self.state.poll_unused(waiter).map(|_| ()) + pub(crate) fn poll_unwanted(&self, waiter: &kio::Waiter) -> Poll<()> { + self.state.poll_wanted_changed(waiter, true).map(drop) } /// The recorded abort reason, or [`Error::Dropped`] if the group closed without one. @@ -2608,7 +2608,7 @@ impl Demand { /// Whether the group is open and has readers right now. pub fn wanted(&self) -> bool { - !self.state.is_closed() && self.state.is_used() + !self.state.is_closed() && self.state.wanted() } /// Return [`wanted`](Self::wanted) once it differs from what this handle last returned: @@ -2624,16 +2624,13 @@ impl Demand { if self.state.is_closed() { return Poll::Ready(Err(self.abort_reason())); } - let wanted = self.state.is_used(); + let wanted = self.state.wanted(); if self.prev != Some(wanted) { self.prev = Some(wanted); return Poll::Ready(Ok(wanted)); } - // Wait for the opposite; closing fails either wait, which errors above. - let _ = ready!(match wanted { - true => self.state.poll_unused(waiter), - false => self.state.poll_used(waiter), - }); + // Closing fails the wait, which errors above. + let _ = ready!(self.state.poll_wanted_changed(waiter, wanted)); } } @@ -4255,17 +4252,17 @@ mod test { } #[test] - fn abort_unused_pairs_with_try_consume() { + fn abort_unwanted_pairs_with_try_consume() { let producer = Info { sequence: 0 }.produce(); let consumer = producer.handle.try_consume().expect("open"); assert!( - !producer.handle.abort_unused(crate::Error::Cancel), + !producer.handle.abort_unwanted(crate::Error::Cancel), "a reader declines the abort" ); drop(consumer); - assert!(producer.handle.abort_unused(crate::Error::Cancel)); + assert!(producer.handle.abort_unwanted(crate::Error::Cancel)); assert!( producer.handle.try_consume().is_none(), "an aborted group mints no reader" diff --git a/rs/moq-net/src/model/listing.rs b/rs/moq-net/src/model/listing.rs index a61b1ab96a8..300bd8ded83 100644 --- a/rs/moq-net/src/model/listing.rs +++ b/rs/moq-net/src/model/listing.rs @@ -562,9 +562,9 @@ impl Consumer { /// A handler's answer to the broadcast's listings, from [`Requests::lister`](super::Requests::lister). /// /// A broadcast has at most one. While it exists, readers wait for it to -/// [`sync`](Self::synced) before reporting [`TracksEvent::Synced`]. Answer while [`poll_used`](Self::poll_used): list what the +/// [`sync`](Self::synced) before reporting [`TracksEvent::Synced`]. Answer while [`poll_wanted`](Self::poll_wanted): list what the /// publisher behind the handler holds and keep it current, then [`clear`](Self::clear) -/// once [`poll_unused`](Self::poll_unused). Dropping it clears what it listed. +/// once [`poll_unwanted`](Self::poll_unwanted). Dropping it clears what it listed. pub(crate) struct Lister { state: kio::Shared, } @@ -584,9 +584,9 @@ impl Lister { Ok(Self { state }) } - fn poll_readers(&self, waiter: &kio::Waiter, used: bool) -> Poll> { + fn poll_readers(&self, waiter: &kio::Waiter, wanted: bool) -> Poll> { let state = ready!(self.state.poll(waiter, |state| { - match state.closing || state.listing.readers.is_empty() != used { + match state.closing || state.listing.readers.is_empty() != wanted { true => Poll::Ready(()), false => Poll::Pending, } @@ -598,12 +598,12 @@ impl Lister { } /// Ready once anyone lists the broadcast; [`Error::Closed`] once it ended. - pub fn poll_used(&self, waiter: &kio::Waiter) -> Poll> { + pub fn poll_wanted(&self, waiter: &kio::Waiter) -> Poll> { self.poll_readers(waiter, true) } /// Ready once nobody lists the broadcast; [`Error::Closed`] once it ended. - pub fn poll_unused(&self, waiter: &kio::Waiter) -> Poll> { + pub fn poll_unwanted(&self, waiter: &kio::Waiter) -> Poll> { self.poll_readers(waiter, false) } @@ -985,10 +985,10 @@ mod tests { let _own = broadcast.create_track("own", None).unwrap(); let lister = broadcast.requests().lister().unwrap(); let waiter = kio::Waiter::noop(); - assert!(lister.poll_used(&waiter).is_pending()); + assert!(lister.poll_wanted(&waiter).is_pending()); let mut tracks = broadcast.consume().tracks().unwrap(); - assert!(matches!(lister.poll_used(&waiter), Poll::Ready(Ok(())))); + assert!(matches!(lister.poll_wanted(&waiter), Poll::Ready(Ok(())))); assert_eq!(drain(&mut tracks), ["+own"], "not synced until the lister says so"); lister.add("up", track::Info::default()).unwrap(); assert!(matches!( @@ -1002,9 +1002,9 @@ mod tests { assert!(matches!(lister.remove("up"), Err(Error::NotFound))); assert_eq!(drain(&mut tracks), ["-up"]); - assert!(lister.poll_unused(&waiter).is_pending()); + assert!(lister.poll_unwanted(&waiter).is_pending()); drop(tracks); - assert!(matches!(lister.poll_unused(&waiter), Poll::Ready(Ok(())))); + assert!(matches!(lister.poll_unwanted(&waiter), Poll::Ready(Ok(())))); } #[test] @@ -1032,18 +1032,18 @@ mod tests { lister.abort(Error::NotFound); assert_eq!(drain(&mut tracks), [format!("error: {}", Error::NotFound)]); assert!( - matches!(lister.poll_unused(&waiter), Poll::Ready(Ok(()))), + matches!(lister.poll_unwanted(&waiter), Poll::Ready(Ok(()))), "a failed reader is not demand" ); let mut again = broadcast.consume().tracks().unwrap(); - assert!(matches!(lister.poll_used(&waiter), Poll::Ready(Ok(())))); + assert!(matches!(lister.poll_wanted(&waiter), Poll::Ready(Ok(())))); assert_eq!(drain(&mut again), NONE, "a fresh answer"); lister.synced(); assert_eq!(drain(&mut again), ["synced"]); drop(tracks); assert!( - lister.poll_unused(&waiter).is_pending(), + lister.poll_unwanted(&waiter).is_pending(), "a failed reader releases nothing" ); } @@ -1135,10 +1135,10 @@ mod tests { assert_eq!(drain(&mut tracks), ["+up", "synced"]); lister.end(); assert_eq!(drain(&mut tracks), ["end"]); - assert!(matches!(lister.poll_unused(&waiter), Poll::Ready(Ok(())))); + assert!(matches!(lister.poll_unwanted(&waiter), Poll::Ready(Ok(())))); let mut again = broadcast.consume().tracks().unwrap(); - assert!(matches!(lister.poll_used(&waiter), Poll::Ready(Ok(())))); + assert!(matches!(lister.poll_wanted(&waiter), Poll::Ready(Ok(())))); assert_eq!(drain(&mut again), NONE, "the finished answer was withdrawn"); } @@ -1163,7 +1163,7 @@ mod tests { let lister = broadcast.requests().lister().unwrap(); broadcast.close(); assert!(matches!( - lister.poll_used(&kio::Waiter::noop()), + lister.poll_wanted(&kio::Waiter::noop()), Poll::Ready(Err(Error::Closed)) )); assert!(matches!(lister.add("a", track::Info::default()), Err(Error::Closed))); diff --git a/rs/moq-net/src/model/origin.rs b/rs/moq-net/src/model/origin.rs index 8ae96cf62a8..000ea5b280a 100644 --- a/rs/moq-net/src/model/origin.rs +++ b/rs/moq-net/src/model/origin.rs @@ -3132,7 +3132,7 @@ struct TrackIo { /// error, so the track ends cleanly if nothing takes over. closed: bool, /// Whether the track had a reader as of the last demand edge. - used: bool, + wanted: bool, } impl TrackIo { @@ -3228,7 +3228,7 @@ async fn run_front(task: FrontTask, origin: TasksWeak) { return Poll::Ready(()); } for io in std::mem::take(&mut in_flight) { - match io.weak.poll_unused(waiter) { + match io.weak.poll_unwanted(waiter) { Poll::Pending => in_flight.push(io), Poll::Ready(_) => io.release(), } @@ -3647,15 +3647,15 @@ async fn serve_front(task: FrontTask) -> Vec { } Action::Forget { track: name } => { // A reader that looked the track up since the machine decided keeps - // it. Feed its `Used` edge here: the demand poll only sees the + // it. Feed its `Wanted` edge here: the demand poll only sees the // current level, so a reader gone before the next poll would // otherwise leave the track unread with no linger armed. if let Some(io) = tracks.get_mut(&name) - && !io.weak.abort_unused(Error::Cancel) + && !io.weak.abort_unwanted(Error::Cancel) { - if !io.used { - io.used = true; - events.push_back(Event::Used { track: name }); + if !io.wanted { + io.wanted = true; + events.push_back(Event::Wanted { track: name }); } continue; } @@ -3710,25 +3710,25 @@ async fn serve_front(task: FrontTask) -> Vec { broadcast.close(); let mut in_flight = Vec::new(); for (_, mut io) in tracks.drain() { - let used = io.weak.is_used(); + let wanted = io.weak.is_wanted(); // A reader still waiting on its source's answer is in flight // too: serve it the copy it asked, so it ends as that copy does. let waiting = io.staged.take().or_else(|| io.query.take()); if io.copy.is_none() - && used && let Some(asked) = waiting + && wanted && let Some(asked) = waiting { io.splice(asked); } // Nothing in flight: unread, or nothing serving it. One whose copy // went with a session closed locally ends cleanly. - if !used || io.copy.is_none() { - io.end(match io.closed && used { + if !wanted || io.copy.is_none() { + io.end(match io.closed && wanted { true => Ok(()), false => Err(err.clone()), }); // An unread track the front has not parked yet still holds // its copy, which would keep its source subscribed. - if !used { + if !wanted { io.routes.release(); } continue; @@ -3784,12 +3784,12 @@ async fn serve_front(task: FrontTask) -> Vec { // the tracks it finished last before its close detaches it. match &mut listing { None => { - if front.serving().is_some() && matches!(lister.poll_used(waiter), Poll::Ready(Ok(()))) { + if front.serving().is_some() && matches!(lister.poll_wanted(waiter), Poll::Ready(Ok(()))) { return Poll::Ready(Step::Listing); } } Some(state) => { - if lister.poll_unused(waiter).is_ready() { + if lister.poll_unwanted(waiter).is_ready() { return Poll::Ready(Step::Unlisted); } if state.source != front.serving() { @@ -3852,11 +3852,11 @@ async fn serve_front(task: FrontTask) -> Vec { } // Watch the demand edge in whichever direction is unmet. Only `Pending` // parks, so an answer steps and the handler looks again, polling anew when - // nothing changed. A closed track is unread: it owes one `Unused` if + // nothing changed. A closed track is unread: it owes one `Unwanted` if // recorded read, and is no edge after. - let edge = match io.used { - true => io.weak.poll_unused(waiter).is_ready(), - false => matches!(io.weak.poll_used(waiter), Poll::Ready(Ok(()))), + let edge = match io.wanted { + true => io.weak.poll_unwanted(waiter).is_ready(), + false => matches!(io.weak.poll_wanted(waiter), Poll::Ready(Ok(()))), }; if edge { return Some(Step::Demand(name.clone())); @@ -3907,7 +3907,7 @@ async fn serve_front(task: FrontTask) -> Vec { copy: None, held: None, closed: false, - used: false, + wanted: false, }, ); Event::TrackAssigned { @@ -3982,7 +3982,7 @@ async fn serve_front(task: FrontTask) -> Vec { // Staged only while the track has a reader: without one the machine // will not splice, and a held copy would keep the source subscribed. if let Ok(info) = &result - && io.used + && io.wanted { io.staged = Some(Asked { info: Some(info.clone()), @@ -4014,19 +4014,19 @@ async fn serve_front(task: FrontTask) -> Vec { let Some(io) = tracks.get_mut(&name) else { continue }; // Read again: the edge may have flipped back since it fired, and the next // wait parks on it. - if io.weak.is_used() == io.used { + if io.weak.is_wanted() == io.wanted { continue; } - io.used = !io.used; - if !io.used { + io.wanted = !io.wanted; + if !io.wanted { // Nothing will be spliced now: let go of the copies a query // holds, or the source stays subscribed with nobody reading. io.query = None; io.staged = None; } - match io.used { - true => Event::Used { track: name }, - false => Event::Unused { + match io.wanted { + true => Event::Wanted { track: name }, + false => Event::Unwanted { track: name, now: timers.now(), }, @@ -7414,7 +7414,7 @@ mod tests { copy: None, held: None, closed: false, - used: true, + wanted: true, }; let mut subscriber = consumer.subscribe(None).now_or_never().unwrap().unwrap(); io.end(Ok(())); @@ -9527,7 +9527,7 @@ mod tests { queued(&server).await.accept(&upstream); let resolved = pending.await.expect("resolves"); let reader = resolved.track("video").unwrap().subscribe(None); - settle(|| source.demand().is_used()).await; + settle(|| source.demand().is_wanted()).await; // The source refuses the track before delivering, which ends it for the reader. source.abort(Error::NotFound).unwrap(); @@ -12991,7 +12991,7 @@ mod tests { for _ in 0..10 { moq_clock::sim::yield_now().await; } - assert!(!incompatible.demand().is_used(), "the refused copy is still subscribed"); + assert!(!incompatible.demand().is_wanted(), "the refused copy is still subscribed"); // The refusal happened: the incumbent leaving surfaces it. drop(track); diff --git a/rs/moq-net/src/model/resume.rs b/rs/moq-net/src/model/resume.rs index 5c5c324445e..fad321bd098 100644 --- a/rs/moq-net/src/model/resume.rs +++ b/rs/moq-net/src/model/resume.rs @@ -5542,12 +5542,12 @@ mod test { let logical = logical(&routes); let demand = logical.demand(); let mut fetched = logical.consume().fetch_group(0, None).now_or_never().unwrap().unwrap(); - assert!(demand.is_used(), "an open fetched group holds the track"); + assert!(demand.is_wanted(), "an open fetched group holds the track"); let mut frame = fetched.next_frame().now_or_never().unwrap().unwrap().unwrap(); drop(fetched); - assert!(demand.is_used(), "an unread frame holds the track"); + assert!(demand.is_wanted(), "an unread frame holds the track"); assert_eq!(frame.read_all().now_or_never().unwrap().unwrap(), b"a".as_ref()); - assert!(!demand.is_used(), "a read frame does not"); + assert!(!demand.is_wanted(), "a read frame does not"); } /// A rerouted fetch keeps its own age budget: the new route is asked with it, and a diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 740a498034d..0057da0c4e1 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -3423,13 +3423,13 @@ impl Producer { Ok(()) } - /// Abort an unused track, returning the producer unchanged if consumers remain. + /// Abort an unwanted track, returning the producer unchanged if consumers remain. /// - /// Consumer creation and the unused check share a lock, so demand returning + /// Consumer creation and the unwanted check share a lock, so demand returning /// after [`Demand::wanted`] went to `None` prevents the abort. `Ok(())` means /// the track is closed, including when it was already closed; existing handles /// may still observe its final state. `Err(producer)` leaves the track unchanged - /// so the caller can continue serving and wait for the next unused wake. + /// so the caller can continue serving and wait for the next unwanted wake. /// /// The abort is the same as [`Self::abort`]: a group consumer that outlived the track's /// consumers ends with `err`, and the group's writer fails. @@ -3440,16 +3440,16 @@ impl Producer { reason = "return the owned producer without allocating on an idle check" ) )] - pub fn abort_unused(self, err: Error) -> std::result::Result<(), Self> { - let used = match self.state.write_unused() { - kio::Unused::Idle(guard) => { + pub fn abort_unwanted(self, err: Error) -> std::result::Result<(), Self> { + let wanted = match self.state.write_unwanted() { + kio::Unwanted::Idle(guard) => { commit_abort(guard, err).abort(); false } - kio::Unused::Closed => false, - kio::Unused::Used => true, + kio::Unwanted::Closed => false, + kio::Unwanted::Wanted => true, }; - if used { + if wanted { return Err(self); } // After the track's own lock is released; a later track of the name, listed in @@ -3900,7 +3900,7 @@ impl TrackWeak { /// Join the cached track's demand, or `None` once it has closed. /// /// The count moves under the same lock the close takes, which is the other half - /// of [`Producer::abort_unused`]: a request either joins in time to decline the + /// of [`Producer::abort_unwanted`]: a request either joins in time to decline the /// teardown, or gets nothing and re-requests the track. It is never handed a track /// that is already on its way out. pub(super) fn try_consume( @@ -3954,8 +3954,8 @@ impl TrackWeak { /// Whether anyone is consuming the track right now. A closed track doesn't /// count even if consumers linger to drain its cache: no new work is owed. - pub(super) fn is_used(&self) -> bool { - !self.state.is_closed() && self.state.is_used() + pub(super) fn is_wanted(&self) -> bool { + !self.state.is_closed() && self.state.wanted() } /// The readers' aggregate demand and contribution count, or `None` without one. @@ -3970,32 +3970,32 @@ impl TrackWeak { /// End the track with `err` unless a reader holds it, atomically with lookups: a /// lookup either gets a consumer in time to keep it, or finds it closed and asks /// afresh. True once the track is gone. - pub(super) fn abort_unused(&self, err: Error) -> bool { + pub(super) fn abort_unwanted(&self, err: Error) -> bool { let Some(producer) = self.state.produce() else { return true; }; - match producer.write_unused() { - kio::Unused::Idle(guard) => { + match producer.write_unwanted() { + kio::Unwanted::Idle(guard) => { commit_abort(guard, err).abort(); true } - kio::Unused::Closed => true, - kio::Unused::Used => false, + kio::Unwanted::Closed => true, + kio::Unwanted::Wanted => false, } } /// `Ready(Ok)` while anyone reads the open track, `Ready(Err)` once it closed, and /// otherwise park `waiter` for the next reader. Only `Pending` registers: act on the - /// answer, not a later [`Self::is_used`], or a reader leaving in between is never seen. - pub(super) fn poll_used(&self, waiter: &kio::Waiter) -> Poll> { - self.state.poll_used(waiter) + /// answer, not a later [`Self::is_wanted`], or a reader leaving in between is never seen. + pub(super) fn poll_wanted(&self, waiter: &kio::Waiter) -> Poll> { + self.state.poll_wanted_changed(waiter, false).map_ok(drop) } /// `Ready(Ok)` while nobody reads the track, `Ready(Err)` once it closed, and /// otherwise park `waiter` for the last reader leaving. Only `Pending` registers, - /// as with [`Self::poll_used`]. - pub(super) fn poll_unused(&self, waiter: &kio::Waiter) -> Poll> { - self.state.poll_unused(waiter) + /// as with [`Self::poll_wanted`]. + pub(super) fn poll_unwanted(&self, waiter: &kio::Waiter) -> Poll> { + self.state.poll_wanted_changed(waiter, true).map_ok(drop) } } @@ -4055,7 +4055,7 @@ impl Demand { /// moves the ask past. `n` never goes back, so once a group reaches it nothing is /// pending until another join asks. pub fn wanted(&self) -> Option { - if !self.is_used() { + if !self.is_wanted() { return None; } let wanted = self.subscriptions().lock().wanted(); @@ -4080,13 +4080,13 @@ impl Demand { /// Poll for [`wanted`](Self::wanted) to differ from what this handle last returned. pub fn poll_wanted_changed(&mut self, waiter: &kio::Waiter) -> Poll>> { - let used = match self.poll_state(waiter) { + let active = match self.poll_state(waiter) { DemandState::Closed => return Poll::Ready(Err(self.abort_reason())), DemandState::Active => true, DemandState::Idle => false, }; let wanted = self.subscriptions().lock().poll_wanted(waiter); - let next = used.then_some(wanted); + let next = active.then_some(wanted); if self.prev == Some(next) { return Poll::Pending; } @@ -4118,8 +4118,8 @@ impl Demand { } /// Whether the track is open and anyone consumes it right now, without waiting. - pub(super) fn is_used(&self) -> bool { - !self.state.is_closed() && self.state.is_used() + pub(super) fn is_wanted(&self) -> bool { + !self.state.is_closed() && self.state.wanted() } /// Whether the track is gone, without waiting. @@ -4134,17 +4134,17 @@ impl Demand { /// to track which edge it's waiting for. pub(super) fn poll_state(&self, waiter: &kio::Waiter) -> DemandState { loop { - match self.state.poll_used(waiter) { + match self.state.poll_wanted_changed(waiter, false) { Poll::Ready(Err(_)) => return DemandState::Closed, - // Not used, and armed for it becoming used. + // Not wanted, and armed for it becoming wanted. Poll::Pending => return DemandState::Idle, - // Used, so arm for the reverse. - Poll::Ready(Ok(())) => match self.state.poll_unused(waiter) { + // Wanted, so arm for the reverse. + Poll::Ready(Ok(_)) => match self.state.poll_wanted_changed(waiter, true) { Poll::Ready(Err(_)) => return DemandState::Closed, Poll::Pending => return DemandState::Active, // Went idle between the two reads, so neither poll armed anything. // Start over rather than returning a state with no waker behind it. - Poll::Ready(Ok(())) => continue, + Poll::Ready(Ok(_)) => continue, }, } } @@ -8120,14 +8120,14 @@ impl Request { /// 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, reason = "async-only"))] - pub(crate) fn reject_unused(&self, err: Error) -> bool { - match self.state.write_unused() { - kio::Unused::Idle(guard) => { + pub(crate) fn reject_unwanted(&self, err: Error) -> bool { + match self.state.write_unwanted() { + kio::Unwanted::Idle(guard) => { commit_abort(guard, err).abort(); true } - kio::Unused::Closed => true, - kio::Unused::Used => false, + kio::Unwanted::Closed => true, + kio::Unwanted::Wanted => false, } } @@ -13036,7 +13036,7 @@ mod test { assert!(group.read_frame().now_or_never().unwrap().unwrap().is_some()); drop(consumer); - assert!(producer.abort_unused(Error::Cancel).is_ok()); + assert!(producer.abort_unwanted(Error::Cancel).is_ok()); let read = group.read_frame().now_or_never().expect("should not block"); assert!(matches!(read, Err(Error::Cancel)), "{read:?}"); let write = writer.write_frame(Timestamp::ZERO, bytes::Bytes::from_static(b"y")); @@ -15235,9 +15235,9 @@ mod test { let second = consumer.fetch_group(5, None); drop(first); - assert!(req.poll_unused(&kio::Waiter::noop()).is_pending()); + assert!(req.poll_unwanted(&kio::Waiter::noop()).is_pending()); drop(second); - assert!(req.poll_unused(&kio::Waiter::noop()).is_ready()); + assert!(req.poll_unwanted(&kio::Waiter::noop()).is_ready()); let retry = consumer.fetch_group(5, None); let fresh = dynamic @@ -15246,7 +15246,7 @@ mod test { .expect("the retry queues a fresh request") .unwrap(); drop(req); - assert!(fresh.poll_unused(&kio::Waiter::noop()).is_pending()); + assert!(fresh.poll_unwanted(&kio::Waiter::noop()).is_pending()); fresh.accept(Info::timed()).unwrap().finish().unwrap(); assert_eq!(retry.await.unwrap().sequence, 5); } @@ -15328,7 +15328,7 @@ mod test { let mut fetched = retry.await.unwrap(); assert_eq!(&fetched.read_frame().await.unwrap().unwrap().payload[..], b"hi"); - assert!(!fresh.is_used(), "the retry left the fresh request"); + assert!(!fresh.is_wanted(), "the retry left the fresh request"); assert!(matches!(fresh.accept(Info::timed()), Err(Error::Duplicate))); } @@ -15364,7 +15364,7 @@ mod test { let hit = consumer.fetch_group(5, None); assert!( - !group.handle().abort_unused(Error::Cancel), + !group.handle().abort_unwanted(Error::Cancel), "the pending hit wants the group" ); let mut fetched = hit.await.unwrap(); @@ -15796,7 +15796,7 @@ mod test { .unwrap(); let group = req.accept(Info::timed()).unwrap(); drop(first.await.unwrap()); - assert!(group.handle().abort_unused(Error::Cancel)); + assert!(group.handle().abort_unwanted(Error::Cancel)); let retry = consumer.fetch_group(5, None); let req = dynamic @@ -17640,12 +17640,12 @@ mod test { let dynamic = request.requests(); let demand = request.demand(); let consumer = request.consume(); - assert!(!demand.is_used(), "a bare handle is not demand"); + assert!(!demand.is_wanted(), "a bare handle is not demand"); let query = consumer.info(); - assert!(demand.is_used()); - assert!(dynamic.demand().is_used()); + assert!(demand.is_wanted()); + assert!(dynamic.demand().is_wanted()); drop(query); - assert!(!demand.is_used()); + assert!(!demand.is_wanted()); let producer = request.accept(Info::timed()); assert_eq!(producer.demand().name(), demand.name()); drop(producer); @@ -17671,11 +17671,12 @@ mod test { }; assert!(dynamic.poll_requested_fill(&waiter).is_pending(), "the second joined"); assert_eq!((fill.start(), fill.end()), (Position::group(3), 4)); - assert!(fill.is_used()); + assert_eq!(fill.wanted(), Some(3)); drop(first); - assert!(fill.poll_unused(&waiter).is_pending()); + assert!(fill.poll_withdrawn(&waiter).is_pending()); drop(second); - assert!(fill.poll_unused(&waiter).is_ready()); + assert!(fill.poll_withdrawn(&waiter).is_ready()); + assert_eq!(fill.wanted(), None); drop(fill); assert!( !producer.state.read().drops.contains_key(&3), @@ -17785,7 +17786,7 @@ mod test { let Poll::Ready(Ok(fill)) = dynamic.poll_requested_fill(&waiter) else { panic!("below the start is a miss"); }; - while let Some(sequence) = fill.next() { + while let Some(sequence) = fill.wanted() { write_filled(fill.accept(sequence, None), sequence); } let mut read = Vec::new(); @@ -17823,7 +17824,7 @@ mod test { // Group 2 is as stale as the groups below it, superseded by group 3 as long ago. assert_eq!(next(&mut subscriber), Some(3)); assert_eq!(drained_drops(&mut subscriber), [(0, 2)]); - assert!(!fill.is_used(), "nobody waits on the stale fill"); + assert_eq!(fill.wanted(), None, "nobody waits on the stale fill"); } /// Groups arriving out of order inside a stalled fill judge each stretch between them by @@ -18020,7 +18021,43 @@ mod test { }; assert_eq!(group.sequence, 0); drop(group); - assert!(fill.poll_unused(&waiter).is_ready(), "nobody waits on the fill now"); + assert!(fill.poll_withdrawn(&waiter).is_ready(), "nobody waits on the fill now"); + } + + /// A parked [`Fill::poll_wanted_changed`] wakes on every move of the group owed: a + /// reader narrowing past it, an answer, and the last reader leaving, which ends at `None`. + #[test] + fn fill_wanted_changed_follows_the_group_owed() { + let mut producer = track_producer("test", Info::timed()); + let dynamic = producer.requests(); + let ascending = |groups| replay().with_range(groups).with_order(Order::Ascending); + let mut subscriber = producer.subscribe(ascending(0..)); + producer.start_at(10).unwrap(); + write_group(&producer, 10); + let noop = kio::Waiter::noop(); + while let Poll::Ready(Ok(Some(_))) = subscriber.poll_recv_group(&noop) {} + let Poll::Ready(Ok(mut fill)) = dynamic.poll_requested_fill(&noop) else { + panic!("below the start is a miss"); + }; + let (flag, waiter) = Flag::waiter(); + assert_eq!(fill.poll_wanted_changed(&waiter), Poll::Ready(Some(0)), "the first poll reports"); + assert!(fill.poll_wanted_changed(&waiter).is_pending()); + + subscriber.update(ascending(8..)).unwrap(); + assert!(subscriber.poll_recv_group(&noop).is_pending()); + assert!(flag.take(), "narrowing woke the parked poll"); + assert_eq!(fill.poll_wanted_changed(&waiter), Poll::Ready(Some(8))); + assert!(fill.poll_wanted_changed(&waiter).is_pending()); + + write_filled(fill.accept(8, None), 8); + assert!(flag.take(), "an answer woke the parked poll"); + assert_eq!(fill.poll_wanted_changed(&waiter), Poll::Ready(Some(9))); + assert!(fill.poll_wanted_changed(&waiter).is_pending()); + + drop(subscriber); + assert!(flag.take(), "the last reader leaving woke the parked poll"); + assert_eq!(fill.poll_wanted_changed(&waiter), Poll::Ready(None)); + assert_eq!(fill.wanted(), None); } /// A reader narrowing its ranges leaves the part of its fill it no longer reads: the @@ -18040,7 +18077,7 @@ mod test { let Poll::Ready(Ok(fill)) = dynamic.poll_requested_fill(&waiter) else { panic!("below the start is a miss"); }; - assert_eq!(fill.next(), Some(0)); + assert_eq!(fill.wanted(), Some(0)); assert!(fill.is_wanted(0)); assert!(fill.poll_unwanted(0, &waiter).is_pending()); @@ -18050,9 +18087,9 @@ mod test { assert!(!fill.is_wanted(0)); assert!(fill.is_wanted(8)); assert!(fill.poll_unwanted(8, &waiter).is_pending()); - assert_eq!(fill.next(), Some(8), "nobody reads below 8 any more"); + assert_eq!(fill.wanted(), Some(8), "nobody reads below 8 any more"); write_filled(fill.accept(8, None), 8); - assert_eq!(fill.next(), Some(9)); + assert_eq!(fill.wanted(), Some(9)); // Over, so what the fill refuses is a drop rather than a group still to come. producer.finish().unwrap(); fill.drop_groups(0..9); @@ -18119,12 +18156,12 @@ mod test { let group = fill.group(0); let readers = group.readers(); let _writing = group.accept(None).unwrap(); - assert!(readers.poll_unused(&waiter).is_pending()); + assert!(readers.poll_unwanted(&waiter).is_pending()); subscriber.update(ascending(1..)).unwrap(); assert!(subscriber.poll_recv_group(&waiter).is_pending()); - assert!(readers.poll_unused(&waiter).is_ready(), "nobody reads group 0 now"); - assert!(fill.poll_unused(&waiter).is_pending(), "the fill still owes 1 and up"); + assert!(readers.poll_unwanted(&waiter).is_ready(), "nobody reads group 0 now"); + assert!(fill.poll_withdrawn(&waiter).is_pending(), "the fill still owes 1 and up"); } /// A time start before every group held asks no fill below the oldest, since which of @@ -18157,7 +18194,7 @@ mod test { (0, 2), "nothing below 10 for the time start" ); - while let Some(sequence) = fill.next() { + while let Some(sequence) = fill.wanted() { write_filled(fill.accept(sequence, None), sequence); } drop(fill); @@ -18183,7 +18220,7 @@ mod test { }; assert_eq!((fill.start().group, fill.end()), (0, 100)); for sequence in 0..50 { - assert_eq!(fill.next(), Some(sequence)); + assert_eq!(fill.wanted(), Some(sequence)); fill.group(sequence).reject(Error::App(sequence as u16)); let _ = subscriber.poll_recv_group(&waiter); assert!(fill.answers() <= 1, "group {sequence}: {} answers held", fill.answers()); @@ -18215,7 +18252,7 @@ mod test { panic!("what the first fill stopped owing is asked afresh"); }; assert_eq!((again.start().group, again.end()), (0, 8)); - assert_eq!(fill.next(), Some(8), "the first fill still owes only 8 and up"); + assert_eq!(fill.wanted(), Some(8), "the first fill still owes only 8 and up"); assert!(!fill.is_wanted(0), "a group stops being wanted for good"); } @@ -18417,11 +18454,11 @@ mod test { panic!("the miss below the start is asked"); }; for _ in 0..REFUSED { - let sequence = fill.next().unwrap(); + let sequence = fill.wanted().unwrap(); fill.drop_groups(sequence..sequence + 1); assert!(subscriber.poll_recv_group(&kio::Waiter::noop()).is_pending()); } - assert_eq!(fill.next(), Some(WIDE - REFUSED - 1)); + assert_eq!(fill.wanted(), Some(WIDE - REFUSED - 1)); } /// A descending reader joining a queued fill makes it newest first, as any descending @@ -18473,7 +18510,7 @@ mod test { (3, Order::Ascending), "refreshed while queued" ); - assert_eq!(fill.next(), Some(0)); + assert_eq!(fill.wanted(), Some(0)); write_filled(fill.accept(0, None), 0); let budget = Duration::from_secs(20); @@ -18487,7 +18524,7 @@ mod test { assert_eq!(fill.priority(), -2); assert_eq!(fill.max_delay(), Some(budget)); assert_eq!(fill.order(), Order::Descending); - assert_eq!(fill.next(), Some(3), "the rest is served newest first"); + assert_eq!(fill.wanted(), Some(3), "the rest is served newest first"); } /// An ordered reader waits on a fill for its miss rather than skipping past it. @@ -18525,7 +18562,7 @@ mod test { panic!("the gap is a miss"); }; drop(subscriber); - assert!(!fill.is_used()); + assert_eq!(fill.wanted(), None); drop(fill); assert!(drops(&producer).is_empty()); } diff --git a/rs/moq-net/src/model/track/fill.rs b/rs/moq-net/src/model/track/fill.rs index 49231da69fd..d91f2da2039 100644 --- a/rs/moq-net/src/model/track/fill.rs +++ b/rs/moq-net/src/model/track/fill.rs @@ -343,7 +343,7 @@ impl Drop for Join { for run in std::mem::take(&mut self.held).iter() { fills.unwant(self.id, run.clone()); } - if !self.attempt.is_used() { + if !self.attempt.wanted() { fills.withdraw(self.id); } } @@ -664,7 +664,10 @@ pub(super) fn poll_requested( asks, result, }; - return Poll::Ready(Ok(Fill { inner: Arc::new(inner) })); + return Poll::Ready(Ok(Fill { + inner: Arc::new(inner), + prev: None, + })); } // No fill queued: surface a track abort so the handler loop can exit. @@ -686,11 +689,26 @@ pub(super) fn poll_requested( /// change, so read them as each group is served. The handler calls [`accept`](Self::accept) for each group /// it has and [`drop_groups`](Self::drop_groups) for the rest. Dropping the fill names /// whatever is left as dropped while a reader still wants it; once nobody does, the fill is -/// withdrawn and records nothing, so stop serving it when [`poll_unused`](Self::poll_unused) is ready. +/// withdrawn and records nothing, so stop serving it once [`wanted`](Self::wanted) is `None`. /// A reader narrowing its ranges may leave a single group, so stop serving that one when -/// [`poll_unwanted`](Self::poll_unwanted) is ready and move on to [`next`](Self::next). +/// [`poll_unwanted`](Self::poll_unwanted) is ready and move on to [`wanted`](Self::wanted). +/// +/// A clone serves the same fill, and the fill is settled once the last clone drops. Each +/// clone watches on its own, so its first [`poll_wanted_changed`](Self::poll_wanted_changed) +/// reports the current value. pub struct Fill { inner: Arc, + // The value `poll_wanted_changed` last returned, or `None` before it returned any. + prev: Option>, +} + +impl Clone for Fill { + fn clone(&self) -> Self { + Self { + inner: self.inner.clone(), + prev: None, + } + } } /// The state a [`Fill`] and its per-group handles share. The last one dropping settles what @@ -769,20 +787,10 @@ impl Fill { self.inner.refuse(&owed, err, self.max_delay()); } - /// Whether a reader still waits on this fill. - pub fn is_used(&self) -> bool { - self.inner.result.is_used() - } - - /// Wait until no reader waits on this fill: it is withdrawn, so stop serving it. - #[cfg(feature = "async")] - pub async fn unused(&self) { - kio::wait(|waiter| self.poll_unused(waiter)).await - } - - /// Poll until no reader waits on this fill. - pub fn poll_unused(&self, waiter: &kio::Waiter) -> Poll<()> { - self.inner.result.poll_unused(waiter).map(|_| ()) + /// Poll until no reader waits on this fill: it is withdrawn, so stop serving it. + #[cfg_attr(not(feature = "async"), allow(dead_code, reason = "async-only"))] + pub(crate) fn poll_withdrawn(&self, waiter: &kio::Waiter) -> Poll<()> { + self.inner.result.poll_wanted_changed(waiter, true).map(drop) } /// Whether a reader still waits for group `sequence` from this fill: false once every @@ -812,8 +820,12 @@ impl Fill { } /// The next group still owed in the fill's order: a reader still reads it, and it was - /// neither answered nor delivered live since it was asked. `None` once none is owed. - pub fn next(&self) -> Option { + /// neither answered nor delivered live since it was asked. `None` once none is owed or + /// no reader waits on the fill, so stop serving it. + pub fn wanted(&self) -> Option { + if !self.inner.result.wanted() { + return None; + } loop { let wanted = self.inner.wanted(); let sequence = { @@ -843,6 +855,32 @@ impl Fill { } } + /// Return [`wanted`](Self::wanted) once it differs from what this handle last returned: + /// immediately on the first call, then on each change. + /// + /// So `while let Some(sequence) = fill.wanted_changed().await` serves each group as it + /// becomes owed, and ends once the fill is withdrawn or answered. + #[cfg(feature = "async")] + pub async fn wanted_changed(&mut self) -> Option { + kio::wait(|waiter| self.poll_wanted_changed(waiter)).await + } + + /// Poll for [`wanted`](Self::wanted) to differ from what this handle last returned. + pub fn poll_wanted_changed(&mut self, waiter: &kio::Waiter) -> Poll> { + // Arm every input of `wanted` before reading it, so a change in between still wakes: + // the readers leaving, narrowing, or taking a group, the answers, and the order. + let _ = self.inner.result.poll_wanted_changed(waiter, true); + let _ = self.inner.result.poll_ref(waiter, |_| Poll::<()>::Pending); + let _ = self.inner.fills.poll(waiter, |_| Poll::Pending); + let _ = self.inner.asks.poll(waiter, |_| Poll::Pending); + let wanted = self.wanted(); + if self.prev == Some(wanted) { + return Poll::Pending; + } + self.prev = Some(wanted); + Poll::Ready(wanted) + } + /// Refuse every group still owed with `err`, which says nothing about whether the source /// has them, so nothing is recorded dropped. #[cfg_attr(not(feature = "async"), allow(dead_code, reason = "async-only"))] @@ -851,12 +889,12 @@ impl Fill { self.inner.refuse(&unresolved, err, self.max_delay()); } - /// The next run of groups still owed, in the fill's order: [`Self::next`]'s group, + /// The next run of groups still owed, in the fill's order: [`Self::wanted`]'s group, /// extended over the owed groups beside it up to the nearest one the cache holds, so a /// session asks its source for the whole run at once. #[cfg_attr(not(feature = "async"), allow(dead_code, reason = "async-only"))] pub(crate) fn next_run(&self) -> Option> { - let sequence = self.next()?; + let sequence = self.wanted()?; let owed = self.inner.owed_around(sequence); let state = self.inner.state.read(); let held = |(_, slot): &(&u64, &super::Slot)| !slot.is_aborted(); @@ -895,7 +933,7 @@ impl Fill { /// Poll until no reader waits for any group in `groups` from this fill. #[cfg_attr(not(feature = "async"), allow(dead_code, reason = "async-only"))] pub(crate) fn poll_unwanted_groups(&self, groups: ops::Range, waiter: &kio::Waiter) -> Poll<()> { - if self.inner.result.poll_unused(waiter).is_ready() { + if self.poll_withdrawn(waiter).is_ready() { return Poll::Ready(()); } let id = self.inner.id; @@ -962,7 +1000,7 @@ impl Inner { } fn is_wanted(&self, sequence: u64) -> bool { - self.result.is_used() && self.fills.read().wants(self.id, sequence) + self.result.wanted() && self.fills.read().wants(self.id, sequence) } fn max_delay(&self) -> Option { @@ -970,7 +1008,7 @@ impl Inner { } fn poll_unwanted(&self, sequence: u64, waiter: &kio::Waiter) -> Poll<()> { - if self.result.poll_unused(waiter).is_ready() { + if self.result.poll_wanted_changed(waiter, true).is_ready() { return Poll::Ready(()); } self.fills @@ -1166,7 +1204,7 @@ impl Drop for Inner { let outcome = self.result.read(); wanted.iter().flat_map(|run| outcome.resolved.gaps(run)).collect() }; - if !left.is_empty() && self.result.is_used() { + if !left.is_empty() && self.result.wanted() { let asked = self.max_delay(); if let Ok(mut state) = TrackState::modify(&self.state) { // Unanswered is a miss downstream, as an explicit `drop_groups` would say. @@ -1218,8 +1256,8 @@ pub(crate) struct Readers { impl Readers { /// Poll until no reader waits on the group. Once the fill ends its per-group demand goes /// with it, so then until none of its readers is left. - pub(crate) fn poll_unused(&self, waiter: &kio::Waiter) -> Poll<()> { - if self.outcome.poll_unused(waiter).is_ready() { + pub(crate) fn poll_unwanted(&self, waiter: &kio::Waiter) -> Poll<()> { + if self.outcome.poll_wanted_changed(waiter, true).is_ready() { return Poll::Ready(()); } self.fills @@ -1277,14 +1315,14 @@ impl GroupFill { } /// Poll until no reader waits on this group, as [`Fill::poll_unwanted`]. - pub(crate) fn poll_unused(&self, waiter: &kio::Waiter) -> Poll<()> { + pub(crate) fn poll_unwanted(&self, waiter: &kio::Waiter) -> Poll<()> { self.inner.poll_unwanted(self.sequence, waiter) } /// Whether a reader still waits on the fill. #[cfg(test)] - pub(crate) fn is_used(&self) -> bool { - self.inner.result.is_used() + pub(crate) fn is_wanted(&self) -> bool { + self.inner.result.wanted() } /// The group's readers, for a session still writing it once accepted. Holds the fill's diff --git a/rs/moq-net/src/server.rs b/rs/moq-net/src/server.rs index 0eb8d2b6d3c..c387f3a7234 100644 --- a/rs/moq-net/src/server.rs +++ b/rs/moq-net/src/server.rs @@ -42,14 +42,14 @@ impl Activity { /// Wait until every admitted connection has left, then refuse new ones. pub async fn close(&self) { loop { - if self.state.unused().await.is_err() { + if self.state.wanted_changed(true).await.is_err() { return; } - match self.state.write_unused() { - kio::Unused::Idle(state) => return state.close(), + match self.state.write_unwanted() { + kio::Unwanted::Idle(state) => return state.close(), // Admitted between the wake and the commit: wait for it too. - kio::Unused::Used => continue, - kio::Unused::Closed => return, + kio::Unwanted::Wanted => continue, + kio::Unwanted::Closed => return, } } } diff --git a/rs/moq-net/src/session.rs b/rs/moq-net/src/session.rs index 7af1a79edea..4ae8f5d8102 100644 --- a/rs/moq-net/src/session.rs +++ b/rs/moq-net/src/session.rs @@ -757,7 +757,7 @@ impl Supervisor { // before the protocol, so the estimate a consumer reads is at most one // interval old. let Some(estimate) = &self.estimate else { return }; - if estimate.poll_used(waiter).is_pending() { + if estimate.poll_wanted_changed(waiter, false).is_pending() { return; } let now = self.runtime.now(); @@ -778,8 +778,8 @@ impl Supervisor { SamplerMode::Idle => { // Demand is a bandwidth consumer appearing or a stats read. let mut demanded = match &self.send_bandwidth { - Some(producer) => match producer.poll_used(waiter) { - Poll::Ready(Ok(())) => true, + Some(producer) => match producer.poll_wanted_changed(waiter, false) { + Poll::Ready(Ok(_)) => true, Poll::Ready(Err(_)) => { self.send_bandwidth = None; false @@ -805,8 +805,11 @@ impl Supervisor { return; } // The interval elapsed: pause unless someone still cares. - let used = self.send_bandwidth.as_ref().is_some_and(bandwidth::Producer::is_used); - if !used && !self.stats.read().demanded { + let wanted = self + .send_bandwidth + .as_ref() + .is_some_and(|producer| producer.demand().wanted()); + if !wanted && !self.stats.read().demanded { self.mode = SamplerMode::Idle; continue; } diff --git a/rs/moq-net/tests/integration/fetch_cancel.rs b/rs/moq-net/tests/integration/fetch_cancel.rs index 3aaff468c8d..68dfbc85ca6 100644 --- a/rs/moq-net/tests/integration/fetch_cancel.rs +++ b/rs/moq-net/tests/integration/fetch_cancel.rs @@ -67,7 +67,7 @@ async fn abandoned_fetch_reaches_the_publisher(version: &str, relays: u64, stage }; let groups = request.requests(); let _track = request.accept(timed()); - let request = match futures::future::select(std::pin::pin!(groups.requested_fill()), &mut waiting).await { + let mut request = match futures::future::select(std::pin::pin!(groups.requested_fill()), &mut waiting).await { futures::future::Either::Left((request, _)) => request.expect("the fetch reaches the publisher"), futures::future::Either::Right(_) => panic!("nothing answered the fetch"), }; @@ -75,7 +75,7 @@ async fn abandoned_fetch_reaches_the_publisher(version: &str, relays: u64, stage match stage { Stage::Unanswered => { drop(waiting); - moq_clock::timeout(Duration::from_secs(5), kio::wait(|waiter| request.poll_unused(waiter))) + moq_clock::timeout(Duration::from_secs(5), async { while request.wanted_changed().await.is_some() {} }) .await .unwrap_or_else(|_| panic!("{ctx}: the publisher's fetch is still wanted")); } @@ -187,7 +187,7 @@ async fn a_refetch_within_the_linger_stays_on_the_relay(version: &str) { }; let groups = request.requests(); let _track = request.accept(timed()); - let request = match futures::future::select(std::pin::pin!(groups.requested_fill()), &mut waiting).await { + let mut request = match futures::future::select(std::pin::pin!(groups.requested_fill()), &mut waiting).await { futures::future::Either::Left((request, _)) => request.expect("the fetch reaches the publisher"), futures::future::Either::Right(_) => panic!("nothing answered the fetch"), }; diff --git a/rs/moq-net/tests/loom.rs b/rs/moq-net/tests/loom.rs index ad1121a64c4..83a235c47e9 100644 --- a/rs/moq-net/tests/loom.rs +++ b/rs/moq-net/tests/loom.rs @@ -343,7 +343,7 @@ fn concurrent_tracks_drain_a_shared_pool() { /// The teardown every wire subscriber runs when its copy of a track goes idle, /// racing a viewer coming back for the same name. /// -/// `poll_unused` is a level snapshot, so the teardown it wakes can only be trusted +/// `poll_wanted_changed` is a level snapshot, so the teardown it wakes can only be trusted /// at the instant it commits. The invariant is that the two can't both win: either /// the subscribe joins the live track and the teardown declines, or the teardown /// commits and the subscribe never sees the dead track at all. A subscription joined @@ -361,7 +361,7 @@ fn an_idle_teardown_never_cancels_a_returning_viewer() { let viewer = thread::spawn(move || consumer.track("video").map(|track| track.subscribe(None).into_inner())); // Held to the end: a declined teardown hands back the producer still serving. - let teardown = track.abort_unused(Error::Cancel); + let teardown = track.abort_unwanted(Error::Cancel); let subscribed = viewer.join().unwrap().and_then(|subscribing| { match subscribing.poll_ok(&kio::Waiter::noop()) { // A live track parks (nothing has been published); only an error matters. @@ -426,7 +426,7 @@ fn a_fetch_never_joins_a_withdrawn_attempt() { let leaver = thread::spawn(move || { drop(first); - request.is_used().then_some(request) + request.wanted().is_some().then_some(request) }); let mut retry = consumer.fetch_group(5, None); let request = match leaver.join().unwrap() { diff --git a/rs/moq-scene/src/source.rs b/rs/moq-scene/src/source.rs index b1e8c46c9c8..835e7c446cb 100644 --- a/rs/moq-scene/src/source.rs +++ b/rs/moq-scene/src/source.rs @@ -174,7 +174,7 @@ impl Driver { pub async fn run(self) { let status = self.status.clone(); let result = tokio::select! { - _ = status.unused() => return, + _ = status.wanted_changed(true) => return, result = self.produce() => result, }; let status = match result { diff --git a/rs/moq-stats/src/produce.rs b/rs/moq-stats/src/produce.rs index 41c7f0717e7..901f010e7a7 100644 --- a/rs/moq-stats/src/produce.rs +++ b/rs/moq-stats/src/produce.rs @@ -530,8 +530,8 @@ impl TrackPair { } } - /// Whether any consumer exists on either flavor. - fn is_used(&self) -> bool { + /// Whether anyone wants either flavor. + fn is_wanted(&self) -> bool { self.plain.demand().wanted().is_some() || self.compressed.demand().wanted().is_some() } @@ -1176,14 +1176,14 @@ impl NodePublisher { // Reclaim before adopting, so a request for a track just reclaimed is // served again by this very drain. self.presence.retain(|_, track| { - if track.pair.is_used() { + if track.pair.is_wanted() { return true; } track.pair.finish(); false }); self.prefixes.retain(|_, track| { - if track.pair.is_used() { + if track.pair.is_wanted() { return true; } track.pair.finish(); diff --git a/rs/moq-tokio/src/connection.rs b/rs/moq-tokio/src/connection.rs index 8886a1045ab..ef86edc8bde 100644 --- a/rs/moq-tokio/src/connection.rs +++ b/rs/moq-tokio/src/connection.rs @@ -1877,6 +1877,9 @@ async fn run_session( /// Own the live session's receive consumer only while an external consumer needs it. #[derive(Default)] struct RecvBandwidth { + // Watches `out`, taken on the first poll. + demand: Option, + wanted: bool, source: Option, } @@ -1887,23 +1890,27 @@ impl RecvBandwidth { waiter: &kio::Waiter, source: impl FnOnce() -> Option, ) { + let demand = self.demand.get_or_insert_with(|| out.demand()); + // Drain every edge, so `waiter` is armed for the next one. Demand can arrive during + // retirement, which the next turn of the loop reports. loop { - match out.poll_unused(waiter) { + match demand.poll_wanted_changed(waiter) { Poll::Pending => break, Poll::Ready(Err(_)) => { self.source = None; return; } - Poll::Ready(Ok(())) => { - self.source = None; - let _ = out.set(None); + Poll::Ready(Ok(wanted)) => { + self.wanted = wanted; + if !wanted { + self.source = None; + let _ = out.set(None); + } } } - match out.poll_used(waiter) { - // Demand can arrive during retirement, before a waiter was armed. - Poll::Ready(Ok(())) => continue, - Poll::Ready(Err(_)) | Poll::Pending => return, - } + } + if !self.wanted { + return; } if self.source.is_none() { self.source = source(); @@ -2430,35 +2437,35 @@ mod tests { first.set(Some(moq_net::bandwidth::Rate::from_bps(123))).unwrap(); let mut forward = RecvBandwidth::default(); forward.poll(&out, &waiter, || Some(first.consume())); - assert!(!first.is_used(), "the connection handle is not demand"); + assert!(!first.demand().wanted(), "the connection handle is not demand"); let reader = handle.consume(); forward.poll(&out, &waiter, || Some(first.consume())); - assert!(first.is_used()); + assert!(first.demand().wanted()); assert_eq!(reader.peek(), Some(moq_net::bandwidth::Rate::from_bps(123))); let clone = reader.clone(); drop(reader); forward.poll(&out, &waiter, || Some(first.consume())); - assert!(first.is_used(), "a remaining clone is demand"); + assert!(first.demand().wanted(), "a remaining clone is demand"); drop(clone); forward.poll(&out, &waiter, || Some(first.consume())); - assert!(!first.is_used(), "the last drop releases the live session's consumer"); + assert!(!first.demand().wanted(), "the last drop releases the live session's consumer"); let reader = handle.consume(); assert_eq!(reader.peek(), None, "an unused estimate is cleared"); forward.poll(&out, &waiter, || Some(first.consume())); - assert!(first.is_used(), "demand can resume on the same session"); + assert!(first.demand().wanted(), "demand can resume on the same session"); drop(forward); - assert!(!first.is_used()); + assert!(!first.demand().wanted()); out.set(None).unwrap(); assert_eq!(reader.peek(), None, "disconnected"); let second = BandwidthProducer::new(); second.set(Some(moq_net::bandwidth::Rate::from_bps(456))).unwrap(); let mut forward = RecvBandwidth::default(); forward.poll(&out, &waiter, || Some(second.consume())); - assert!(second.is_used(), "the same reader demands the replacement session"); + assert!(second.demand().wanted(), "the same reader demands the replacement session"); assert_eq!(reader.peek(), Some(moq_net::bandwidth::Rate::from_bps(456))); drop(reader); forward.poll(&out, &waiter, || Some(second.consume())); - assert!(!second.is_used()); + assert!(!second.demand().wanted()); } #[test] @@ -2497,7 +2504,7 @@ mod tests { forward.poll(&out, &kio::Waiter::noop(), || Some(source.consume())); assert!(demand.consumer.lock().unwrap().is_some()); assert!( - source.is_used(), + source.demand().wanted(), "new demand must be forwarded without unrelated activity" ); } diff --git a/rs/moq-transcode/src/lib.rs b/rs/moq-transcode/src/lib.rs index 34a8eb666ea..a4a1ab6ea4d 100644 --- a/rs/moq-transcode/src/lib.rs +++ b/rs/moq-transcode/src/lib.rs @@ -1472,7 +1472,7 @@ mod tests { rung.info().await.unwrap(); let fetching = rung.fetch_group(7, None); - let abandoned = tokio::time::timeout(std::time::Duration::from_secs(5), source_fetches.requested_fill()) + let mut abandoned = tokio::time::timeout(std::time::Duration::from_secs(5), source_fetches.requested_fill()) .await .expect("the fetch never reached the source") .expect("the source track closed"); @@ -1481,7 +1481,9 @@ mod tests { // Leave before a single source frame exists. Encoding can only happen // if the handler keeps this fetch. drop(fetching); - tokio::time::timeout(std::time::Duration::from_secs(5), abandoned.unused()) + tokio::time::timeout(std::time::Duration::from_secs(5), async { + while abandoned.wanted_changed().await.is_some() {} + }) .await .expect("the abandoned fetch kept its source fetch"); diff --git a/rs/moq-transcode/src/rung.rs b/rs/moq-transcode/src/rung.rs index 1cd4485ed3e..c9487780076 100644 --- a/rs/moq-transcode/src/rung.rs +++ b/rs/moq-transcode/src/rung.rs @@ -554,8 +554,7 @@ fn spawn_fill( tasks.spawn(async move { let _permit = permit; let mut last = None; - while fill.is_used() - && let Some(sequence) = fill.next() + while let Some(sequence) = fill.wanted() && last != Some(sequence) { last = Some(sequence); diff --git a/swift/Sources/Moq/Requests.swift b/swift/Sources/Moq/Requests.swift index 49ecc774fa0..108f8c084b7 100644 --- a/swift/Sources/Moq/Requests.swift +++ b/swift/Sources/Moq/Requests.swift @@ -39,10 +39,10 @@ public final class TrackRequest: Sendable { /// A run of groups readers miss, for a `TrackRequests` to serve. /// -/// Accept each group you have and drop the rest; `next()` names what is still owed, in the +/// Accept each group you have and drop the rest; `wanted` names what is still owed, in the /// fill's order. Releasing the fill names whatever is left as dropped while a reader still -/// wants it; once nobody does, it is withdrawn, so stop serving it when `unused()` returns. -/// Stop serving a single group when `unwanted(sequence:)` returns for it, and move on to `next()`. +/// wants it; once nobody does, it is withdrawn, so stop serving it once `wanted` is nil. +/// Stop serving a single group when `unwanted(sequence:)` returns for it, and move on to `wanted`. public final class Fill: Sendable { let ffi: MoqFill @@ -81,9 +81,15 @@ public final class Fill: Sendable { ffi.order() } - /// The next group still owed in the fill's order, or nil once every group is answered. - public func next() -> UInt64? { - ffi.next() + /// The next group still owed in the fill's order, or nil once every group is answered or + /// no reader waits on the fill. + public var wanted: UInt64? { + ffi.wanted() + } + + /// Suspend until `wanted` changes and return it. The first call returns at once. + public func wantedChanged() async throws -> UInt64? { + try await ffi.wantedChanged() } /// Insert group `sequence` and return a producer for it. The first group starts at @@ -97,16 +103,6 @@ public final class Fill: Sendable { ffi.dropGroups(start: start, end: end) } - /// Whether a reader still waits on this fill. - public var isUsed: Bool { - ffi.isUsed() - } - - /// Suspend until no reader waits on this fill. - public func unused() async throws { - try await ffi.unused() - } - /// Whether a reader still waits for group `sequence` from this fill: false once every /// reader took it or narrowed past it, or nobody waits on the fill. public func isWanted(sequence: UInt64) -> Bool { diff --git a/test/audio-quality/clients/js/src/probe.test.ts b/test/audio-quality/clients/js/src/probe.test.ts index 9df2a49b35f..97a75e2ed95 100644 --- a/test/audio-quality/clients/js/src/probe.test.ts +++ b/test/audio-quality/clients/js/src/probe.test.ts @@ -15,7 +15,7 @@ test("audio-only sampling holds probe demand until its final RTT sample", async const installed = clock.install(); const network = new Observed<{ rtt: Milli } | undefined>(undefined); const producer = new Effect((effect) => { - if (effect.get(network.used)) effect.set(network, { rtt: Milli(12) }, undefined); + if (effect.get(network.wanted)) effect.set(network, { rtt: Milli(12) }, undefined); }); const empty = new Signal(undefined); const watch = { @@ -36,7 +36,7 @@ test("audio-only sampling holds probe demand until its final RTT sample", async expect(measurement.drain().map((sample) => sample.rtt)).toEqual([12]); await measurement.finish(); expect(measurement.drain().at(-1)?.rtt).toBe(12); - expect(network.used.peek()).toBe(false); + expect(network.wanted.peek()).toBe(false); await clock.advance(Nano.fromMilli(Milli(500))); expect(measurement.drain()).toEqual([]); } finally { From 9cfe96876085a61bfd1b869b594131ccfc9b2718 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 28 Oct 2026 01:53:11 -0700 Subject: [PATCH 3/6] fix: wanted rename follow-ups Format, gate the async-only bandwidth poll, regenerate the Dart bindings, fix the remaining FFI test, and delete the finished quest. Co-Authored-By: Claude Opus 5.5 --- dart/moq_ffi/lib/src/moq.dart | 119 ++++++++----------- quest/m1/api-audit/README.md | 1 - quest/m1/api-audit/wanted.md | 39 ------ rs/kio/src/loom.rs | 18 ++- rs/kio/src/tests.rs | 5 +- rs/moq-c/src/bandwidth.rs | 5 +- rs/moq-ffi/src/test.rs | 2 +- rs/moq-net/src/ietf/publisher.rs | 4 +- rs/moq-net/src/model/bandwidth.rs | 5 +- rs/moq-net/src/model/origin.rs | 5 +- rs/moq-net/src/model/track.rs | 11 +- rs/moq-net/tests/integration/fetch_cancel.rs | 10 +- rs/moq-tokio/src/connection.rs | 10 +- rs/moq-transcode/src/lib.rs | 4 +- 14 files changed, 105 insertions(+), 133 deletions(-) delete mode 100644 quest/m1/api-audit/wanted.md diff --git a/dart/moq_ffi/lib/src/moq.dart b/dart/moq_ffi/lib/src/moq.dart index 1729c8a4564..40e3045d634 100644 --- a/dart/moq_ffi/lib/src/moq.dart +++ b/dart/moq_ffi/lib/src/moq.dart @@ -12744,16 +12744,15 @@ abstract class MoqFillInterface { MoqGroupProducer accept({required int sequence}); void dropGroups({required int start, required int end}); int endGroup(); - bool isUsed(); bool isWanted({required int sequence}); Duration? maxDelay(); - int? next(); MoqOrder order(); int priority(); int startFrame(); int startGroup(); - UniffiRustFuture unused(); UniffiRustFuture unwanted({required int sequence}); + int? wanted(); + UniffiRustFuture wantedChanged(); } final _MoqFillFinalizer = Finalizer>((ptr) { @@ -12819,17 +12818,6 @@ class MoqFill implements MoqFillInterface { ); } - bool isUsed() { - return rustCallWithLifter( - (status) => uniffi_moq_ffi_fn_method_moqfill_is_used( - uniffiClonePointer(), - status, - ), - FfiConverterBool.lift, - null, - ); - } - bool isWanted({required int sequence}) { return rustCallWithLifter( (status) => uniffi_moq_ffi_fn_method_moqfill_is_wanted( @@ -12853,15 +12841,6 @@ class MoqFill implements MoqFillInterface { ); } - int? next() { - return rustCallWithLifter( - (status) => - uniffi_moq_ffi_fn_method_moqfill_next(uniffiClonePointer(), status), - FfiConverterOptionalUInt64.lift, - null, - ); - } - MoqOrder order() { return rustCallWithLifter( (status) => @@ -12904,18 +12883,6 @@ class MoqFill implements MoqFillInterface { ); } - UniffiRustFuture unused() { - return uniffiRustCallAsync( - () => uniffi_moq_ffi_fn_method_moqfill_unused(uniffiClonePointer()), - ffi_moq_ffi_rust_future_poll_void, - ffi_moq_ffi_rust_future_complete_void, - ffi_moq_ffi_rust_future_cancel_void, - ffi_moq_ffi_rust_future_free_void, - (_) {}, - moqExceptionErrorHandler, - ); - } - UniffiRustFuture unwanted({required int sequence}) { return uniffiRustCallAsync( () => uniffi_moq_ffi_fn_method_moqfill_unwanted( @@ -12930,6 +12897,28 @@ class MoqFill implements MoqFillInterface { moqExceptionErrorHandler, ); } + + int? wanted() { + return rustCallWithLifter( + (status) => + uniffi_moq_ffi_fn_method_moqfill_wanted(uniffiClonePointer(), status), + FfiConverterOptionalUInt64.lift, + null, + ); + } + + UniffiRustFuture wantedChanged() { + return uniffiRustCallAsync( + () => + uniffi_moq_ffi_fn_method_moqfill_wanted_changed(uniffiClonePointer()), + ffi_moq_ffi_rust_future_poll_rust_buffer, + ffi_moq_ffi_rust_future_complete_rust_buffer, + ffi_moq_ffi_rust_future_cancel_rust_buffer, + ffi_moq_ffi_rust_future_free_rust_buffer, + FfiConverterOptionalUInt64.lift, + moqExceptionErrorHandler, + ); + } } class FfiConverterMoqFill { @@ -18989,14 +18978,6 @@ external int uniffi_moq_ffi_fn_method_moqfill_end_group( Pointer uniffiStatus, ); -@Native, Pointer)>( - assetId: _uniffiAssetId, -) -external int uniffi_moq_ffi_fn_method_moqfill_is_used( - Pointer ptr, - Pointer uniffiStatus, -); - @Native, Uint64, Pointer)>( assetId: _uniffiAssetId, ) @@ -19014,14 +18995,6 @@ external RustBuffer uniffi_moq_ffi_fn_method_moqfill_max_delay( Pointer uniffiStatus, ); -@Native, Pointer)>( - assetId: _uniffiAssetId, -) -external RustBuffer uniffi_moq_ffi_fn_method_moqfill_next( - Pointer ptr, - Pointer uniffiStatus, -); - @Native, Pointer)>( assetId: _uniffiAssetId, ) @@ -19054,17 +19027,25 @@ external int uniffi_moq_ffi_fn_method_moqfill_start_group( Pointer uniffiStatus, ); -@Native Function(Pointer)>(assetId: _uniffiAssetId) -external Pointer uniffi_moq_ffi_fn_method_moqfill_unused( - Pointer ptr, -); - @Native Function(Pointer, Uint64)>(assetId: _uniffiAssetId) external Pointer uniffi_moq_ffi_fn_method_moqfill_unwanted( Pointer ptr, int sequence, ); +@Native, Pointer)>( + assetId: _uniffiAssetId, +) +external RustBuffer uniffi_moq_ffi_fn_method_moqfill_wanted( + Pointer ptr, + Pointer uniffiStatus, +); + +@Native Function(Pointer)>(assetId: _uniffiAssetId) +external Pointer uniffi_moq_ffi_fn_method_moqfill_wanted_changed( + Pointer ptr, +); + @Native Function(Pointer, Pointer)>( assetId: _uniffiAssetId, ) @@ -20584,18 +20565,12 @@ external int uniffi_moq_ffi_checksum_method_moqfill_drop_groups(); @Native(assetId: _uniffiAssetId) external int uniffi_moq_ffi_checksum_method_moqfill_end_group(); -@Native(assetId: _uniffiAssetId) -external int uniffi_moq_ffi_checksum_method_moqfill_is_used(); - @Native(assetId: _uniffiAssetId) external int uniffi_moq_ffi_checksum_method_moqfill_is_wanted(); @Native(assetId: _uniffiAssetId) external int uniffi_moq_ffi_checksum_method_moqfill_max_delay(); -@Native(assetId: _uniffiAssetId) -external int uniffi_moq_ffi_checksum_method_moqfill_next(); - @Native(assetId: _uniffiAssetId) external int uniffi_moq_ffi_checksum_method_moqfill_order(); @@ -20609,10 +20584,13 @@ external int uniffi_moq_ffi_checksum_method_moqfill_start_frame(); external int uniffi_moq_ffi_checksum_method_moqfill_start_group(); @Native(assetId: _uniffiAssetId) -external int uniffi_moq_ffi_checksum_method_moqfill_unused(); +external int uniffi_moq_ffi_checksum_method_moqfill_unwanted(); @Native(assetId: _uniffiAssetId) -external int uniffi_moq_ffi_checksum_method_moqfill_unwanted(); +external int uniffi_moq_ffi_checksum_method_moqfill_wanted(); + +@Native(assetId: _uniffiAssetId) +external int uniffi_moq_ffi_checksum_method_moqfill_wanted_changed(); @Native(assetId: _uniffiAssetId) external int uniffi_moq_ffi_checksum_method_moqgroupproducer_abort(); @@ -21313,18 +21291,12 @@ void _checkApiChecksums() { if (uniffi_moq_ffi_checksum_method_moqfill_end_group() != 42504) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } - if (uniffi_moq_ffi_checksum_method_moqfill_is_used() != 6429) { - throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); - } if (uniffi_moq_ffi_checksum_method_moqfill_is_wanted() != 47156) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } if (uniffi_moq_ffi_checksum_method_moqfill_max_delay() != 1258) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } - if (uniffi_moq_ffi_checksum_method_moqfill_next() != 7725) { - throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); - } if (uniffi_moq_ffi_checksum_method_moqfill_order() != 32232) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } @@ -21337,10 +21309,13 @@ void _checkApiChecksums() { if (uniffi_moq_ffi_checksum_method_moqfill_start_group() != 8622) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } - if (uniffi_moq_ffi_checksum_method_moqfill_unused() != 3173) { + if (uniffi_moq_ffi_checksum_method_moqfill_unwanted() != 13637) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } - if (uniffi_moq_ffi_checksum_method_moqfill_unwanted() != 13637) { + if (uniffi_moq_ffi_checksum_method_moqfill_wanted() != 65141) { + throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); + } + if (uniffi_moq_ffi_checksum_method_moqfill_wanted_changed() != 42560) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } if (uniffi_moq_ffi_checksum_method_moqgroupproducer_abort() != 59787) { diff --git a/quest/m1/api-audit/README.md b/quest/m1/api-audit/README.md index e75b46483f7..dbafdd15054 100644 --- a/quest/m1/api-audit/README.md +++ b/quest/m1/api-audit/README.md @@ -75,7 +75,6 @@ Rejected in planning: - [Two broadcast resolutions](/quest/m1/api-audit/resolve.md) - Rust, JS, and the bindings resolve a broadcast with `request(path, epoch)` or `follow(path, epoch)`, and every racy two-step caller is gone - [Binding publications](/quest/m1/api-audit/binding-publication.md) - moq-ffi, the C ABI, and every wrapper hand out a publication handle that announces, replacing the private adapters -- [Demand is wanted, everywhere](/quest/m1/api-audit/wanted.md) - every `used`/`unused` pair becomes `wanted()` and `wanted_changed()`, and unused ones go - [One strict path rule](/quest/m1/api-audit/strict-paths.md) - `Relative` refuses empty segments, `Path` refuses non-canonical escapes, the error is `path::Error`, and auth mints only strict paths ## Related diff --git a/quest/m1/api-audit/wanted.md b/quest/m1/api-audit/wanted.md deleted file mode 100644 index 728092c4d1a..00000000000 --- a/quest/m1/api-audit/wanted.md +++ /dev/null @@ -1,39 +0,0 @@ -# [M] Demand is wanted, everywhere - -## Goal - -Every demand signal reads as `wanted`: a snapshot `wanted()` plus an async -`wanted_changed()` (and its poll), where "not wanted" is `false` or `None`. -No `used`/`unused`/`is_used` methods remain in kio, moq-net, `@moq/net`, -moq-ffi, moq-c, or the wrappers, and methods without a consumer are deleted -instead of renamed. - -## Plan - -Decided 2026-10-25 (maintainer), extending the `wanted` shape that -[#6751](https://github.com/moq-dev/moq/pull/6751) gave `group::Demand`: - -- Yes/no demand returns `bool`; `track::Demand::wanted() -> Option` - keeps its meaning. -- Broadcast demand is deleted: Rust's `broadcast::Demand` went in - [#6744](https://github.com/moq-dev/moq/pull/6744), and the `@internal` JS - `Broadcast.Demand` (read only by tests and a bench) goes too. The origin - closes a superseded producer explicitly instead of a publisher watching - demand. -- `group::Demand` already says `wanted`. The JS group demand stays internal. -- `bandwidth::Producer` gets a `demand()` handle with `wanted() -> bool` and - `wanted_changed()`, like track. -- `track::Fill` merges `is_used()` and `next()` into - `wanted() -> Option` (the next group owed), with `wanted_changed()` - replacing `unused()`. Check that it returns `None` once no reader is left. -- kio `Producer`/`ProducerWeak` rename too: `wanted() -> bool` and - `wanted_changed()`; delete the test-only async variants. -- The Announce consumer count is a `wanted`-shaped signal, not a new `unused()`. -- moq-c's `MOQ_DEMAND_USED/UNUSED` become `WANTED/UNWANTED`; moq-c takes the - break with every binding. - -Fix the stale `Used`/`Unused` advice in the Go wrapper README -(`go/wrapper/README.md`, about line 143). The Impact note -corrects the `rs/moq-json` and `rs/moq-flate` Unreleased advice for the cut. - -Public API: breaking renames and deletions in every language. Wire: none. diff --git a/rs/kio/src/loom.rs b/rs/kio/src/loom.rs index ea2e8791b20..764f6786d57 100644 --- a/rs/kio/src/loom.rs +++ b/rs/kio/src/loom.rs @@ -142,7 +142,11 @@ fn first_consumer_wakes_wanted_changed() { // The consumer rides back on the join handle so it outlives the `wanted_changed()` poll. let maker = thread::spawn(move || second.consume()); - assert_eq!(block_on(producer.wanted_changed(false)), Ok(true), "the new consumer was missed"); + assert_eq!( + block_on(producer.wanted_changed(false)), + Ok(true), + "the new consumer was missed" + ); drop(maker.join().unwrap()); }); } @@ -157,7 +161,11 @@ fn last_consumer_wakes_wanted_changed() { let dropper = thread::spawn(move || drop(consumer)); - assert_eq!(block_on(producer.wanted_changed(true)), Ok(false), "the last drop was missed"); + assert_eq!( + block_on(producer.wanted_changed(true)), + Ok(false), + "the last drop was missed" + ); dropper.join().unwrap(); }); } @@ -172,7 +180,11 @@ fn consumer_churn_resolves_unwanted() { let churn = thread::spawn(move || drop(second.consume())); - assert_eq!(block_on(producer.wanted_changed(true)), Ok(false), "wanted_changed() stalled on churn"); + assert_eq!( + block_on(producer.wanted_changed(true)), + Ok(false), + "wanted_changed() stalled on churn" + ); churn.join().unwrap(); }); } diff --git a/rs/kio/src/tests.rs b/rs/kio/src/tests.rs index abaf1212620..a41d78c8750 100644 --- a/rs/kio/src/tests.rs +++ b/rs/kio/src/tests.rs @@ -67,7 +67,10 @@ fn value_modification_does_not_wake_consumer() { // Dropping the last consumer is what should wake it. drop(consumer); - assert!(waker.count() >= 1, "dropping the last consumer did not wake wanted_changed()"); + assert!( + waker.count() >= 1, + "dropping the last consumer did not wake wanted_changed()" + ); assert!(matches!(fut.as_mut().poll(&mut cx), Poll::Ready(Ok(false)))); } diff --git a/rs/moq-c/src/bandwidth.rs b/rs/moq-c/src/bandwidth.rs index 96e0695c1c2..dabdf958bb3 100644 --- a/rs/moq-c/src/bandwidth.rs +++ b/rs/moq-c/src/bandwidth.rs @@ -299,7 +299,10 @@ mod consumer_tests { 0 ); assert_eq!(moq_bandwidth_consumer_close(id.into()), 0); - assert!(!producer.demand().wanted(), "closing releases demand before the callback runs"); + assert!( + !producer.demand().wanted(), + "closing releases demand before the callback runs" + ); assert!(receiver.recv().await.unwrap() < 0); } } diff --git a/rs/moq-ffi/src/test.rs b/rs/moq-ffi/src/test.rs index 8b9dd8a9c16..656923b9117 100644 --- a/rs/moq-ffi/src/test.rs +++ b/rs/moq-ffi/src/test.rs @@ -2576,7 +2576,7 @@ async fn dynamic_track_serves_fetch_miss_and_priority() { (5, 0, 6) ); assert_eq!(request.priority(), 11); - assert_eq!(request.next(), Some(5)); + assert_eq!(request.wanted(), Some(5)); let group = request.accept(5).unwrap(); group diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index ae066a58fde..ebd1ff9e652 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -1678,7 +1678,9 @@ impl Publisher { async fn released(&self, credit: &kio::Producer<()>) { let mut session = self.session.clone(); kio::wait(|waiter| { - if session.poll_closed(&mut waiter.context()).is_ready() || credit.poll_wanted_changed(waiter, true).is_ready() { + if session.poll_closed(&mut waiter.context()).is_ready() + || credit.poll_wanted_changed(waiter, true).is_ready() + { return Poll::Ready(()); } Poll::Pending diff --git a/rs/moq-net/src/model/bandwidth.rs b/rs/moq-net/src/model/bandwidth.rs index b9098ffcc7b..1609feebe33 100644 --- a/rs/moq-net/src/model/bandwidth.rs +++ b/rs/moq-net/src/model/bandwidth.rs @@ -125,10 +125,9 @@ impl Producer { /// Poll for consumer presence to differ from `last`, with the new value once it does. /// Errors once the channel closes. + #[cfg_attr(not(feature = "async"), allow(dead_code, reason = "async-only"))] pub(crate) fn poll_wanted_changed(&self, waiter: &kio::Waiter, last: bool) -> Poll> { - self.state - .poll_wanted_changed(waiter, last) - .map_err(|_| Error::Dropped) + self.state.poll_wanted_changed(waiter, last).map_err(|_| Error::Dropped) } } diff --git a/rs/moq-net/src/model/origin.rs b/rs/moq-net/src/model/origin.rs index 000ea5b280a..2c2c6660a46 100644 --- a/rs/moq-net/src/model/origin.rs +++ b/rs/moq-net/src/model/origin.rs @@ -12991,7 +12991,10 @@ mod tests { for _ in 0..10 { moq_clock::sim::yield_now().await; } - assert!(!incompatible.demand().is_wanted(), "the refused copy is still subscribed"); + assert!( + !incompatible.demand().is_wanted(), + "the refused copy is still subscribed" + ); // The refusal happened: the incumbent leaving surfaces it. drop(track); diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 0057da0c4e1..ef60f5e8fff 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -18040,7 +18040,11 @@ mod test { panic!("below the start is a miss"); }; let (flag, waiter) = Flag::waiter(); - assert_eq!(fill.poll_wanted_changed(&waiter), Poll::Ready(Some(0)), "the first poll reports"); + assert_eq!( + fill.poll_wanted_changed(&waiter), + Poll::Ready(Some(0)), + "the first poll reports" + ); assert!(fill.poll_wanted_changed(&waiter).is_pending()); subscriber.update(ascending(8..)).unwrap(); @@ -18161,7 +18165,10 @@ mod test { subscriber.update(ascending(1..)).unwrap(); assert!(subscriber.poll_recv_group(&waiter).is_pending()); assert!(readers.poll_unwanted(&waiter).is_ready(), "nobody reads group 0 now"); - assert!(fill.poll_withdrawn(&waiter).is_pending(), "the fill still owes 1 and up"); + assert!( + fill.poll_withdrawn(&waiter).is_pending(), + "the fill still owes 1 and up" + ); } /// A time start before every group held asks no fill below the oldest, since which of diff --git a/rs/moq-net/tests/integration/fetch_cancel.rs b/rs/moq-net/tests/integration/fetch_cancel.rs index 68dfbc85ca6..17d47f458ad 100644 --- a/rs/moq-net/tests/integration/fetch_cancel.rs +++ b/rs/moq-net/tests/integration/fetch_cancel.rs @@ -75,9 +75,11 @@ async fn abandoned_fetch_reaches_the_publisher(version: &str, relays: u64, stage match stage { Stage::Unanswered => { drop(waiting); - moq_clock::timeout(Duration::from_secs(5), async { while request.wanted_changed().await.is_some() {} }) - .await - .unwrap_or_else(|_| panic!("{ctx}: the publisher's fetch is still wanted")); + moq_clock::timeout(Duration::from_secs(5), async { + while request.wanted_changed().await.is_some() {} + }) + .await + .unwrap_or_else(|_| panic!("{ctx}: the publisher's fetch is still wanted")); } Stage::MidResponse => { let mut group = request.accept(0, timed()).unwrap(); @@ -187,7 +189,7 @@ async fn a_refetch_within_the_linger_stays_on_the_relay(version: &str) { }; let groups = request.requests(); let _track = request.accept(timed()); - let mut request = match futures::future::select(std::pin::pin!(groups.requested_fill()), &mut waiting).await { + let request = match futures::future::select(std::pin::pin!(groups.requested_fill()), &mut waiting).await { futures::future::Either::Left((request, _)) => request.expect("the fetch reaches the publisher"), futures::future::Either::Right(_) => panic!("nothing answered the fetch"), }; diff --git a/rs/moq-tokio/src/connection.rs b/rs/moq-tokio/src/connection.rs index ef86edc8bde..e71b03eb9d3 100644 --- a/rs/moq-tokio/src/connection.rs +++ b/rs/moq-tokio/src/connection.rs @@ -2448,7 +2448,10 @@ mod tests { assert!(first.demand().wanted(), "a remaining clone is demand"); drop(clone); forward.poll(&out, &waiter, || Some(first.consume())); - assert!(!first.demand().wanted(), "the last drop releases the live session's consumer"); + assert!( + !first.demand().wanted(), + "the last drop releases the live session's consumer" + ); let reader = handle.consume(); assert_eq!(reader.peek(), None, "an unused estimate is cleared"); forward.poll(&out, &waiter, || Some(first.consume())); @@ -2461,7 +2464,10 @@ mod tests { second.set(Some(moq_net::bandwidth::Rate::from_bps(456))).unwrap(); let mut forward = RecvBandwidth::default(); forward.poll(&out, &waiter, || Some(second.consume())); - assert!(second.demand().wanted(), "the same reader demands the replacement session"); + assert!( + second.demand().wanted(), + "the same reader demands the replacement session" + ); assert_eq!(reader.peek(), Some(moq_net::bandwidth::Rate::from_bps(456))); drop(reader); forward.poll(&out, &waiter, || Some(second.consume())); diff --git a/rs/moq-transcode/src/lib.rs b/rs/moq-transcode/src/lib.rs index a4a1ab6ea4d..7473fc1664c 100644 --- a/rs/moq-transcode/src/lib.rs +++ b/rs/moq-transcode/src/lib.rs @@ -1484,8 +1484,8 @@ mod tests { tokio::time::timeout(std::time::Duration::from_secs(5), async { while abandoned.wanted_changed().await.is_some() {} }) - .await - .expect("the abandoned fetch kept its source fetch"); + .await + .expect("the abandoned fetch kept its source fetch"); // The handler dropped the output request with the caller, so this is a // fresh request rather than a join of the one still encoding. From 5fe5d63c4924644252403dd8b11a036170fb9ea5 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 29 Oct 2026 18:09:55 -0700 Subject: [PATCH 4/6] fix: wake fill watchers on live arrivals and keep FFI waits cancel-safe Fill::poll_wanted_changed now registers on the track state, so a parked wait wakes when the owed group arrives through the live feed and the cache answers it. MoqFill::wanted_changed drives its watcher in place through ffi::Task instead of a detached task, so a call the foreign side cancelled but has not freed cannot consume the change the next call waits for. Co-Authored-By: Claude Opus 5.5 --- rs/moq-ffi/src/producer.rs | 9 ++++---- rs/moq-ffi/src/test.rs | 35 ++++++++++++++++++++++++++++++ rs/moq-net/src/model/track.rs | 27 +++++++++++++++++++++++ rs/moq-net/src/model/track/fill.rs | 4 +++- 4 files changed, 70 insertions(+), 5 deletions(-) diff --git a/rs/moq-ffi/src/producer.rs b/rs/moq-ffi/src/producer.rs index 481eb205bf3..644504b6f86 100644 --- a/rs/moq-ffi/src/producer.rs +++ b/rs/moq-ffi/src/producer.rs @@ -448,12 +448,12 @@ impl MoqTrackRequests { pub struct MoqFill { inner: moq_net::track::Fill, // The watcher `wanted_changed` advances, kept apart so concurrent waits queue in turn. - changes: Arc>, + changes: crate::ffi::Task, } impl MoqFill { fn new(inner: moq_net::track::Fill) -> Self { - let changes = Arc::new(tokio::sync::Mutex::new(inner.clone())); + let changes = crate::ffi::Task::new(inner.clone()); Self { inner, changes } } } @@ -499,8 +499,9 @@ impl MoqFill { /// Wait for `wanted` to change and return the new value. The first call returns at once. pub async fn wanted_changed(&self) -> Result, MoqError> { - let changes = self.changes.clone(); - crate::ffi::detached(async move { Ok::<_, MoqError>(changes.lock().await.wanted_changed().await) }).await + // Driven in place, not detached: a cancelled call left unfreed must not advance the + // watcher and swallow the change the next call waits for. + self.changes.run(|mut fill| async move { Ok(fill.wanted_changed().await) }).await } /// Insert group `sequence` and return a producer for filling it. The first group starts diff --git a/rs/moq-ffi/src/test.rs b/rs/moq-ffi/src/test.rs index d0fd3152341..3626d4aba09 100644 --- a/rs/moq-ffi/src/test.rs +++ b/rs/moq-ffi/src/test.rs @@ -2624,6 +2624,41 @@ async fn fill_goes_unwanted_when_the_fetcher_leaves() { assert!(!fill.is_wanted(5)); } +/// A foreign cancel only stops polling `wanted_changed`; uniffi drops it later, at +/// `rust_future_free`. Until then the cancelled call must not consume the change the next +/// call is waiting for. +#[tokio::test] +async fn fill_wanted_changed_cancelled_before_free_leaves_the_change() { + let broadcast = MoqBroadcastProducer::new().unwrap(); + let track = broadcast.publish_track("events".into(), None).unwrap(); + let dynamic = track.requests().unwrap(); + let consumer = broadcast.consume().unwrap(); + + let fetch = tokio::spawn(async move { consumer.fetch_group("events".into(), 5, None).await }); + let fill = tokio::time::timeout(TIMEOUT, dynamic.requested_fill()) + .await + .expect("timed out waiting for fill") + .unwrap(); + assert_eq!(fill.wanted_changed().await.unwrap(), Some(5)); + + // Poll once, then stop without dropping, which is all `rust_future_cancel` does. + let mut cancelled = Box::pin(fill.wanted_changed()); + let first = std::future::poll_fn(|cx| std::task::Poll::Ready(cancelled.as_mut().poll(cx))).await; + assert!(first.is_pending()); + + fetch.abort(); + // Give anything still driving the cancelled call the chance to take the change. + ffi_caught_up().await; + + let next = fill.wanted_changed(); + drop(cancelled); + let wanted = tokio::time::timeout(TIMEOUT, next) + .await + .expect("the cancelled call took the change") + .unwrap(); + assert_eq!(wanted, None); +} + #[tokio::test] async fn dynamic_track_rejects_fetch_miss() { let broadcast = MoqBroadcastProducer::new().unwrap(); diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 2167ad5b65f..41d0f68e877 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -18567,6 +18567,33 @@ mod test { assert_eq!(fill.wanted(), None); } + /// The owed group arriving through the live feed wakes a parked + /// [`Fill::poll_wanted_changed`]: the cache answers it, so the fill no longer owes it. + #[test] + #[cfg(feature = "async")] + fn fill_wanted_changed_follows_a_live_arrival() { + let mut producer = track_producer("test", Info::timed()); + let dynamic = producer.requests(); + let ascending = |groups| replay().with_range(groups).with_order(Order::Ascending); + let mut subscriber = producer.subscribe(ascending(9..)); + producer.start_at(10).unwrap(); + write_group(&producer, 10); + let noop = kio::Waiter::noop(); + while let Poll::Ready(Ok(Some(_))) = subscriber.poll_recv_group(&noop) {} + let Poll::Ready(Ok(mut fill)) = dynamic.poll_requested_fill(&noop) else { + panic!("below the start is a miss"); + }; + let (flag, waiter) = Flag::waiter(); + assert_eq!(fill.poll_wanted_changed(&waiter), Poll::Ready(Some(9))); + assert!(fill.poll_wanted_changed(&waiter).is_pending()); + + write_group(&producer, 9); + assert!(flag.take(), "the owed group arriving live woke the parked poll"); + assert_eq!(fill.poll_wanted_changed(&waiter), Poll::Ready(None)); + assert_eq!(fill.wanted(), None); + drop(subscriber); + } + /// A reader narrowing its ranges leaves the part of its fill it no longer reads: the /// handler is not asked for it, a fetch of it in flight is cancelled, and neither a bulk /// refusal nor dropping the fill names it dropped. diff --git a/rs/moq-net/src/model/track/fill.rs b/rs/moq-net/src/model/track/fill.rs index f3ac8b3a2d8..d73e12de47f 100644 --- a/rs/moq-net/src/model/track/fill.rs +++ b/rs/moq-net/src/model/track/fill.rs @@ -870,9 +870,11 @@ impl Fill { /// Poll for [`wanted`](Self::wanted) to differ from what this handle last returned. pub fn poll_wanted_changed(&mut self, waiter: &kio::Waiter) -> Poll> { // Arm every input of `wanted` before reading it, so a change in between still wakes: - // the readers leaving, narrowing, or taking a group, the answers, and the order. + // the readers leaving, narrowing, or taking a group, the answers, the order, and a + // group arriving through the live feed, which the cache then answers. let _ = self.inner.result.poll_wanted_changed(waiter, true); let _ = self.inner.result.poll_ref(waiter, |_| Poll::<()>::Pending); + let _ = self.inner.state.poll_ref(waiter, |_| Poll::<()>::Pending); let _ = self.inner.fills.poll(waiter, |_| Poll::Pending); let _ = self.inner.asks.poll(waiter, |_| Poll::Pending); let wanted = self.wanted(); From f44f94edbf3d47001ce9d1c3ceb346bb187a5ea0 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 29 Oct 2026 18:21:01 -0700 Subject: [PATCH 5/6] style: cargo fmt Co-Authored-By: Claude Opus 5.5 --- rs/moq-ffi/src/producer.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rs/moq-ffi/src/producer.rs b/rs/moq-ffi/src/producer.rs index 644504b6f86..77b16b46ec3 100644 --- a/rs/moq-ffi/src/producer.rs +++ b/rs/moq-ffi/src/producer.rs @@ -501,7 +501,9 @@ impl MoqFill { pub async fn wanted_changed(&self) -> Result, MoqError> { // Driven in place, not detached: a cancelled call left unfreed must not advance the // watcher and swallow the change the next call waits for. - self.changes.run(|mut fill| async move { Ok(fill.wanted_changed().await) }).await + self.changes + .run(|mut fill| async move { Ok(fill.wanted_changed().await) }) + .await } /// Insert group `sequence` and return a producer for filling it. The first group starts From 7c378e11ae79f4262d98cc908af9e8e7619fb664 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Fri, 30 Oct 2026 20:58:19 -0700 Subject: [PATCH 6/6] fix(test): run the fill wanted_changed test without async Main gated write_filled behind the async feature, but this PR's fill_wanted_changed_follows_the_group_owed test is sync and runs in the explicit-time shape too. Ungate the helper so both shapes compile. Co-Authored-By: Claude Opus 5.5 --- rs/moq-net/src/model/track.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 93a190b37de..f9abbcbe599 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -18013,7 +18013,6 @@ mod test { (producer, dynamic, subscriber) } - #[cfg(feature = "async")] fn write_filled(group: Result, millis: u64) { let mut group = group.unwrap(); group