smarm-dev
024abc2e73
Mainline now carries roadmap #1-#5; the aarch64 context-switch port is a single untested commit on the arm-port branch. Update the task.md resume block and the README build section accordingly.
264 lines
17 KiB
Markdown
264 lines
17 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: `smarm`. Two branches (the old single `arm-port` stack was split):
|
||
- `master` — the mainline, and HEAD. Carries roadmap #1–#5: cooperative
|
||
cancellation, supervisor strategies (one_for_one/all, rest_for_one) + the
|
||
orphaned-timer shutdown fix, links/trap_exit, selective receive, gen_server,
|
||
and demonitor/`MonitorId`. Tagged `v0.4.0`. x86-64 Linux only.
|
||
- `arm-port` — `master` plus a single commit: `feat(arch): aarch64 context
|
||
switch + cycle counter`. Extracts the x86-64 context-switch / stack-init /
|
||
cycle-counter out of `context.rs` into a `target_arch`-gated `src/arch/`
|
||
(x86_64 + aarch64 backends) and adds an AAPCS64 backend. ⚠️ UNTESTED: never
|
||
built or run on real ARM hardware. The x86-64 path is unchanged
|
||
(`arch/x86_64.rs` is the old `context.rs` body verbatim), so the x86 suite
|
||
passing says nothing about the aarch64 backend. Build + test on-device
|
||
before trusting it.
|
||
- 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: README now points at the experimental, untested aarch64 port on the
|
||
`arm-port` branch. The module table still calls `context` x86-64-only — true
|
||
for `master`, since the `src/arch/` split rides on `arm-port`. Fold the arch/
|
||
split and ARM64-supported wording into the README module table + build section
|
||
once `arm-port` is validated on hardware and merged.
|
||
|
||
## 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.
|