T-015 / ADR-0032 — symmetric Deadlock rollback + ipc_cancel_recv - #17
Conversation
Careful re-read pass complete: - All forward-references point at real T-015 (5/5 dependency-chain steps). - Negative consequences (one new IPC entry point; doc-comment precision; cancel exercised only by Deadlock test) are real costs the project pays for symmetric-rollback + multi-waiter / preemption forward-compatibility. - Simulation table covers Phase 2 Deadlock path (rows 0-3b) end-to-end; Option A's row 3b adds the ipc_cancel_recv call before scheduler-state restoration. T-015 implementation lands in subsequent commits per the ADR's Dependency chain. Refs: ADR-0032 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cel_recv (T-015 / ADR-0032) Adds the recovery primitive defined by ADR-0032 and wires it into ipc_recv_and_yield's Phase 2 Deadlock branch so the bridge restores both scheduler AND endpoint state to the caller's pre-call shape on SchedError::Deadlock — restoring the Phase A "error path leaves observable state unchanged" invariant on both axes. Kernel changes: - kernel/src/ipc/mod.rs: new pub fn ipc_cancel_recv(ep_arena, queues, ep_cap, &caller_table) -> Result<(), IpcError>. Reverses an Idle → RecvWaiting transition; no-op for any other state. RECV-rights validated (same right that authorised the original ipc_recv); zero new unsafe — operates on existing EndpointArena / IpcQueues raw-pointer surface under UNSAFE-2026-0014's umbrella. - kernel/src/sched/mod.rs: ipc_recv_and_yield's Phase 2 Deadlock branch now drops its &mut Scheduler before opening a momentary &mut EndpointArena + &mut IpcQueues + &CapabilityTable block to call ipc_cancel_recv, then returns Err(SchedError::Deadlock). The two rollback scopes don't overlap; cancel_result is debug_asserted Ok (Phase 1 just validated ep_cap; cancel cannot fail in v1's cooperative single-thread invariant). - SchedError::Deadlock and ipc_recv_and_yield doc-comments updated: Rollback scope now spans both scheduler and endpoint state. Tests (152 → 158 host tests, 158/158 miri clean): - 5 new ipc::tests::cancel_recv_* unit tests covering the cancel state machine (RecvWaiting → Idle, no-op on Idle / SendPending, rights enforcement, idempotency). - 1 new sched::tests::ipc_recv_and_yield_deadlock_rolls_back_endpoint_state regression test — empirical form of ADR-0032's Simulation table row 3b. - existing T-007 ipc_recv_and_yield_returns_deadlock_when_ready_queue_empty test gained the endpoint-state assertion (the post-rollback ipc_recv must return Pending, proving the slot was reset to Idle). QEMU smoke verified — full demo trace through `tyrne: all tasks complete` + `boot-to-end elapsed = 4088000 ns`. Byte-for-byte identical message sequence to the post-T-014 baseline; T-015 adds no v1-reachable code path because Deadlock is structurally unreachable with register_idle installed (per ADR-0026). Refs: ADR-0032, T-015, UNSAFE-2026-0014 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e, audit log Amendment, roadmap promotions Doc-only updates that pair with the kernel implementation in 7a402cb. - ADR-0017 §Revision notes — new 2026-05-07 rider records the additive ipc_cancel_recv recovery primitive: it does NOT extend the three-primitive user-observable surface (send/recv/notify) ADR-0017 enumerated; it is consumed exclusively by ipc_recv_and_yield's Deadlock branch in v1 (kernel-internal). Future userspace destroy drains and a syscall-ABI ADR may expose it. - docs/architecture/ipc.md §State machine — the mermaid diagram gains the RecvWaiting → Idle reverse arc labelled "ipc_cancel_recv (recovery)"; one explanatory paragraph below describes the recovery contract and the symmetric-rollback invariant. - docs/audits/unsafe-log.md — UNSAFE-2026-0014 fourth Amendment names the new Deadlock-branch momentary &mut EndpointArena + &mut IpcQueues + &CapabilityTable site under the existing umbrella. No new audit entry needed; surface-matching discipline only. - docs/analysis/tasks/phase-b/T-015-...md — Status flipped Draft → Done (2026-05-07); Review history gained two new rows recording ADR-0032 Accept + implementation gates + smoke verification. - docs/analysis/tasks/phase-b/README.md — T-015 row promoted to Done. - docs/roadmap/current.md — header callout updated to record T-015's close as a B2-prep follow-on; Active task / Last completed tasks / Active decisions / Next task to open all reflect the post-T-015 state. ADR-0032 added to Active decisions. - docs/roadmap/phases/phase-b.md — B1 Status block notes the T-015 follow-on close; ADR ledger row for ADR-0032 updated to Accepted + pointing at T-015 Done. Refs: ADR-0032, T-015, ADR-0017, UNSAFE-2026-0014 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
Reviewer's GuideImplements ADR-0032 by adding an ipc_cancel_recv recovery primitive and wiring it into ipc_recv_and_yield’s Deadlock path so both scheduler and endpoint state are rolled back symmetrically, with accompanying tests and documentation/audit updates. Sequence diagram for ipc_recv_and_yield Deadlock rollback using ipc_cancel_recvsequenceDiagram
actor Task
participant Scheduler
participant IpcBridge as IpcBridge_ipc_recv_and_yield
participant IpcLayer as IpcLayer_ipc_recv
participant Endpoint
Task->>IpcBridge: ipc_recv_and_yield(ep_cap)
IpcBridge->>IpcLayer: ipc_recv(ep_cap)
IpcLayer->>Endpoint: register receiver
Endpoint-->>IpcLayer: state = RecvWaiting
IpcLayer-->>IpcBridge: RecvOutcome.Pending
IpcBridge->>Scheduler: block current on ep_handle
Scheduler->>Scheduler: ready.dequeue().or(idle)
Scheduler-->>IpcBridge: None (no next task, no idle)
IpcBridge->>Scheduler: restore current and task_states
Scheduler-->>IpcBridge: scheduler state restored
IpcBridge->>IpcLayer: ipc_cancel_recv(ep_cap)
IpcLayer->>Endpoint: RecvWaiting -> Idle
Endpoint-->>IpcLayer: state = Idle
IpcLayer-->>IpcBridge: Ok(())
IpcBridge-->>Task: Err(SchedError.Deadlock)
Class diagram for ipc_cancel_recv and scheduler Deadlock rollback integrationclassDiagram
class EndpointState {
<<enum>>
Idle
SendPending
RecvWaiting
RecvComplete
}
class EndpointHandle {
slot_index: usize
generation: u32
}
class EndpointArena {
+get(slot_id) EndpointHandle
}
class IpcQueues {
+state_of(handle EndpointHandle) EndpointState*
+reset_if_stale_generation(handle EndpointHandle)
}
class CapabilityTable {
+lookup(handle CapHandle) Capability
}
class CapHandle
class CapRights {
<<bitflags>>
SEND
RECV
}
class IpcError {
<<enum>>
InvalidCapability
QueueFull
PendingAfterResume
Other
}
class Scheduler {
+current: Option_TaskHandle_
+task_states: TaskState_array_
+ready: ReadyQueue
+idle: Option_TaskHandle_
}
class TaskState {
<<enum>>
Ready
Blocked_on_ep_handle_
Running
}
class SchedError {
<<enum>>
Deadlock
NoCurrentTask
Ipc
}
class IpcApi {
+ipc_send(ep_arena EndpointArena, queues IpcQueues, ep_cap CapHandle, caller_table CapabilityTable, msg Message, cap Capability) SendOutcome
+ipc_recv(ep_arena EndpointArena, queues IpcQueues, ep_cap CapHandle, caller_table CapabilityTable) RecvOutcome
+ipc_notify(notification_cap CapHandle) IpcError
+ipc_cancel_recv(ep_arena EndpointArena, queues IpcQueues, ep_cap CapHandle, caller_table CapabilityTable) Result_void__IpcError_
}
class BridgeApi {
+ipc_recv_and_yield(sched Scheduler, cpu Cpu, ep_arena EndpointArena, queues IpcQueues, caller_table CapabilityTable, ep_cap CapHandle) Result_RecvOutcome__SchedError_
}
Scheduler --> IpcQueues : uses
Scheduler --> EndpointArena : uses
Scheduler --> CapabilityTable : uses
BridgeApi --> Scheduler : mut_borrow
BridgeApi --> IpcApi : calls
IpcApi --> EndpointArena : uses
IpcApi --> IpcQueues : uses
IpcApi --> CapabilityTable : uses
IpcApi --> IpcError : returns
IpcQueues --> EndpointState : owns_array
Scheduler --> SchedError : returns
class ReadyQueue
class TaskHandle
class Message
class Capability
class SendOutcome
class RecvOutcome
Scheduler --> ReadyQueue
Scheduler --> TaskHandle
IpcApi --> Message
IpcApi --> Capability
IpcApi --> SendOutcome
IpcApi --> RecvOutcome
State diagram for EndpointState with ipc_cancel_recv recovery arcstateDiagram-v2
state EndpointState {
Idle
SendPending
RecvWaiting
RecvComplete
}
Idle --> SendPending: ipc_send enqueues
SendPending --> Idle: ipc_recv collects
Idle --> RecvWaiting: ipc_recv registers
RecvWaiting --> RecvComplete: ipc_send delivers
RecvComplete --> Idle: ipc_recv collects
RecvWaiting --> Idle: ipc_cancel_recv
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request implements the ipc_cancel_recv recovery primitive as specified in ADR-0032, ensuring that both scheduler and endpoint states are symmetrically rolled back when a deadlock is detected during ipc_recv_and_yield. The implementation includes the new primitive in the IPC subsystem, its integration into the scheduler's deadlock branch, and comprehensive updates to documentation, ADRs, and the project roadmap. Verification was confirmed through new unit tests, a scheduler regression test, and QEMU smoke testing. I have no feedback to provide as there were no review comments.
…losure status Re-verified all 13 §Follow-up backlog items from the 2026-05-07 PR #12-#17 multi-axis review against the current integration-branch state. All 9 hygiene items + the 1 forward-flagged P10 harness item are now closed; 3 remain forward-flagged on appropriate downstream venues. Two updates to the consolidated review file: 1. **Top-of-file closure-status banner** — readers see the per-item disposition at-a-glance without scrolling to the bottom backlog. 2. **§Follow-up backlog per-item closure annotations** — items 1-9 gain ✅ + closing-PR + closing-commit references; item 11 (P10 harness) gains ✅ + integration-PR reference + measured-baseline numbers; items 10/12/13 keep their forward-flagged status with "status unchanged 2026-05-08" markers. Re-verification at integration-branch HEAD (per item): | # | Item | Closing PR / commit | Verification | |---|---|---|---| | 1 | current.md + perf re-baseline `.text 22,020` | PR #18 / `94a6c0f` | grep "22,020 bytes" current.md → 1 hit | | 2 | cancel_recv_on_recv_complete test | PR #18 / `25854a1` | grep test name in ipc/mod.rs → 1 hit; host-test 159/159 | | 3 | ipc_cancel_recv doc-rider on cap-bearing state | PR #18 / `25854a1` | grep "destroy-drain callers (Phase B2+)" → 1 hit | | 4 | cancel-block SAFETY wording | PR #18 / `25854a1` | grep "caller_table.*shared.*reborrow" → 1 hit | | 5 | UNSAFE-2026-0014 SHA back-fill (c30f4ee, 7a402cb) | PR #18 / `94a6c0f` | grep both SHAs in unsafe-log.md → 2 hits each | | 6 | unsafe-policy.md §3 mechanical-edit exemption | PR #18 / `94a6c0f` | grep "Mechanical-edit exemption" → 1 hit | | 7 | ADR-0026 §Simulation chronology rider | PR #18 / `94a6c0f` | grep "§Simulation rule was retro-extracted" → 1 hit | | 8 | master-plan AC cross-reference | PR #18 / `94a6c0f` | grep "Closure-trio coordination cross-reference" in security + perf master-plans → 1 hit each | | 9 | ADR-0026 §skill-clause reconciliation rider | PR #18 / `94a6c0f` | grep "single-commit Propose+Accept landing reconciliation" → 1 hit | | 11 | P10 wall-clock harness | this integration PR (replaces #19/#20/#21) | tools/perf-harness.sh exists; baseline report exists; band p10=3.884/p50=4.642/p90=5.584 ms | Forward-flagged (status unchanged): - Item 10: RecvWaiting waiter-identity gap — ADR-0030 / ADR-0019 venue - Item 12: cancel-on-cap-bearing-state destroy-drain ADR — first userspace-destroy task venue - Item 13: B5+ preemption-rollback re-validation of ADR-0032 — B5+ preemption ADR venue This commit only touches the consolidated review's annotation; track files preserved as historical artefacts (their per-track verdicts are the snapshot at the moment of the review, not subject to back-edits). The review's per-item findings (Track-A NIT-2 SchedQueue::new doc rename; Track-G MIN-G1/G2/G3; Track-H MIN-1/MIN-2; Track-A MIN-2 ipc_cancel_recv doc-rider; Track-D D1; Track-F §F-1) are all closed in PR #18 + this integration PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the orchestrator review's findings on PR #22 (16 actionable items; one item — PR #19 description "180/180" wording — not closeable post-merge because the gh PR is already closed and its description is immutable from this branch). **3 Majors (fix-on-branch):** - M1 path-drift in 2026-05-08 consolidated review L124: `(../../decisions/...)` → `(../../../decisions/...)`. The 4-deep file needs 3 `..` to reach `docs/decisions/`. Other relative paths in the same file already use the correct depth; this was the lone broken outlier. - M2 `commit-style.md:130` anchor: the new "PR-number references" rider linked to `track-g-process.md#min-g3` for "PR-numbering fix-up precedent", but MIN-G3 is about closure-trio smoke-trace AC asymmetry, not PR-numbering. Dropped the wrong anchor; the link now points at the 2026-05-08 review's §"Cross-PR observations" §1 ("PR-numbering hygiene drift recurrence"), which IS the canonical source of the rider. - M3 158→159 narrative drift in `current.md:15` and `phase-b.md:103`: added "Current state (post-PR-#18 hygiene): 159/159" addendum that preserves the historical PR-#17-merge-moment claim of 158/158 (factually accurate at the time) while disambiguating from the current 159 baseline that PR #18's `25826a1` brought us to. **7 Minors (substance + governance polish):** - `hal.md` §Mmu present-tense overclaim: `MapperFlush` extension is T-016 deliverable, not current `main`; added a "tense hedge" blockquote making the post-T-016 framing explicit and acknowledging that the current trait surface is still pre-`MapperFlush`. - ADR-0027 §(a) MMIO range mismatch: `0x0902_0000` (the UART's actual end-of-MMIO) vs `0x0920_0000` (the 2 MiB-block-aligned end of the bootstrap's identity-map for that block). Reworded to explicitly cover the block-aligned superset; clarified the unmapped slack is harmless under device-nGnRnE. - ADR-0027 §(a) `TCR_EL1.A1` forward-flag: `A1=0` documented (selects which TTBR holds the ASID; v1 keeps `A1=0` because only `TTBR0_EL1` is active); the future high-half ADR (ADR-0033) gets named as the decider for `A1=1` migration. - `track-3-pr-20-governance.md:31` quoted-excerpt path: the embedded `(../../.claude/skills/write-adr/SKILL.md)` link in a quoted ADR-0027 passage was broken from track-3's depth (5-deep). Restructured the quote into a `> blockquote` with link targets stripped; preserves quote fidelity while avoiding broken-link rendering. - Glossary: added entries for `mmu_bootstrap` and `.boot_pt` — identifiers introduced by ADR-0027 / T-016 that future readers will hit and want a 1-line definition for. - `tools/perf-harness.sh` `export LC_ALL=C` at script entry: forces period-decimal + ASCII-digit awk output regardless of host locale. Without this, `LC_ALL=tr_TR.UTF-8` would emit `5,169` (decimal comma) in baseline reports — valid in tr_TR, mis-parsed in en_US. - 2026-05-08 consolidated review §Follow-up backlog: each of the 15 items now carries an inline ✅ + closing-commit-SHA annotation, matching the [2026-05-07 review's `8b6147d`-style closure-status pattern](docs/analysis/reviews/code-reviews/ 2026-05-07-pr-12-to-17-multi-axis-review.md). Item 8 (PR #19 description "180/180" wording) annotated as "not closeable post-merge" with rationale. **5 Nits (refinements; one was already closed):** - ADR-0027 §Simulation Step 1 row: collapsed the awkward `L2_low[64..72]` + `L2_low[72]` notation into the cleaner half-open `L2_low[64..73]` (= 9 indices = 9 entries) and added a footnote clarifying the half-open Rust convention used throughout the ADR. memory-management.md mermaid diagram updated similarly. - ADR-0027 §References framing: tightened "first ADR drafted under §Simulation; this is the second" to "first ADR drafted under the §Simulation rule (recovery-primitive subject); this ADR is the first non-recovery-primitive state-machine ADR drafted under the same rule" — matches the §Context para 2 framing precisely. - `tools/perf-harness.sh:223` (`QEMU_VERSION` extraction): replaced `head -n 1` with `awk 'NR==1 { print; exit }'` to avoid the `head` SIGPIPE that `set -o pipefail` would propagate. Defensive hardening; observable behaviour unchanged. - `tools/perf-harness.sh:209` watchdog kill: added `kill -0` liveness guard before `kill -KILL`. Before: kill on a dead PID returned ESRCH which `2>/dev/null || true` swallowed silently. After: explicit aliveness check makes the intent self-documenting. - `tools/perf-harness.sh:54-62` trap function: added a one-line comment documenting the idempotency property — both globals empty → no-op; stale PIDs → ESRCH suppressed → safe to fire multiple times. - (Already closed) `unsafe-log.md` 2026-05-08 closure-path Amendments cite Track-H NIT-1: verified — all 3 Amendments (UNSAFE-2026-0019/0020/0021) already include the trace-back link to the prior review's NIT-1. Verification: cargo fmt clean, cargo host-test 159/159, host-clippy clean (-D warnings), kernel-clippy clean, kernel-build clean, perf-harness 3-iter sanity run produces consistent stats with `LC_ALL=C` decimal-period output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
db24d6d— separate diff for the careful-re-read pass perwrite-adrskill step 10, the first project-side application of that discipline as its own commit).ipc_cancel_recvrecovery primitive added inkernel/src/ipc/mod.rs; reverses anIdle → RecvWaitingtransition, no-op on any other state, RECV-rights validated, zero newunsafe(covered by UNSAFE-2026-0014's umbrella).ipc_recv_and_yield's Phase 2 Deadlock branch inkernel/src/sched/mod.rsnow callsipc_cancel_recvafter dropping its&mut Scheduler<C>and before returningErr(SchedError::Deadlock), so both scheduler and endpoint state restore to the caller's pre-call shape.tyrne: all tasks complete+boot-to-end elapsed = 4088000 ns. Byte-for-byte identical message sequence to the post-T-014 baseline (T-015 adds no v1-reachable code path; Deadlock is structurally unreachable withregister_idleinstalled per ADR-0026).Closes the last open B2-prep follow-on item from the B1 closure retro §Adjustments before B2's first userspace-driven endpoint destroy lands.
Diff shape
kernel/src/ipc/mod.rs,kernel/src/sched/mod.rsdocs/decisions/0032-...md,docs/decisions/README.mddocs/decisions/0017-...mdrider,docs/architecture/ipc.md,docs/audits/unsafe-log.md, T-015 + phase-b README + current.md + phase-b.md3 commits:
db24d6d(ADR Accept),7a402cb(kernel impl + tests),c258ee3(doc-side).Test plan
cargo fmt --all -- --check— cleancargo host-clippy— clean (-D warnings)cargo kernel-clippy— cleancargo host-test— 158/158 pass (incl. 5 new IPC + 1 new sched + extended T-007)cargo +nightly miri test— 158/158 passcargo kernel-build— clean./tools/run-qemu.sh— full demo trace; boot-to-end ~4.1 ms; byte-for-byte unchanged from post-T-014 baseline&mutsite (no new audit entry — covered under existing umbrella)What's next after this lands
B2 prep — ADR-0027 (kernel virtual memory layout) drafting. ADR-0027 is the first ADR drafted under the
write-adrskill §Simulation discipline codified after ADR-0026's caught-by-table experience: its Decision outcome must include a 3–5 row state-machine table walking the worst-case interaction (page-table walk / TTBR switch / mapping handshake) before the Accept can flip. Design-first applies —docs/architecture/memory-management.mdlands alongside or before the implementation task (T-016 or next free slot).🤖 Generated with Claude Code
Summary by Sourcery
Introduce a kernel-internal IPC cancel primitive and wire it into scheduler deadlock handling for symmetric rollback, while updating ADRs, roadmap docs, and unsafe-audit notes to reflect the new behaviour.
New Features:
Enhancements:
Tests: