moq-net: RAII broadcast announcements via OriginPublish; remove broadcast abort/close#1640
Conversation
…st close Make the origin announcement lifecycle sans-io. Previously `publish_broadcast` spawned a task per broadcast that awaited `broadcast.closed()` and then removed it from the tree, which required a runtime. Now publishing returns a guard whose `Drop` does the removal synchronously, so the origin no longer watches closure and no longer spawns. This is paired with a conceptual change: a broadcast is just a collection of tracks, so it can no longer be "closed" with a code. Removed `BroadcastProducer::abort` / `BroadcastDynamic::abort`; a broadcast ends only when every producer is dropped, and pending dynamic track requests are rejected when the last dynamic handler goes away. There is no wire code for broadcast closure, so nothing is lost. API changes (moq-net, breaking): - `publish_broadcast` returns `Result<OriginPublish, Error>` (was `bool`). The guard keeps the broadcast announced; drop it (or call `OriginPublish::unannounce`) to remove it. The announcement is independent of the broadcast's own lifetime, so you can now unannounce without closing the broadcast. - `create_broadcast` returns `Result<BroadcastPublish, Error>`, which derefs to `BroadcastProducer` and unannounces on drop. - New `Error::Loop` (hop chain already contains this origin) and reused `Error::Unauthorized` (path out of scope) distinguish the two reject reasons, instead of a bare `false`. - Removed `BroadcastProducer::abort` and `BroadcastDynamic::abort`. The Lock-based origin tree is unchanged: connections stay scoped to their place in the tree so they don't contend on a global lock. Callers updated to hold the guard for the broadcast's lifetime. The lite/ietf subscribers store it in their per-broadcast maps so Ended/restart/reflected drop it (which also makes unannounce-on-Ended prompt instead of waiting for the broadcast to fully close). The runtime-bound FFI edges (libmoq, moq-ffi) keep fire-and-forget by spawning a small boundary watcher that drops the guard on close, so their public signatures and the language wrappers are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| // Errors only if the origin rejects the publish (loop / out of scope). libmoq origins | ||
| // are full-scope and broadcasts carry empty hop chains, so this is unreachable; ignore | ||
| // it to match the prior fire-and-forget contract. | ||
| if let Ok(publish) = origin.publish_broadcast(path, broadcast.clone()) { |
There was a problem hiding this comment.
I think we need to store it in the slab? And add an unpublish method.
Also please return an error instead of assuming it will always be impossible.
There was a problem hiding this comment.
Done. publish now stores the OriginPublish guard in a new published slab and returns a publish handle; added moq_origin_unpublish (C: moq_origin_unpublish(handle)) which removes the entry and drops the guard to unannounce. It now propagates the origin error via ? instead of swallowing it. Note this drops the old auto-unannounce-on-close behavior: the announcement lives until unpublish, independent of the broadcast closing. (Written by Claude)
| origin.publish_broadcast(&name, publish.consume()); | ||
| let _publish = origin | ||
| .publish_broadcast(&name, publish.consume()) | ||
| .expect("origin should allow publishing"); |
There was a problem hiding this comment.
.context instead of .expect for all of these that use anyhow.
There was a problem hiding this comment.
Switched all the anyhow call sites (moq-cli client/server, clock/chat/video examples, moq-boy, moq-rtc) to .context("failed to publish broadcast")?. (Written by Claude)
| // Held for the lifetime of this task; dropping it (on return) unannounces the broadcast. | ||
| let _publish = origin | ||
| .publish_broadcast(&settings.broadcast, broadcast_consumer) | ||
| .map_err(|err| anyhow::anyhow!("failed to publish broadcast {}: {}", settings.broadcast, err))?; |
There was a problem hiding this comment.
Please just use .context, I don't need to know the name.
There was a problem hiding this comment.
Done, now .context("failed to publish broadcast")? with no name. (Written by Claude)
…publish - Remove the loop check (and `Error::Loop`) from `publish_broadcast`. Relays already filter reflections on the announce path, so the origin check was pure redundancy; keep a `debug_assert` as a dev tripwire. `publish_broadcast` now fails only with `Error::Unauthorized` (path out of scope). - libmoq: store the `OriginPublish` guard in a slab and return a publish handle; add `moq_origin_unpublish` to drop it. Propagate the origin error instead of assuming the publish always succeeds. - Use `anyhow::Context` instead of `.expect` / `anyhow!` for the anyhow callers (moq-cli, examples, moq-boy, moq-gst, moq-rtc), dropping the broadcast name from the message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Summary
Makes the origin announcement lifecycle sans-io and reframes what a broadcast is.
Previously
publish_broadcastspawned a task per broadcast (web_async::spawn) that awaitedbroadcast.closed()and then removed it from the origin tree, which required a runtime. Now publishing returns anOriginPublishguard whoseDropdoes the removal synchronously, so the origin no longer watches closure and no longer spawns. This is the first concrete step toward drivingmoq-netfrompoll_xxx/wakers instead oftokio.Paired conceptual change: a broadcast is just a collection of tracks, so it can no longer be "closed" with a code.
BroadcastProducer::abort/BroadcastDynamic::abortare removed. A broadcast ends only when every producer is dropped; pending dynamic track requests are rejected when the last dynamic handler goes away. There is no wire code for broadcast closure, so nothing is lost on the wire. You can now unannounce without closing (drop the guard while the producer keeps serving tracks).API changes (moq-net, breaking → targeting
dev)publish_broadcastreturnsResult<OriginPublish, Error>(wasbool). Hold the guard to stay announced; drop it (or callOriginPublish::unannounce) to remove it.create_broadcastreturnsResult<BroadcastPublish, Error>;BroadcastPublishderefs toBroadcastProducerand unannounces on drop.Error::Loop(broadcast's hop chain already contains this origin) and reusedError::Unauthorized(path outside allowed prefixes) distinguish the two reject reasons that were previously a barefalse/None.BroadcastProducer::abortandBroadcastDynamic::abort.The
Lock-based origin tree is unchanged — connections stay scoped to their place in the tree so they don't contend on a global lock.Callers
doc/libneed no changes.Cross-package sync
Skipped
js/netanddoc/concept: this is a Rust-internal lifecycle refactor (RAII guard + removingabort); the wire protocol is unchanged, and the JS@moq/netAPI has its own idioms. Worth a follow-up if we want JS to mirror the guard/unannounce model.Test plan
cargo test -p moq-net— 366 pass (origin tests now exerciseOriginPublish::dropvia a#[cfg(test)]helper that reproduces the old fire-and-forget behavior)cargo test -p moq-ffi— 18 pass (exercises the relocatedannouncewatcher)cargo check --all-targetsacross every changed crate — cleancargo clippy -p moq-net --all-targets(via nix) — cleancargo fmt(via nix) appliedjust check— run once any concurrent localmoq-clidemo (shared target dir) has exited🤖 Generated with Claude Code