smarm-dev
518103750b
Monitors could be installed but never taken down. That gap was about to
bite: the gen_server call timeout we want next is the Erlang dance —
monitor the server, wait for the reply or a Down or a deadline, then
demonitor — and without a way to remove a registration, every timed-out
call would leak a monitor on the server's slot and risk a stale Down
arriving later. So this lands the cleanup primitive before anything
depends on the old shape.
The decision flagged in the roadmap ("decide the monitor API NOW") is
resolved by giving each registration a process-unique MonitorId and
returning it to the caller. monitor() now hands back a
Monitor { id, target, rx } rather than a bare Receiver<Down>: read the
notice from rx as before, and pass &Monitor to demonitor to tear exactly
that registration down. The id comes from a monotonic counter in shared
state, bumped under the same lock that does the registration, so it's
deterministic and never reused — which is what lets demonitor name one of
several monitors on the same target unambiguously. target rode along on
the struct (over the sketched {id, rx}) purely so demonitor can go
straight to the slot instead of scanning every slot for the id.
demonitor returns Option<MonitorId> rather than a bool: Some(id) when a
live registration was found and removed, None when there was nothing left
to remove — it already fired (the registration is drained on finalize),
it was a NoProc, or the slot has been reclaimed. The generation half of
the pid quietly protects that last case: a recycled slot index fails
slot_mut's generation check, so a late demonitor is a clean no-op and can
never strip a different actor's monitor that happens to share the index.
The one piece of real care is reentrancy. Removing a registration drops
the slot's Sender, and Sender::drop can unpark a parked receiver, which
re-enters the shared mutex — which is not reentrant. So demonitor moves
the sender out of the Vec under the lock and lets it drop only after the
lock is released, the same discipline finalize_actor already follows for
its monitor and supervisor sends.
Flushing a Down the target already queued isn't a separate flag; it falls
out of dropping the Monitor. demonitor(&m); drop(m) stops future notices
and discards any queued one — the gen_server-call cleanup in one move.
Storage is now Vec<(MonitorId, Sender<Down>)>. The three slot-reset sites
were left alone on purpose: they clear/rebuild the Vec, which doesn't care
about the element type, so there's no fourth reset obligation. finalize
just destructures (_, m). Because chained_spawn and yield_many register no
monitors, that Vec stays empty on the hot path — taking an empty Vec costs
the same and the notify loop runs zero times — and a before/after general
probe confirmed both medians sit within noise.
Tests cover the three behaviours that matter: a demonitored watcher gets
no Down (its channel closes), demonitoring one of several leaves the
siblings firing normally, and demonitoring after the Down has already
fired reports None.
259 lines
16 KiB
Markdown
259 lines
16 KiB
Markdown
# smarm — task.md (next steps)
|
|
|
|
Handoff for a future session reusing this sandbox. Read top to bottom once
|
|
before starting; the gotchas section is hard-won and will save you a faceplant.
|
|
|
|
## Resume the environment
|
|
|
|
- Repo: `/home/claude/work/smarm`, branch `arm-port`, currently 8 commits past
|
|
`master`:
|
|
- `a8fec02` aarch64 context-switch port
|
|
- `183fd05` monitors (`monitor()` → `Receiver<Down>`)
|
|
- `f617266` one-for-one supervisor (restart policies + intensity cap)
|
|
- `a8ddb4a` cooperative cancellation (roadmap #1 — done)
|
|
- `e80334b` fix: orphaned timers no longer block runtime shutdown
|
|
- `351dc9c` one_for_all / rest_for_one + ordered shutdown (roadmap #2 — done)
|
|
- `31133a6` docs(roadmap): mark #1 and #2 done
|
|
- `6581484` links + trap_exit (roadmap #3 — done)
|
|
- (+ a trailing docs commit marking #3 done and refreshing supervisor/README)
|
|
- Toolchain is installed but NOT on PATH in a fresh shell. First line of every
|
|
session: `. "$HOME/.cargo/env"` (rustc/cargo 1.96).
|
|
- Build `cargo build` · all tests `cargo test` · one suite `cargo test --test monitor`.
|
|
- Bench probe `cargo bench --bench general` (custom print-only harness; compiles
|
|
tokio in release the first time — slow — but `target/` persists across git
|
|
checkouts so it's paid once).
|
|
- Perf regression check: `git checkout <pre-change-sha>` →
|
|
`cargo bench --bench general | tee before.txt` → `git checkout arm-port` →
|
|
run again → diff the `smarm 1-thread` medians for `chained_spawn` and
|
|
`yield_many` (those exercise spawn/finalize/scheduler). Numbers are noisy on
|
|
this shared CPU; treat as "regression beyond noise?" not a precise delta.
|
|
|
|
## Roadmap (dependency order)
|
|
|
|
### 1. Cooperative cancellation — the keystone ✅ DONE (`a8ddb4a`)
|
|
Everything below (one_for_all/rest_for_one, links) needs a *safe* way to stop a
|
|
running peer. Forcible teardown of another green thread's stack is unsound here
|
|
(shared heap + Drop). So: cooperative stop the actor observes and unwinds itself.
|
|
|
|
Shipped as designed (sentinel unwind, not Result-threading). Notes for what
|
|
came next / future readers:
|
|
- Stop flag lives on `Actor` behind `Arc<AtomicBool>` (fresh per spawn), NOT a
|
|
`Slot` field — sidesteps the three-place reset, at the cost of one small
|
|
alloc per spawn. The scheduler hands the resume path a raw `*const AtomicBool`
|
|
(no per-resume refcount traffic); `yield_many` bench stayed at baseline,
|
|
`chained_spawn` ~+6% from that alloc (left as-is; move to a `Slot` field if it
|
|
ever matters).
|
|
- Observation points: amortised `maybe_preempt`/`check!()` path + the wakeup
|
|
side of `park_current`/`yield_now`. Sentinel = `StopSentinel` (zero-size),
|
|
recognised in the trampoline → `Outcome::Stopped`. `join()` on a stopped actor
|
|
returns `Ok(())` (no payload to propagate; reason is on the monitor channel).
|
|
- Documented gaps confirmed by tests: no-observation-point loop can't be stopped
|
|
(same as preemption); a user `catch_unwind` can swallow the sentinel but the
|
|
flag stays set so the next yield re-raises.
|
|
|
|
Original plan, for reference:
|
|
- Add a per-actor stop flag (Slot field + atomic, or check via shared state).
|
|
- `request_stop(pid)`: set the flag, unpark if parked.
|
|
- Realize the stop as a **controlled unwind**: when the scheduler resumes a
|
|
stop-requested actor, inject a sentinel panic (dedicated payload type) so the
|
|
existing `trampoline` `catch_unwind` tears the stack down and runs Drop. The
|
|
trampoline recognizes the sentinel and reports a new `Outcome::Stopped`
|
|
(distinct from a user `Panic`). This avoids changing every blocking-op
|
|
signature.
|
|
- Alternative considered: thread `Result<_, Cancelled>` through recv/sleep/
|
|
lock/io. Rejected — large API churn. Go with the sentinel unwind.
|
|
- Caveat to document: user code with its own `catch_unwind` can swallow the
|
|
sentinel (cf. Erlang `catch`); re-check the flag at the next yield and/or
|
|
re-raise. And a tight no-alloc loop without `check!()` can't be stopped —
|
|
same inherent limitation as preemption.
|
|
- Observation points: `maybe_preempt()`/`check!()` (cheap flag check) and the
|
|
blocking parks (recv/sleep/mutex/io) on the stop-driven unpark.
|
|
- Tests: looping actor on `check!()` gets stopped → `Outcome::Stopped`, Drop
|
|
guards ran; parked-on-recv actor gets stopped; no-check loop documents the gap.
|
|
|
|
### 2. one_for_all / rest_for_one + ordered shutdown ✅ DONE (`351dc9c`)
|
|
Shipped. What landed vs the plan:
|
|
- `Strategy::{OneForOne,OneForAll,RestForOne}` selected via `.strategy()`,
|
|
default `OneForOne`. The struct keeps the `OneForOne` name (compat; existing
|
|
tests untouched) — a rename to `Supervisor` is a deferred refactor.
|
|
- The *triggering* child's `Restart` policy decides whether anything restarts;
|
|
the strategy decides which live siblings are cycled (all / index-> after the
|
|
failed one). Survivors are `request_stop`'d in reverse start order, awaited on
|
|
the existing `supervisor_channel` funnel (no new channel, no `select`),
|
|
restarted in start order. One failure = one intensity tick regardless of group
|
|
size. Out-of-band signals during an await are stashed and replayed.
|
|
- `Signal::Stopped(pid)` + `DownReason::Stopped` added (kept distinct from Exit,
|
|
as planned). A `Stopped` signal counts as abnormal for the restart decision.
|
|
- Ordered shutdown: on cap-trip / mailbox-close-with-survivors, stop remaining
|
|
children in reverse start order and await them (no-op on the normal exit).
|
|
- ⚠️ Surfaced + fixed a latent keystone bug (`e80334b`): a cancelled
|
|
sleeping/timeout actor orphans its timer entry, and the scheduler's shutdown
|
|
check counted pending timers → `run()` hung until the dead actor's deadline
|
|
fired (a `sleep(30s)` sleeper hung shutdown 30s). Fix: timers no longer gate
|
|
shutdown (`live == 0` already implies nothing a timer could wake); heap is
|
|
cleared on exit. Independent of the supervisor work.
|
|
- Tests: all-restart (sibling cycled despite clean exit), suffix-restart
|
|
(prefix child left alone), reverse-order teardown.
|
|
|
|
Original plan, for reference:
|
|
- one_for_all: on any child failure, `request_stop` all siblings, await their
|
|
termination signals, restart all per spec.
|
|
- rest_for_one: stop+restart the failed child and those started after it.
|
|
- Supervisor shutdown: stop children in reverse start order.
|
|
- Decide signal surface: add `Signal::Stopped(pid)` + `DownReason::Stopped`
|
|
rather than folding into Exit (clearer for the supervisor's await logic).
|
|
- Tests: all-restart, suffix-restart, reverse-order shutdown.
|
|
|
|
### 3. Links + trap_exit ✅ DONE (`6581484`)
|
|
- `Slot.links: Vec<Pid>` (bidirectional); `link`/`unlink`; `trap_exit()` flag
|
|
lives on `Actor` (fresh per spawn → a restarted child starts un-trapped, and
|
|
no fourth slot-reset site).
|
|
- On finalize, reverse links are cleared under the lock (always — keeps the
|
|
cascade acyclic), then for each linked peer: abnormal death (`Panic`/
|
|
`Stopped`) → `request_stop(peer)` unless peer traps, in which case deliver an
|
|
`ExitSignal` *message* instead. Normal exit does NOT propagate. Linking an
|
|
already-dead pid delivers an immediate `NoProc` signal (message if trapping,
|
|
else `request_stop(self)` — not a silent no-op).
|
|
- Resolved: the trap inbox is a **dedicated** channel (`trap_exit() ->
|
|
Receiver<ExitSignal>`), distinct from the monitor `Down` channel; `ExitSignal`
|
|
reuses `DownReason` and carries no panic payload (joiner-only, as with
|
|
monitors). `spawn_link` deferred to #5.
|
|
- Tests (`tests/link.rs`): linked pair one panics → other stopped (+Drop ran);
|
|
trap_exit → other gets a message and survives; normal exit doesn't propagate;
|
|
dead-pid link stops a non-trapper / messages a trapper; `unlink` prevents
|
|
propagation.
|
|
|
|
### 4. Selective receive (independent track) ✅ DONE (`03f3875`)
|
|
Shipped. What landed vs the plan:
|
|
- `Receiver::recv_match(pred) -> Result<T, RecvError>` scans the queued
|
|
`VecDeque` front-to-back, removes+returns the first match, leaves the rest in
|
|
arrival order; parks and re-scans when nothing matches. `try_recv_match` (the
|
|
non-blocking variant, mirroring `try_recv`) rolled in same commit.
|
|
- Wakeup turned out cheaper than feared: `Sender::send` *already* took
|
|
`parked_receiver` on every push, so "wake on ANY send" needed no send-side
|
|
change. The one real edit was relaxing `Sender::drop` to unpark the parked
|
|
receiver on the last-sender drop regardless of queue emptiness — a selective
|
|
receiver can park on a non-empty no-match queue and must wake to observe
|
|
closure. No-op for plain `recv` (only ever parks on an empty queue); stress
|
|
suite stays green.
|
|
- `pred` is `Fn(&T) -> bool` (not `FnMut`) on purpose: it's re-run from scratch
|
|
on every scan, so a stateful predicate would re-count surprisingly. It runs
|
|
under the channel lock — keep it cheap/pure, don't re-enter the channel.
|
|
- Close semantics: `recv_match` returns `Err(RecvError)` only when closed AND no
|
|
queued message matches; a match is still returned on a closed channel.
|
|
Non-matches are left for a later `recv`.
|
|
- Tests (`tests/selective_recv.rs`): out-of-order match pulled first; non-matches
|
|
remain in order; park-on-non-empty then wake on a match; closed-with-only-
|
|
non-matches → Err; closed-but-match-present → match; `try_recv_match` states.
|
|
|
|
Original plan, for reference:
|
|
- Add `Receiver::recv_match(pred) -> T`: scan the queued `VecDeque`, remove+return
|
|
first match, leave the rest in order; park and re-scan on new arrivals.
|
|
- This changes channel wakeup: a parked selective receiver must wake on ANY send
|
|
(not just empty→nonempty) and re-scan. Touches `channel.rs` carefully — the
|
|
stress tests guard lost-wakeup invariants; keep them green.
|
|
- Tests: messages arrive out of interest-order; match pulled first; non-matches
|
|
remain for a later `recv`.
|
|
|
|
### 5. Grab-bag (each its own small commit)
|
|
- `spawn_link`: spawn-and-link atomically (deferred from #3); thin wrapper over
|
|
`spawn_under` + `link`, but do it under one lock so there's no window where
|
|
the child dies before the link is recorded.
|
|
- `demonitor`: needs a per-monitor id to remove a specific sender. Decide the
|
|
monitor API NOW before more code depends on it — likely return a
|
|
`Monitor { id, rx }` instead of a bare `Receiver<Down>`.
|
|
✅ DONE (this commit). What landed vs the plan:
|
|
- `monitor()` now returns `Monitor { id, target, rx }` (added `target` over
|
|
the sketched `{id, rx}` so `demonitor` jumps straight to the slot instead of
|
|
scanning every slot for the id). `MonitorId(u64)` is opaque, from a
|
|
monotonic `next_monitor_id` counter on `SharedState`, bumped under the
|
|
shared lock in `monitor()` — no atomics, deterministic, never reused.
|
|
- `Slot.monitors: Vec<(MonitorId, Sender<Down>)>`. The three slot-reset sites
|
|
were untouched — they `.clear()`/`Vec::new()`, which is element-type-
|
|
agnostic, so no new reset obligation. `finalize_actor` just destructures
|
|
`(_, m)` and sends as before.
|
|
- `demonitor(&Monitor) -> Option<MonitorId>`: `Some(id)` when a live
|
|
registration was found+removed, `None` when it had already fired (drained on
|
|
finalize), was `NoProc`, or the slot was reclaimed. Chose `Option<MonitorId>`
|
|
over a bare bool — names which registration went. Generation half of the pid
|
|
makes a stale demonitor a clean no-op: a recycled slot index fails
|
|
`slot_mut`'s generation check, so it can never strip a different actor's
|
|
monitor.
|
|
- ⚠️ Reentrancy: the removed `Sender` is `remove`d out of the Vec under the
|
|
lock but **dropped after the lock is released** — `Sender::drop` can unpark a
|
|
parked receiver → `with_shared`, and the shared mutex is non-reentrant. Same
|
|
discipline as `finalize_actor`.
|
|
- "Flush" (discard a `Down` the target already queued) falls out of dropping
|
|
the `Monitor`: `demonitor(&m); drop(m)`. That's the cleanup the still-to-come
|
|
gen_server **call timeout** wants — monitor the server, wait reply-or-Down-
|
|
or-deadline, then demonitor+drop so a timed-out call leaks no registration
|
|
and no stale `Down`.
|
|
- Perf: touches `Slot` + `finalize_actor`, but `chained_spawn`/`yield_many`
|
|
register no monitors, so the Vec stays empty (take-empty is identical cost,
|
|
finalize loop runs zero times). before/after `general` probe medians within
|
|
noise. Tests (`tests/monitor.rs`): demonitor-stops-delivery, one-of-many
|
|
(siblings untouched), after-fire-is-None.
|
|
- Named registry: `register(name,pid)`/`whereis`/`send_by_name`; a
|
|
`HashMap<String,Pid>` in `SharedState`.
|
|
- gen_server-style call/cast: request-reply correlation as a thin layer over
|
|
channels (`call` sends `{req, reply_tx}`, awaits `reply_rx`); no runtime change.
|
|
✅ DONE (`a4fcf6c`). What landed vs the plan:
|
|
- `GenServer` trait on the state value: assoc `Call`/`Reply`/`Cast` types,
|
|
required `handle_call`/`handle_cast`, optional `init`/`terminate` hooks.
|
|
`ServerRef<G>` is a clonable inbox sender + `pid()`; `start` / `start_under`.
|
|
- One inbox, not two: a single `Envelope { Call(req, reply_tx) | Cast }`
|
|
channel, dispatched by variant. Forced by no-`select`/no-unified-mailbox —
|
|
a server can't wait on a call channel and a cast channel at once.
|
|
- Server-down falls out of channel closure (no monitor needed): `send` fails
|
|
if the inbox is gone; the reply sender drops on the server's unwind so a
|
|
parked caller wakes to `Err`. Both → `Call/CastError::ServerDown`.
|
|
- `terminate` runs via a drop guard → fires on *every* exit path (clean inbox
|
|
close, handler panic, `request_stop`), not just the clean one. Caveat: it
|
|
may run mid-unwind, so keep it non-blocking (a panic inside it during an
|
|
unwind double-panics → abort).
|
|
- No `handle_info`, no call timeout — both deferred to land with timeouts
|
|
(`handle_info` needs the still-unmade cross-channel mailbox merge; a call
|
|
timeout needs a per-`recv` deadline / `Signal::Timeout`).
|
|
- Pure additive layer (no Slot/scheduler/spawn/finalize change) → no perf
|
|
check run. Tests (`tests/gen_server.rs`): cast→call roundtrip, init/terminate
|
|
ordering, both server-down paths.
|
|
- Docs lag: README still says "x86-64 Linux only — ARM64 … deferred" and the
|
|
module table calls `context` x86-64-only; update for the aarch64 backend.
|
|
|
|
## Gotchas / invariants (respect these)
|
|
|
|
- **Shared mutex is non-reentrant.** `Sender::send` can call `unpark` →
|
|
`with_shared`. NEVER send on a channel while holding the shared lock. Pattern:
|
|
`mem::take` the senders/data under the lock, send after releasing. See
|
|
`finalize_actor` (supervisor signal + monitor Downs both sent post-lock).
|
|
- **`finalize_actor` order:** take stack/waiters/monitors under lock + set
|
|
Done/outcome → recycle stack (post-lock) → deliver supervisor Signal + monitor
|
|
Downs (post-lock) → unpark joiners → reclaim slot iff `outstanding_handles==0`.
|
|
Death notifications always precede reclamation, so a pid carried in a
|
|
Signal/Down is still matchable even as its slot is about to be reused.
|
|
- **Slot lifecycle is reset in THREE places** — `Slot::vacant()`,
|
|
`reclaim_slot()` (runtime.rs), and the slot-init block in `spawn_under`
|
|
(scheduler.rs). Any new Slot field must be reset in all three (monitors was).
|
|
- **Pid = (index, generation);** stale handles caught by generation mismatch in
|
|
`slot()/slot_mut()`. The monitor `NoProc` path relies on this.
|
|
- **No `select`, no unified per-process mailbox.** Why the supervisor uses the
|
|
single `supervisor_channel` funnel rather than N monitor channels. trap_exit
|
|
resolved this by giving each trapping actor a dedicated `Receiver<ExitSignal>`
|
|
inbox (see #3); selective receive (#4) stayed *per-channel* (`recv_match`
|
|
scans one channel's queue) rather than introducing a cross-channel mailbox —
|
|
if selective receive ever needs to span the monitor/trap inboxes too, that
|
|
cross-channel merge is the still-unmade decision.
|
|
- **Cooperative-only**: preemption and (future) cancellation both depend on the
|
|
actor reaching `check!()`/yield/alloc/blocking points.
|
|
- `run()` is single-thread (`Config::exact(1)`); tests rely on deterministic
|
|
single-thread ordering (parent runs until it parks). Multi-thread via
|
|
`runtime::init(Config…)`.
|
|
|
|
## Workflow expectations (from the human)
|
|
|
|
- TDD: write the failing test first, then implement.
|
|
- Commit incrementally with conventional-commit messages; keep each commit a
|
|
reviewable unit (they diff in their IDE and are the filter to the codebase).
|
|
- Run the full suite before each commit; check perf when a change touches
|
|
Slot/scheduler/spawn/finalize hot paths.
|