Capture the handoff roadmap in-repo, with #1 and #2 marked complete (commit SHAs, decisions taken, and the orphaned-timer bug fixed in e80334b). #3 (links + trap_exit) flagged as next.
This commit is contained in:
smarm-dev
@@ -0,0 +1,170 @@
|
|||||||
|
# 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 6 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)
|
||||||
|
- 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 ← NEXT
|
||||||
|
- `Slot.links: Vec<Pid>` (bidirectional); `link`/`unlink`; per-actor
|
||||||
|
`trap_exit` flag.
|
||||||
|
- On finalize, for each linked peer: abnormal death → `request_stop(peer)`
|
||||||
|
unless peer traps, in which case deliver an exit *message* instead. Normal
|
||||||
|
exit does NOT propagate. Linking an already-dead pid delivers an immediate
|
||||||
|
exit signal.
|
||||||
|
- Open decision: which channel a trapping actor reads exit messages from
|
||||||
|
(reuse a Down-style channel vs a dedicated trap inbox) — ties into the
|
||||||
|
no-unified-mailbox constraint below. (`Signal::Stopped` + the reverse-order
|
||||||
|
stop helper from #2 are now available to build on.)
|
||||||
|
- Tests: linked pair one panics → other stopped; with trap_exit → other gets a
|
||||||
|
message and survives; normal exit doesn't propagate.
|
||||||
|
|
||||||
|
### 4. Selective receive (independent track)
|
||||||
|
Channels are strict FIFO MPSC; there's no pattern-matched `receive`.
|
||||||
|
- 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)
|
||||||
|
- `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>`.
|
||||||
|
- 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.
|
||||||
|
- 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, and why
|
||||||
|
trap_exit/selective-receive delivery need an explicit channel 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.
|
||||||
Reference in New Issue
Block a user