From e570138da5c6ce39731b9973efec0c6ae1feaec0 Mon Sep 17 00:00:00 2001 From: "Claude (sandbox)" Date: Thu, 20 Aug 2026 13:20:42 +0000 Subject: [PATCH] docs(roadmap): supervisor start order is not start readiness Filed from the urus v0.3 endpoint work. start_child spawns and moves on, so a later sibling can whereis an earlier named child before that child's actor has run. Notes why blocking spawn is not the fix ('has begun executing' != 'has bound its name', plus a per-accept round-trip tax and every spawn becoming a context-switch point), that OTP has the same async spawn and synchronises one level up in gen_server:start_link, the readiness-ack shape if scheduled, and the structural workaround urus uses today (registrar spawns its own consumers). --- ROADMAP.md | 22 +++++ TODO_HANDOFF.md | 229 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 251 insertions(+) create mode 100644 TODO_HANDOFF.md diff --git a/ROADMAP.md b/ROADMAP.md index ebc36b5..fdbd931 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -283,6 +283,28 @@ outright — `join` what you need finished. No forcing sweep follows. - A root-exit shutdown reaches only actors live *at that instant*; a non-trapping forest root that spawns before it unwinds leaves that spawn to itself (Erlang: an unlinked spawn is nobody's child). +- **Supervisor start *order* is not start *readiness*.** `start_child` + spawns and moves straight on, so an earlier child is merely *scheduled*, + not initialised, when a later sibling starts. A later child that resolves + an earlier one by name (`whereis_server`) can therefore miss it — the + classic "named registry sibling, then its consumers" tree. Ordered + `OneForOne`/`RestForOne` shutdown is unaffected (reverse order is honoured + and each stop *is* awaited); this is a start-side gap only. + Making `spawn` itself block does NOT fix it — it would only shrink the + window to "child has begun executing", while the property callers need is + "child has bound its name / opened its socket", which only the child can + declare. It would also tax the hot path (one round-trip per accepted + connection) and turn every spawn into a context-switch point. OTP has the + same async `spawn` and puts the synchronisation one level up: + `gen_server:start_link` blocks the caller until `init/1` returns. + Fix shape when scheduled: a readiness ack in the supervisor's child-start + path (`ChildSpec` variant whose factory receives a ready-signal; + `NamedGenServerBuilder::run` acks after its name bind, gen_server default + acks after `init`; plain closures ack at spawn as today, i.e. opt-in with + no cost to existing children). Until then the workaround is structural: + have the registrar spawn its own consumers so the ordering is program + order inside one actor, not a cross-actor guarantee (urus v0.3 endpoint + does exactly this). ## Invariants & gotchas (respect these across all cycles) diff --git a/TODO_HANDOFF.md b/TODO_HANDOFF.md new file mode 100644 index 0000000..33ca1cc --- /dev/null +++ b/TODO_HANDOFF.md @@ -0,0 +1,229 @@ +# urus / smarm handoff — updated 2026-08-19 (session 3) + +## TL;DR for the next session +**smarm is done for now** (5 unpushed commits on local `master`, see below). +**Next = urus v0.3 endpoint refactor.** You should NOT need to read smarm +scheduler internals; the contract you build on is fully described here and in +`smarm_full/examples/graceful_shutdown.rs` (read that file first — it is the +exact shape urus's tree will take) plus `smarm_full/tests/root_exit.rs`. + +### The smarm contract urus builds on (all on local master, verified by tests) +- `request_stop(pid)` = kill (cooperative hard stop). `request_shutdown(pid)` = + polite: trapping target gets `ExitSignal{reason: Shutdown}`, non-trapping is + stopped outright. `RuntimeHandle::{request_stop,request_shutdown}` do the same + from any OS thread (signal handler); grab `rt.handle()` before `rt.run`. +- Supervisor traps; `request_shutdown(sup)` = ordered reverse-start shutdown, + per-child `ChildSpec::shutdown(Shutdown::{Timeout(d)|Infinity|BrutalKill})` + (default Timeout(5s)); sup then returns normally. `request_stop(sup)` + hard-stops children too (no orphans). +- gen_server: `ctx.trap_exit()` in init; `handle_shutdown() -> Exit|Continue`; + `handle_exit(sig)`; `ctx.stop_handle().stop()` = normal self-exit; + `terminate()` may block only on the graceful path (Exit / stop / inbox close). + `GenServerRef::shutdown()` is graceful and waits. +- gen_statem: same in event clothes — `cx.trap_exit()` in initial enter, + `shutdown` rows (default `stop`), `exit sig` rows, `cx.stop()` / `stop` tail, + optional `terminate { }` block. `GenStatemRef::shutdown()`. +- **Root exit = program done**: when the root actor returns, the runtime + `request_shutdown`s every *forest root* (live actor whose parent is the run + or dead). Supervisors cascade; trapping actors may drain (timers keep + working) and end the run when they stop; non-trapping are stopped; **no + forcing sweep** (`join` what must finish). The old "wait until nothing + runnable then kill all" deferral is gone. +- **Gotcha for urus:** a gen_server's lifetime is governed by its refs — drop + the last `GenServerRef` and the inbox closes → clean exit *even mid-drain*. + The endpoint must be pinned (named, or its ref held by the supervisor + wrapper) or it will terminate the moment the root drops its ref. +- **Known gap (ROADMAP open item):** a gen_server can't be a direct `ChildSpec` + child; use the trapping wrapper pattern in `examples/graceful_shutdown.rs:: + drainer_child` (starts `under(self_pid())`, forwards shutdown, waits). Doing + an inline `GenServerBuilder::run()` first may be worth a short smarm detour — + decide with Markk. + +### smarm commits this session (local master, NOT pushed, NOT tagged) +`250f312` root-exit = graceful shutdown of forest roots (tests/root_exit.rs) +`6ceb138` gen_statem shutdown parity (tests/gen_statem_shutdown.rs) +`849a424` docs + examples/graceful_shutdown.rs + README "Stopping actors" +On top of `1002777` (cross-thread wake) and `9c8f59c` (graceful shutdown). +Cargo.toml still `0.6.1`. Release cut (push, tag — v0.7 is justified by the +API surface — version bump) is Markk's. Full suite, doc tests, examples, +`cargo fmt`, `cargo clippy --lib` all clean. (`clippy --tests` has pre-existing +unwrap lints in tests/fd_select.rs, untouched.) + +### Decisions taken this session (Markk) +- Root exit means "program done" (Go/tokio/OTP), not "wait for pending work"; + the previously agreed "sleep(50ms) must finish" test was dropped as encoding + the wrong contract (a timer-wheel gate would re-wedge periodic-timer daemons). +- No behaviour-preserving deferral, no forcing second sweep. +- Examples/docs done in the same session; urus next session. + +--- +# Previous handoff (still accurate where not superseded above) + + +## Next-session goal +Phase 1 is **done and committed**; Phase 2 is next: +1. **smarm v0.6.2** — cross-thread wake root fix. **DONE**, committed on `master` + as `1002777`. Not yet tagged, not yet version-bumped (Cargo.toml still reads + `0.6.1`), and **not yet pushed to origin** — it exists only in the delivered + snapshot zip and the local sandbox clone. Cutting the release (push + tag + `v0.6.2` + bump `0.6.1`→`0.6.2`) is Markk's step. +2. **urus v0.3** — endpoint refactor. Working against `smarm = { path = "../smarm_full" }` + with `git update-index --skip-worktree Cargo.toml` (Markk approved); release commit + swaps back to the tag once Markk cuts it. Phase 2 plan below is STALE where it says + drain-in-terminate; the endpoint is a trapping GenServer: `handle_shutdown` → + `Continue`, enter Draining, `StopHandle::stop()` when the conn set empties. + +Decisions below are locked unless marked *(confirm)*. + +## Reconstruction (the sandbox resets between sessions) +A fresh sandbox has an empty home and **no Rust toolchain**. To restore: +- Install rustup/cargo. smarm reformats under **rustc 1.97.1**; urus `rust-version` + is 1.95. Use 1.97.1. +- urus: `git clone https://git.kalsbeek.dev/Markk116/urus` — `origin` is registered + and public-read. master `8bdec97` = the v0.2.x line. (Zips in outputs are stale; + prefer the remote now.) +- smarm: `git clone https://git.kalsbeek.dev/Markk116/smarm`. Latest tag **v0.6.1** + (`ca1c983`). The cross-thread wake fix is committed as `1002777` on top of the + post-v0.6.1 README commit `8f2d513` (= origin/master). **It is NOT on origin + yet** — a fresh clone won't have it until Markk pushes. Restore it from the + snapshot zip if working before the push. **v0.6.2 is not yet tagged.** +- urus pins smarm by git **tag** in `Cargo.toml` (currently `v0.6.0`). A trivial + first commit bumps it to `v0.6.1` (also picks up `try_spawn` + monitor + terminal-outcome fixes). + +## Why (context — the finding that drives the plan) +urus's shutdown machinery (the `AtomicBool` listener flag + the `SHUTDOWN_POLL` +loop in `serve.rs`) is scaffolding around two smarm properties. Their statuses +differ, which is the whole point: + +- **Issue A — lossy stop vs a QUEUED actor: ALREADY FIXED in smarm.** Commit + `7bab4d2` added an entry-side `check_cancelled()` in `park_current`. A + `request_stop` against a listener parked in `wait_readable_timeout` now unwinds + cleanly (it parks via `try_select_timeout → park_current`). urus's flag + its + stale "smarm's lossy stop-while-QUEUED window" comment can be deleted. +- **Issue B — foreign-thread wake is a no-op: FIXED in `1002777` (was present + through v0.6.1).** The gap: `unpark`/`unpark_at`/`request_stop` all route through + `try_with_runtime`, which reads a thread-local that is `None` on any non-scheduler + thread, so no cross-thread wake worked — a signal handler / OS thread could not + wake *or* stop a parked actor, which is why `serve.rs` polls the shutdown signal + instead of parking on it. Now closed (see Phase 1 below): urus's `SHUTDOWN_POLL` + loop can be deleted and its `Handle::shutdown` can park on a handle-driven stop. + +## Phase 1 — smarm cross-thread wake (root fix) — DONE (`1002777`) +Shipped as one commit generalizing RFC 018 (a producer reaches the runtime through +a `Weak` it holds) from the IO backend to channel senders and a new handle: +- **`Runtime::handle() -> RuntimeHandle`** (`Send + Sync`), holding a + `Weak`. Grab it before `rt.run` and hand it to the signal thread. +- **`RuntimeHandle::request_stop(Pid)`** — upgrades the Weak and calls + `request_stop_inner` on the inner; no-op if the runtime is gone. This is the + signal-handler-drives-shutdown path; it cascades the ordered stop down the tree + exactly like an in-runtime `request_stop`. +- **Send-wake:** the receiver captures `scheduler::runtime_weak()` into its + `parked_receiver` tuple **at park time** (not at channel creation — the resolved + sub-decision; a parked receiver is a live actor so the Weak is provably upgradable, + and it scopes the capture to when a wake is possible). `send()` and last-sender + `drop` wake via `scheduler::unpark_at_via(pid, epoch, &weak)`: thread-local path + when on a scheduler thread (preempt-gated, slot-eligible), captured Weak otherwise. + In-runtime timer wakes (recv/select) were left on `scheduler::unpark_at`. + +**API scope decision (signed off):** `RuntimeHandle` exposes **`request_stop` only**. +No public `unpark`/`unpark_at` on the handle — send-wake needs no user-facing handle, +and "unpark off-runtime" is covered because `request_stop` drives `unpark` on the +upgraded inner. No `is_alive()`. Both are one-line additions if a consumer appears. + +**No RFC written** — pattern was already established (RFC 018), agreed not needed. + +Tests: `tests/cross_thread_wake.rs` (foreign-thread send wakes a parked receiver; +foreign-thread `request_stop` wakes+stops a parked actor; a lingering handle never +blocks all-done and degrades to a no-op once the runtime drops). Full suite green; +`cargo fmt` + `cargo clippy --lib` clean. + +**Remaining release step (Markk):** push `master`, tag `v0.6.2`, bump Cargo.toml +`0.6.1`→`0.6.2`. Left paired with the tag as the release cut, not done in `1002777`. + +## Phase 1b — smarm graceful shutdown (OTP lift) — DONE (`9c8f59c`, on top of `1002777`) +Decided this session (Markk): B — fix at the smarm level rather than a two-stop +split in urus. No RFC (Markk: "just implement it"). Shipped, tested, committed on +the local `master`, **not pushed, not tagged**. It should ship as the same +release as 1002777 (v0.6.2, or v0.7 given the API surface — Markk's call). +- `request_shutdown(pid)` / `RuntimeHandle::request_shutdown` = `exit(Pid, shutdown)`; + `request_stop` = `exit(Pid, kill)`. Trapping target gets `ExitSignal{reason: + DownReason::Shutdown}`; non-trapping is stopped outright. +- `ChildSpec::shutdown(Shutdown::{BrutalKill, Timeout(d), Infinity})`, default 5s. + Supervisor traps exits; `request_shutdown(sup)` = ordered top-down shutdown, + returns normally. **Also fixed**: `request_stop(sup)` used to ORPHAN children + (probe-verified; the handoff's "cascade" claim was wrong) — `Live` drop guard now + hard-stops them. +- gen_server: `ctx.trap_exit()`, `handle_shutdown() -> ShutdownAction::{Exit, + Continue}`, `handle_exit(ExitSignal)`, `ctx.stop_handle().stop()` = normal + self-exit (`{stop, normal}`; previously impossible — only abnormal `Stopped`). + `GenServerRef::shutdown()` is graceful now. +- Root finding that forced this: gen_server `terminate()` runs from a Drop guard, + mid-unwind on the stop path; any park in it = double panic = abort. So + "drain-in-terminate()" (the old Phase 2 plan) was never viable. + +### ~~Next-session smarm work~~ DONE this session (see TL;DR) +1. **Root-exit sweep**: make `Pop::RootDrain` also require an empty timer wheel + (and it already requires nothing runnable; io_out is only checked for AllDone — + check whether it should gate RootDrain too). TDD: an actor in `sleep(50ms)` when + the root returns must finish, not be swept. Then a `Reservoir`-style test that a + *truly* parked-forever daemon still gets swept. +2. **gen_statem parity**: `ctx.trap_exit()`, `handle_shutdown -> ShutdownAction`, + `handle_exit`, stop handle. Mirror gen_server; mechanical. +3. **Examples review**: `examples/*.rs` predate all of this. Rework where they show + shutdown/teardown to use `request_shutdown`, `Shutdown` policies, and + `StopHandle`; `named_genserver.rs` first (uses `shutdown`). Also + `docs/smarm - Deep Dive.html` says terminate() must be non-blocking — now only + true on the unwind paths; and README could use a "Stopping actors" paragraph + (request_stop = kill, request_shutdown = shutdown, Shutdown policy). + +### Open smarm items found on the way (noted, not scheduled) +- (root-exit sweep and gen_statem parity moved up to the scheduled list.) +- Sweep in the supervisor `Live` drop guard is `request_stop` (kill propagates as + kill); OTP would deliver a trappable `killed`. Chosen for boundedness. + +## Phase 2 — urus v0.3: endpoint refactor (after v0.6.2 is tagged) +Target = the spec's original shape (`urus-spec.md` §2.1/§6: `listener_sup` under the +**user's** root supervisor). Deviation to unwind: `serve` owning `rt.run`. +- App owns the runtime: `smarm::init(cfg).run(|| root_sup.run())`, root e.g. + `RestForOne[ app actors…, urus::endpoint(config, pipeline) ]`. This is what kills + the `Arc` idiom for the right reason (app state born in-runtime as a + supervised, ordered child). +- `urus::endpoint` = one GenServer child owning the registry + an **internal** + listener sub-supervisor + drain-in-`terminate()`. Listeners stay internal, not + app-visible peers. +- Shutdown = `request_stop` the root supervisor (or via the runtime handle from a + signal thread) → cascades down → `endpoint.terminate()` runs the drain + (`drain_timeout`, force-stop sweep). +- **DELETE:** the `AtomicBool` listener flag (A fixed) and the `SHUTDOWN_POLL` loop + + its apologetic comment (B fixed → park, don't poll). +- Nuance: `request_stop` → `Signal::Stopped` is *abnormal* → `Transient` restarts. + Stop-without-restart = stop the **supervisor**, not the children. +- *(confirm)* Keep `serve`/`serve_with`/`serve_with_shutdown` as thin wrappers that + build the one-child tree internally, so the simple case stays one line. +- *(confirm)* Keep `Handle`/`ShutdownSignal`? Now that cross-thread wake works, + `Handle::shutdown` can map to handle-driven `request_stop` on the endpoint. +- Breaking → cut **urus v0.3**; bump the smarm pin to `v0.6.2` here. + +## Working norms +- Every bash call: `export PATH=$HOME/.cargo/bin:$PATH` (once the toolchain's in). +- **TDD**: failing test first, then implement; keep suites green. +- **Hammer ritual** for ANY connection-lifecycle change (the urus #2 shutdown work + qualifies): 35× subset (`shutdown timeout reaped slowloris streaming chunked sse + stalled ws_ channels session`) + 3× full + 1× trace. `scripts/hammer.sh` does NOT + pass feature flags — loop manually with `--features phoenix`. Subset filter must + NOT use `--test integration` (session tests live in lib). +- Example smoke tests: hold the server's stdin open (`mkfifo` + `sleep > fifo`) or + the Enter-to-shutdown thread fires on EOF instantly. +- Background procs are reaped BETWEEN bash calls; `pkill -f` matches your own shell. +- Artefact store (specs): `curl -H "Authorization: Bearer sk-llmingest-2e45d80c63db24c6781e761eb2a9a58e83d9f48ef77a42185bad311d07c80e68" https://artefacts.kalsbeek.dev/artifacts/` + — `urus-spec.md`, `urus-bench-spec.md`, `rfc_008-implementation-notes.md`, … +- smarm feature flags: `smarm-trace`, `smarm-causal` (urus re-exports both). + +## Local cross-repo testing — KEEP OUT OF COMMITS +To test urus #2 against un-tagged smarm 0.6.2, point urus's `Cargo.toml` smarm dep +at a local path (`smarm = { path = "../smarm" }`) instead of the git tag. +- Must NOT land in commits. Guard: `git update-index --skip-worktree Cargo.toml` + after editing (undo with `--no-skip-worktree`), or stash before committing. +- The committed `Cargo.toml` stays pinned to the git tag; restore the tag (bumped to + `v0.6.2`) for the release commit.