diff --git a/README.md b/README.md index d0bd9af..0ee04ee 100644 --- a/README.md +++ b/README.md @@ -358,10 +358,11 @@ independent of HTTP (it imports only smarm) and built for the WebSocket relay pattern: ```rust -let bus: PubSub = PubSub::new(); // in-runtime only! -let rx = bus.subscribe("room:lobby")?; // Receiver> -bus.broadcast("room:lobby", "hi".to_string())?; -bus.broadcast_from(smarm::self_pid(), "room:lobby", "no echo".into())?; +const BUS: PubSub = PubSub::new("chat"); // a name; spawns nothing +// BUS.child() goes in your supervision tree (or serve_with*'s child vec) +let rx = BUS.subscribe("room:lobby")?; // Receiver> +BUS.broadcast("room:lobby", "hi".to_string())?; +BUS.broadcast_from(smarm::self_pid(), "room:lobby", "no echo".into())?; ``` One generic instance per message domain; payloads broadcast as `Arc` @@ -373,19 +374,25 @@ explicit pid for relay patterns. Mailboxes are unbounded: `broadcast` never blocks the table, and a slow subscriber's memory bill is bounded by the two cleanup paths above. -Two composition rules that matter (both enforced by -`shutdown_with_open_chat_terminates` in the integration suite): +Composition (enforced by `shutdown_with_open_chat_terminates` in the +integration suite): -1. `PubSub::new()` spawns an actor, so it must run in-runtime. If your - app owns its tree, start the table as a supervised sibling of the - endpoint and address it by name — the clean shape. Under `serve*` - there is no in-runtime moment before the first request, so build it - lazily from a handler via a **non-static** `Arc>>` - captured by the route closure; a `static` cell pins the table actor - forever and graceful shutdown never returns. +1. **The handle is an address, not the table.** `PubSub` is a name: + `const`, `Copy`, spawns nothing, fine in a `static` or outside the + runtime. The actor is `PubSub::child()`, a `ChildSpec` for your + supervision tree — or for `serve_with*`'s app-children vec, which puts + it ahead of the endpoint so it stops only after the endpoint has + drained. Operations resolve the name per call, so a restarted table is + reached transparently. 2. Relay/producer actors hold the `Receiver` (plus e.g. a `WsSender` - clone) — **never a `PubSub` clone**, or relay and table keep each - other alive past shutdown. + clone). They may hold the handle too — it pins nothing — but usually + have no use for one. + +Prior to v0.8 both of these read the other way round: `PubSub::new()` +spawned the table, the handle owned its life, and a non-static +`Arc>>` lazily built from the first handler was the +required idiom. smarm 0.7 made a server's lifetime its own, and that +whole apparatus went away with it. See [`examples/ws_chat.rs`](examples/ws_chat.rs): rooms as topics, `on_open` subscribes + spawns the relay, `on_message` uses @@ -425,8 +432,10 @@ impl Channel

for Room { } } -// in-runtime, non-static — the ws_chat OnceLock pattern applies -let hub = ChannelHub::new(PrefixRouter::new().channel_default::("room:*")); +// A description: spawns nothing. `children` are the ChildSpecs it needs +// (bus table + one registry per session route) — hand them to serve_with*. +let (hub, children) = + ChannelHub::new("chat-bus", PrefixRouter::new().channel_default::("room:*")); // route handler: hub.upgrade(conn) // from anywhere with a hub handle: hub.broadcast("room:lobby", "news", payload) ``` diff --git a/ROADMAP.md b/ROADMAP.md index a10c82b..8a5d6ae 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -413,15 +413,92 @@ your root sup on `serve*`. **Open after this cycle:** -- One unreproduced test failure seen once in ~120 full-suite runs (output - discarded by the loop that caught it; not reproduced in 60x lib + 20x - integration + 10x concurrent + 15x full since). Candidates: the - `free_port()` bind race the harness already documents, or a timing margin - in the new endpoint tests. Grab the test name next time it fires. +- ~~One unreproduced test failure seen once in ~120 full-suite runs~~ + **DIAGNOSED AND FIXED** in v0.8 below — it was the pubsub/channels drain + hang, not the `free_port()` race. It hid because `hammer.sh` builds with + default features while the failure needs `--all-features` load. - `endpoint()` returns `impl Fn()`, so calling it twice = two endpoints contending for one `Config.name` (second panics on the clash). Honest failure, but the type doesn't prevent the mistake; a consume-on-first-use - newtype would. + newtype would. `ChannelHub::new` returning `(hub, children)` in v0.8 is + the same lesson applied: make the type refuse the mistake. + +## v0.8 — Bus, hub and session registries are supervised — DONE (2026-08-20) + +Same crate version (**0.3.0**); this cycle and v0.7 ship together and are +both breaking. + +v0.7 put the endpoint under a supervisor. This puts everything else there +too, because smarm 0.7 left it no choice: commit `415effb` ("lifetime is +the actor's — refs are addresses") removed the rule the pubsub table, +channel hub bus and session registries were built on. A `GenServerRef` no +longer owns its server; the loop holds its own inbox sender and the inbox +never closes when the last ref drops. Nothing commanded these three to +stop, and what still terminated a run was the root-exit sweep racing the +drain. + +That is the flake above: `shutdown_with_open_chat_terminates` and +`channels_wire::shutdown_with_open_channel_terminates`, 4 failures in 25 +`--all-features` integration runs, all "serve did not return: ... outlived +the drain: Timeout". 0 in 10 full-suite runs after the fix. + +**Description separated from instantiation.** The gotchas here all +descended from constructors that spawn — `PubSub::new()`, +`ChannelHub::new()` and `PrefixRouter::channel_session()` all started +actors, which forced in-runtime-only construction, which forced the +`Arc>` lazy-init from the first handler, which forced the +"cell must not be `static`" and "a relay must never hold a `PubSub` clone" +rules. Five documented rules propping up one inverted dependency. + +``` +your root sup (RestForOne) +├── ChildSpec(Permanent, PubSub::child) <- bus table, named +├── ChildSpec(Permanent, session registry) <- one per channel_session +└── ChildSpec(Permanent, urus::endpoint(..)) <- starts last, drains first +``` + +- **`PubSub` is a name, not a ref.** `const`-constructible, `Copy`, + spawns nothing, legal outside the runtime and in a `static`. Operations + resolve through the registry per call, so a supervisor-restarted table is + reached transparently. `PubSub::new()` is gone: `PubSub::new(name)` + + `PubSub::child()`. +- **`ChannelHub::new(bus, router) -> (hub, Vec)`**, `#[must_use]`. + Both halves come back together because a hub whose children were never + started compiles and fails on the first join. There is no `children()` + method to forget to call. +- **`channel_session` takes a registry name**; each registry is separately + named and supervised. +- **`serve_with` / `serve_with_shutdown` take a `Vec`** of app + children and build `RestForOne[..app children, endpoint]`. App children + start before the endpoint and — shutdown being ordered in reverse — stop + after it has drained, so a request still in flight can still reach the + bus. `RestForOne` because a bus crash leaves live sockets addressing a + table that no longer knows them. +- Deleted: the `Arc` idiom from both examples and both test + pipelines, and every module rule that existed only to hand-manage a + refcount. `ws_chat` lost 21 lines and a struct field; examples net -25. + +**Cost accepted:** one registry resolution per operation, including per +broadcast. Caching a `GenServerRef` in the handle would save it and go +stale across exactly the restart the supervisor exists to perform. Bench +before optimising. + +**Open after this cycle:** +- `channel_session("session:*", "chat-sessions", f)` puts two unrelated + string literals side by side and nothing catches a transposition. A + `RegistryName` newtype makes it a type error. Not done. +- `PubSub` cannot be protected the way `ChannelHub` was: a `const` is + copied at each use, so it can't be consume-on-first-use. Forgetting + `BUS.child()` compiles and fails at runtime with `PubSubDown`. Judged + worth it — the `const` is what makes the handle pleasant. +- Session actors are still plain-spawned by their registry, not supervised + (they're dynamic, one per key — OTP would want `simple_one_for_one`). + Their exit chain is now commanded rather than refcounted: the registry is + shut down, its state drops, control senders drop, parked sessions wake on + the closed arm. Sound, but it is the last place a lifetime is implied by + a drop rather than stated. +- `hammer.sh` still passes no feature flags, which is why this hid for a + whole cycle. A feature matrix is the obvious fix. ## Known bugs diff --git a/src/channels/mod.rs b/src/channels/mod.rs index 3986dbb..b77eec2 100644 --- a/src/channels/mod.rs +++ b/src/channels/mod.rs @@ -9,12 +9,12 @@ //! //! # Topology //! -//! - The app builds one [`ChannelHub`] (a [`TopicRouter`] + a -//! `PubSub>`). **In-runtime only** — `PubSub::new` -//! spawns the table actor — and the hub must be NON-static (the -//! `Arc>`-in-the-route-closure pattern from v0.5; -//! a `static` hub pins the pubsub table and hangs -//! `serve_with_shutdown`). +//! - The app builds one [`ChannelHub`] (a [`TopicRouter`] + the name of +//! a `PubSub>`). It is a description and spawns nothing, +//! so it may be built anywhere; [`ChannelHub::new`] hands back the +//! `ChildSpec`s for the actors it needs — the bus table, plus one +//! registry per session route — which go in the supervision tree +//! ahead of the endpoint. //! - [`ChannelHub::upgrade`] turns an HTTP `Conn` into a channel //! socket: a [`WsHandler`] running in the connection actor that //! decodes frames and routes them by topic. diff --git a/src/channels/session.rs b/src/channels/session.rs index 84f1b21..e47345c 100644 --- a/src/channels/session.rs +++ b/src/channels/session.rs @@ -22,10 +22,17 @@ //! the transport is the point. Its exits: explicit leave, rejected //! (re)join, TTL expiry, buffer cap, pubsub relay death, or the //! registry's control sender dropping — which is exactly the shutdown -//! chain (conns die -> router `Arc`s drop -> registry's inbox closes -//! -> registry exits -> control senders drop -> every parked session -//! wakes on the closed control arm, terminates, and exits; `AllDone` -//! composes without links). +//! chain (the supervisor shuts the registry down after the endpoint +//! has drained -> the registry's state drops -> control senders drop +//! -> every parked session wakes on the closed control arm, +//! terminates, and exits; `AllDone` composes without links). +//! +//! Note this is the last lifetime in urus implied by a drop rather +//! than stated: sessions are dynamic (one per key), so a fixed +//! `ChildSpec` list cannot hold them. The trigger is now a command +//! rather than a refcount, which is what the v0.8 cycle was about, but +//! a dynamic supervisor (OTP's `simple_one_for_one`) is the honest +//! shape if smarm grows one. //! //! As-landed decisions (veto by diff): //! - **Every attach calls `ch.join()` again** on the same instance —