diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ce9eb37d..b56c235f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -275,8 +275,7 @@ jobs: # absent from the sysroot entirely. The prebuilt-sysroot thumb job # above SHIPS alloc, so an `extern crate alloc` regression passes # there and still breaks halo. This job is the halo-certification - # gate (phase 22 / measurement PR 0 — see - # docs/simple_someip/plans/2026-06-09-phase22-125-memory-reduction-design.md). + # gate. name: build-std core gate (no alloc in sysroot) needs: check runs-on: ubuntu-latest diff --git a/docs/simple_someip/plans/2026-06-09-phase22-125-memory-reduction-design.md b/docs/simple_someip/plans/2026-06-09-phase22-125-memory-reduction-design.md deleted file mode 100644 index e336f18b..00000000 --- a/docs/simple_someip/plans/2026-06-09-phase22-125-memory-reduction-design.md +++ /dev/null @@ -1,274 +0,0 @@ -# Memory-footprint reduction: issue #125 + Phase 22 close-out - -**Date:** 2026-06-09 (rev 2 — post adversarial review, same day) -**Base:** `feature/phase21_api_symmetry` (PR #114), after PR #124 merges into it -**Closes:** issue #125; completes the Phase 22 server alloc-elimination plan - -## Problem - -Issue #125 reports Embassy arena exhaustion in the consuming firmware -build (halo / TC4): `TaskStorage` entries for simple_someip tasks -dominate the arena, and static pool symbols are oversized. Two root -causes: - -1. **Async state-machine bloat.** `Inner::run_future` is a single - `select_biased!` loop that inlines the entire - `handle_control_message` call tree (~370 lines, itself awaiting - `bind_unicast` → socket spawn → send → oneshot recv). Rustc reserves - layout for the sum of all nested awaited futures along the deepest - path. The same pattern exists in `socket_loop_future` and the - server's `recv_loop` / `announce_loop`. -2. **Buffers and pools held by value.** - - `socket_manager.rs:569` holds a `[u8; UDP_BUFFER_SIZE]` (1500 B) - live across the whole socket loop, and `socket_manager.rs:632` - adds a second 1500 B buffer during E2E sends. With - `UNICAST_SOCKETS_CAP = 8` (`inner.rs:40`), that is **~12 KiB - always-live and ~24 KiB worst case** during concurrent E2E sends - — all of it inside future state, i.e. inside the Embassy arena. - - Static channel pools holding `SendMessage` / `ReceivedMessage` - elements that embed full `Message

` payloads by value. The - `define_static_channels!` macro already lets consumers tune - **pool sizes** per type; what is hardcoded in crate code is the - bounded **slot caps** (`C::bounded::()` call sites), the - control queue `Deque<_, 32>`, and the pending-responses map (64). - -Separately, the Phase 22 plan (make `server,bare_metal` build under -halo's `-Zbuild-std=core`, i.e. no alloc in the sysroot) gates any -on-target measurement of the server loops. PR #124 (Feliciano) has -since implemented most of Phase 22 — **verified 2026-06-09**: on -#124's branch, both `--features server,bare_metal` and -`--features client,server,bare_metal` compile clean under -`cargo +nightly build -Zbuild-std=core --target thumbv7em-none-eabihf`. - -### Framing: arena vs. `.bss` - -Moving buffers out of futures does **not** reduce total RAM — it moves -bytes from the Embassy arena (TaskStorage) into consumer-declared -statics (`.bss`), and shrinks them where the consumer right-sizes the -declarations. That is the correct fix for #125's failure mode: the -arena is the thing that exhausts, and `.bss` is sized explicitly and -predictably. Reviewers of the before/after tables should expect -TaskStorage entries to shrink while some static symbols grow or move -to the consumer's crate; the headline win is arena predictability -plus whatever the consumer saves by right-sizing. - -## Decisions already made - -- **Measurement:** in-repo harness for development and regression - gating; the locally available TC4 build is the final acceptance - check once hooked up. On-target "before" numbers remain capturable - later by building the pre-optimization commit — baselines do not - block on TC4 bring-up. -- **Scope:** everything in #125 (client, server, pools), with Phase 22 - folded into the same stack. -- **Client restructuring aggressiveness:** moderate — buffer - extraction plus handler-tree flattening, staying in ordinary async - Rust. No hand-written poll state machines (the polled module from - PR #126 already serves users who need exact layouts). -- **Buffer sizing (rev 2):** buffers become **caller-sized**. Once - extracted from the futures, socket loops take `&'static mut [u8]` - slices, so the buffer count and length are chosen by the consumer's - static declaration at runtime-slice granularity — no const-generic - threading through `Client`'s parameters. Halo can declare e.g. - 2 × 512 B = 1 KiB instead of 12 KiB. The tokio path provisions - 8 × 1500 internally (API and behavior unchanged). - - *512 B rationale (sized against Iris generic interface 0.11):* the - largest defined payload is `SoftwareApplicationInfo` at 256 B - (≈284 B on-wire with SOME/IP + E2E P04 headers); `ScanCmd` is 88 B. - ScanCmd's command list is the growth risk (`uint16` length field), - but the crate's hard ceiling is already one UDP datagram - (`UDP_BUFFER_SIZE = 1500`, no SOME/IP-TP), so nothing larger was - ever sendable; outgrowing 512 B is a logged drop fixed by a - one-line bump in the consumer's declaration. Any halo-side traffic - beyond the generic interface (e.g. HWP1 method requests) needs the - same size check before the declaration is locked. -- **Pool capacities:** narrowed from "promote everything to - const-generic knobs". Pool sizes are already consumer-tunable via - `define_static_channels!`. The remaining hardcoded numbers (slot - caps 16, `Deque<_, 32>`, pending map 64) can only become knobs on - stable Rust as **literal const parameters threaded through - `Client`/`Inner`'s public types** (associated-const capacities at - the call sites would require unstable `generic_const_exprs`). That - churn is taken only where PR 0/TC4 measurement shows the win - justifies it; otherwise the numbers stay hardcoded and the decision - is recorded. -- **PR #124:** merges as-is; our review findings are fixed by us in - this stack rather than requested from the author. -- **Stack hygiene:** all 37 stale phase PRs beneath #114 were closed - without merging on 2026-06-09 (branches retained). - -## PR #124 coverage of Phase 22 (reviewed 2026-06-09) - -| Phase 22 item | Status in #124 | -|---|---| -| Item 4 — `_alloc`-gate `Server::run` | Done (`run` + `run_inner`) | -| Item 5 — remove `Pin>` GATs | Done via `core::future::Ready` (simpler than the planned hand-written futures; supersedes the saved pre-flight patch) | -| Item 2 — started latch without `Arc` | Done via cfg-switched `StartedLatch` alias instead of an `Hstart` generic | -| Item 3 — Arc type-param defaults | Done via cfg-switched `Default*Handle` aliases instead of dropping defaults | -| Items 1+10 — import reshape, `server` feature drops `_alloc` | Done | -| CI gate `server,bare_metal -Zbuild-std=core` | **Missing** (gap-filled in PR 0; verified locally that it passes) | - -**Why the existing CI doesn't already cover this:** phase21's CI does -build `server,bare_metal` for thumbv7em-none-eabihf — but against the -**prebuilt sysroot, which ships `alloc`**, so an `extern crate alloc` -regression would never E0463 there. Halo's proxy builds with -`-Zbuild-std=core`, where `alloc` is absent from the sysroot entirely. -PR 0's build-std job is the only configuration that certifies halo's -actual constraint. Relatedly, the existing `nm` alloc-symbol audit -covers only the `client,bare_metal` rlib; PR 0 extends it to -`server,bare_metal` (stable-toolchain, nearly free). - -**Accepted trade-off:** the cfg-switched aliases violate strict feature -additivity (enabling `_alloc` changes type identities). This is -documented as a hazard rather than redesigned — halo builds with a -fixed feature set, and explicit `new_with_handles` callers spell their -types. The generic-parameter design remains available if a real -unification break ever appears. - -**Pre-flight note:** the Phase 22 plan's open risk — whether a concrete -(non-boxed) future satisfies `Server::run`'s phase-21F -`for<'a> Sub::SubscribeFuture<'a>: Send` HRTB — was verified resolved -on 2026-06-09 against phase21 tip `892cb5b`. `core::future::Ready` -satisfies the same bounds. - -## The stack - -Four PRs off `feature/phase21_api_symmetry`, post-#124. - -### PR 0 — Measurement harness + CI gates - -- **Future-size regression tests:** `size_of_val`-based assertions on - client `run_future`, `socket_loop_future`, and the server run - future, in both tokio and bare-metal-deps configurations (modeled on - the existing `client_new_run_future_is_send_static` witness, which - already returns the run future by value). These are **host-arch - proxies**: x86_64 layouts differ from thumbv7 (pointer width, - alignment), so budgets are generous regression tripwires, not - targets. Budgets start at current size (recording the baseline) and - tighten as PRs 2–3 land. -- **`-Z print-type-sizes` capture script** in `tools/`, producing the - TaskStorage-style table for a thumbv7em build. This is the - **authoritative** size number. Baseline committed. -- **New CI jobs:** `--no-default-features --features server,bare_metal` - and `client,server,bare_metal` under `-Zbuild-std=core` for - thumbv7em (nightly + rust-src — new CI infrastructure; the current - workflows are stable-only). Verified passing locally on #124's - branch. Plus the `nm` alloc-symbol audit extended to the server - rlib. - -### PR 1 — #124 follow-ups (small, lands the breaking change early) - -The #124 review findings, fixed by us: - -- (a) Document (or deliberately change) the eager-vs-lazy semantics of - the `Ready`-based `subscribe`/`unsubscribe` — the locked mutation now - happens at future construction, not first poll. -- (b) `NonSdRequestCallback` gains a context argument. **Design note:** - a stored `*mut c_void` would make `Server` `!Send` and break - `Server::run`'s declared `+ Send` bound. The shape is decided in the - implementation plan from: `ctx: usize` (caller casts), a newtype - with a documented `unsafe impl Send`, or a generic observer - parameter. Breaking now is free (nothing published); breaking later - is not. Flag to Feliciano before this lands so no further FFI builds - on the bare `fn` shape. -- (c) Record the shared-socket-topology rationale for - `announce_only_future` (it partially reintroduces the split-future - shape phase 21 removed). The originally-planned MSRV check is moot: - the crate is edition 2024 (requires Rust ≥ 1.85); `use<>` precise - capture needs only 1.82. -- (d) Strengthen the non-SD-observer negative test (currently cannot - fail — the witness callback is never registered). - -### PR 2 — #125 client async-state reduction - -- **Buffer extraction via a claim/release buffer pool.** Socket loops - are spawned dynamically per bind/unbind (up to - `UNICAST_SOCKETS_CAP` live), so buffers need checkout/return - semantics: a buffer pool in the consumer's static storage (same - shape as `OneshotPool` — claim on bind, release on unbind, no - `&'static mut` aliasing). Loops take `&'static mut [u8]` slices; - count and length are the consumer's choice (halo: ~1 KiB total). - The tokio path provisions 8 × 1500 internally — API unchanged. - Defined behavior changes: an inbound datagram larger than the - claimed buffer is dropped with a log; the existing oversize-send - rejection (`socket_manager.rs:447`) checks `buf.len()` instead of - `UDP_BUFFER_SIZE`. The E2E `protected` buffer gets the same pool - treatment, or is restructured to not be live across the - `protect().await` point — whichever measures better. -- **Handler-tree flattening:** `handle_control_message` splits into a - synchronous "decode + decide" section returning a small action - value; the actual awaits are hoisted to shallow helpers at - `run_future`'s top level. **Expectation setting:** the awaited - futures remain part of `run_future`'s layout — the wins are locals - no longer held across awaits, avoided per-nesting-level argument - duplication, and better variant overlap. The buffers are expected to - be the dominant win; flattening is secondary and is kept only where - PR 0's numbers move. -- Doc debt: rewrite `src/client/mod.rs:12-30` (describes the old - buffer-in-future architecture and the 12 KiB math). -- Every change is validated against PR 0's numbers; changes that don't - move the measurement are dropped, not merged on faith. -- **Deferred follow-up (recorded, not scheduled):** a readiness-split - receive (`await readiness, then synchronous copy-out`) would let one - shared buffer serve all socket loops on a single-threaded executor - (~1.5 KiB total regardless of socket count). It requires a - `TransportSocket` trait change; only worth it if caller-sizing - proves insufficient. - -### PR 3 — #125 server + pools, final numbers - -- Same flatten/extract treatment for `recv_loop`, `announce_loop`, - `send_subscribe_nack_from_view`, on the post-#124 code. -- Pool-capacity knobs per the narrowed decision above (literal const - params only where measurement justifies the churn; otherwise record - and keep). -- Final before/after tables (TaskStorage sizes + `llvm-nm` pool - symbols) in the PR description — issue #125's acceptance criteria. - -### Scope cuts from issue #125 (recorded) - -- **SD encode monomorphization** (`Header::encode`, - `ServiceEntry::encode`, `EventGroupEntry::encode`, generic over - `embedded_io::Write`): code-size pressure, not arena/RAM pressure. - Out of scope for the arena-exhaustion failure mode. If flash size - becomes the constraint, the cheap fix is an inner non-generic - `&mut dyn embedded_io::Write` function — separate issue. -- **`unbind_discovery`:** addressed only implicitly via the - `run_future` flattening in PR 2 (it is one arm of the same control - path); no dedicated work item unless PR 0's table shows it as an - independent hotspot. - -## Invariants - -- No behavioral changes except the agreed `NonSdRequestCallback` - signature change and the two defined buffer-size behaviors in PR 2. -- Existing suite (~543 tests as of #114, plus #124's additions) green - on every PR; embassy-net loopback live-wire test guards the announce - path. -- No nightly-only features in the crate itself (halo's consumer is - nightly; the crate stays stable; CI may use nightly for measurement - and build-std jobs). -- Wire format untouched. - -## Risks / coordination - -- **#126 (polled module)** is Feliciano's, has an outstanding - hold-merge punch list, and also bases on phase21. Merge order - relative to this stack is decided between Justin and Feliciano; the - polled module is a parallel surface, so PRs 2–3 should rebase - trivially either way. -- **#124 is force-pushed actively.** Our stack starts only after it - merges into phase21, to avoid chasing a moving base. -- **Future-size assertions can be brittle across rustc versions** and - are host-arch proxies. Budgets use generous headroom (e.g. +25%) - over the post-optimization baseline; the thumbv7em - `print-type-sizes` harness is the authoritative number. -- **The buffer pool's claim/release lifecycle** is new unsafe-adjacent - surface (handing out `&'static mut [u8]`); the implementation plan - includes loom-style or witness tests for double-claim and - release-on-unbind. -- The whole stack still sits on the unmerged #114 tower - (134+ commits ahead of main); the eventual consolidation rebase is a - known cost of the established workflow, accepted to keep reviewable - PR boundaries. diff --git a/docs/simple_someip/plans/2026-06-10-pr1-124-followups-design.md b/docs/simple_someip/plans/2026-06-10-pr1-124-followups-design.md deleted file mode 100644 index cda70430..00000000 --- a/docs/simple_someip/plans/2026-06-10-pr1-124-followups-design.md +++ /dev/null @@ -1,162 +0,0 @@ -# PR 1 — #124 follow-ups: design - -Second PR of the #125 / phase-22 close-out stack -(`2026-06-09-phase22-125-memory-reduction-design.md`, "PR 1" section). -Closes the four review findings recorded there against PR #124, plus -one stale-doc item surfaced during PR 0. - -**Branch:** `feature/pr1_124_followups` off -`feature/pr0_measurement_harness` (PR base = the PR 0 branch). PR 1 -must stack on PR 0: changing the callback shape touches `ServerDeps` -initializers in test files PR 0 modified -(`tests/bare_metal_e2e.rs`, `tests/bare_metal_server.rs`). - -**External gate:** Feliciano gets the callback-signature heads-up -before this merges, so no further halo FFI builds on the bare `fn` -shape (stack-plan gate 2 — user action, still open as of -2026-06-10). - -## 1. `NonSdRequestCallback` gains a context argument (BREAKING) - -Decision (2026-06-10): **`ctx: usize`**, over an unsafe-Send -`*mut c_void` newtype and over a generic observer type parameter. - -Revised 2026-06-11: a parallel branch (`feat/embassy-mem-channel-cap`) -independently reshaped the callback to a library-parsed -`(service_id, method_id, payload, e2e_status)` contract. The -reconciled contract is the union of both — ctx + source + decoded -fields — keeping parse/E2E in the audited crate (MISRA/ASIL) rather -than in N C consumers. `e2e_status` is 0 (unchecked) on the server -path today; `source` is future-proofing (unused by halo and dft). - -```rust -pub type NonSdRequestCallback = fn( - ctx: usize, - source: core::net::SocketAddrV4, - service_id: u16, - method_id: u16, - payload: &[u8], - e2e_status: u8, -); -``` - -- Storage everywhere becomes `Option<(NonSdRequestCallback, usize)>` - — a plain tuple, no wrapper struct: the `Server` field, - `ServerDeps.non_sd_observer`, and - `ServerDeps::with_non_sd_observer`. `recv_loop` threads the pair - down and invokes `cb(ctx, data, src)`. -- Rationale (recorded on the type alias's doc comment): a stored - `*mut c_void` makes `Server` `!Send` and breaks `Server::run`'s - declared `+ Send` bound; `usize` is trivially `Send + Sync`, - keeps the field `Copy`, and matches the `uintptr_t` the C caller - holds anyway. halo passes `(dispatch, state_ptr as usize)`; - Rust-native users pass `(f, 0)`. -- **No `unsafe` enters this crate.** The library stores, copies, and - passes back a plain integer through a safe `fn`-pointer call — - `Server: Send` holds by construction, with no `unsafe impl` and no - soundness contract for the library to document or uphold. The - unsafe dereference (`ctx as *mut T`, then `unsafe { &*ptr }`) - happens in the consumer's callback body — halo's FFI dispatch - code, which is already unsafe territory and is the only party - that can verify the pointee's lifetime and thread-safety. Known - trade-off: `usize` carries no provenance, so the compiler can't - stop a caller passing a wrong address — but the rejected - `*mut c_void` newtype was equally untyped; only the - generic-observer design would have fixed that, at the - 8th-type-parameter cost. -- Rejected alternatives, for the record: the unsafe-Send newtype - moves an unverifiable soundness contract into this library; the - generic observer adds an 8th type parameter that ripples through - `ServerDeps`/`ServerHandles`/both cfg-switched alias families, - and halo's FFI would still write its own unsafe-Send wrapper. -- Breaking now is free: 0.8.0 is unpublished and halo is the only - consumer. CHANGELOG gets a breaking-change entry with the - before/after signature. - -## 2. Eager-`Ready` timing documented (no code change) - -Decision (2026-06-10): document, don't make lazy. -`StaticSubscriptionHandle::subscribe`/`unsubscribe` execute the -locked mutation when the future is *constructed* (inside -`core::future::ready(...)`), unlike the `Box::pin(async)` impls, -which are lazy. The only in-tree caller (`runtime.rs`) awaits -immediately, so laziness would be ~80 lines of poll boilerplate for -behavior no current caller observes. - -- Trait-level note on `SubscriptionHandle::subscribe`/`unsubscribe`: - implementations with a fully synchronous critical section may - perform the mutation at future construction; callers must not - assume construction is side-effect-free. -- Matching sentence on the `StaticSubscriptionHandle` impl block. -- The phase-22 preflight patch's hand-written `StaticSubscribeFuture` - (`.claude/phase22_item5_preflight.patch` in the main worktree) is - permanently obsolete. - -## 3. `announce_only_future` rationale (doc-only) - -The method's doc already explains the shared-socket topology -(supplementary Servers via `new_with_handles` announce on the shared -SD socket; the primary owns all inbound loops). It gains the honest -acknowledgment that this partially reintroduces the split-future -shape phase 21 removed, and why that is acceptable here: an -announce-only future never touches the recv path, so the -single-run-future invariant that motivated phase 21b (no two futures -racing on the same sockets/session counter) is preserved — the -`started` latch still guards the full run-future. - -The originally-planned MSRV check is recorded as moot: the crate is -edition 2024 (Rust ≥ 1.85); `use<>` precise capture needs only 1.82. - -## 4. Strengthen the non-SD-observer negative test - -`non_sd_observer_none_preserves_ignore_behavior` -(`tests/bare_metal_server.rs`) cannot currently fail: `record_none` -is never wired into the server, so no code path can populate -`OBSERVED_NONE` regardless of any routing regression. - -Replace with negative tests that have a live witness — register the -recording callback as a real observer, then assert it does NOT fire -for: - -- (i) an SD unicast datagram (exercises the SD-vs-non-SD routing - branch), and -- (ii) a non-SD **multicast** datagram (exercises the - unicast-vs-multicast branch — the mock needs to mark the datagram - as arriving on the SD/multicast socket; mechanics resolved in the - implementation plan). - -The `None` case shrinks to what it actually proves: the run loop -processes a non-SD unicast datagram without panicking when no -observer is registered. The positive test -(`non_sd_observer_some_receives_unicast_method_request`) is updated -for the new signature and asserts the ctx value round-trips. - -## 5. Stale `UDP_BUFFER_SIZE` rustdoc - -Verified false during PR 0 review (2026-06-10): `send_ack` -(`src/server/runtime.rs`) builds into a stack -`[0u8; crate::UDP_BUFFER_SIZE]`, and the `server,bare_metal` rlib -audits to zero allocator symbols. The Vec-to-stack conversion was -the phase-21 per-event-allocation cleanup (`7c58649`, PR #114 -stack), not #124 — the rustdoc claim was stale even before #124, -which never touched those paths. The constant's rustdoc paragraph -claiming announcement builders / `SubscribeAck`/`Nack` "still use -heap `Vec` buffers — known gap" is rewritten: all outbound SD paths -are stack-buffered and capped by `UDP_BUFFER_SIZE`. - -## Error handling - -No new fallible paths. The callback invocation remains -fire-and-forget from `recv_loop` (a misbehaving callback is the -consumer's responsibility — same contract as today, restated in the -type-alias docs). - -## Verification - -fmt, clippy `--workspace --all-features` + `--no-default-features` -(both `-D warnings -D clippy::pedantic`), full suite at -`--test-threads=1`, doc tests, the three `-Zbuild-std=core` thumb -builds, and the `nm` server audit — the latter two also enforced in -CI since PR 0. Future-size witnesses from PR 0 must stay within -budget (the tuple adds 2×usize to the run-future capture; budgets -have 25% headroom). diff --git a/docs/simple_someip/plans/2026-06-10-pr1-124-followups.md b/docs/simple_someip/plans/2026-06-10-pr1-124-followups.md deleted file mode 100644 index 8a42ed2f..00000000 --- a/docs/simple_someip/plans/2026-06-10-pr1-124-followups.md +++ /dev/null @@ -1,869 +0,0 @@ -# PR 1 — #124 Follow-ups Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Land the four #124 review follow-ups — ctx-carrying `NonSdRequestCallback` (breaking), eager-`Ready` timing docs, `announce_only_future` rationale, live-witness negative tests — plus the stale `UDP_BUFFER_SIZE` rustdoc fix. - -**Architecture:** One breaking signature change (`fn(data, src)` → `fn(ctx: usize, data, src)`, stored as `Option<(NonSdRequestCallback, usize)>` on `ServerDeps`/`ServerHandles`/`Server` and threaded through `recv_loop`); a mock-transport split in `tests/bare_metal_server.rs` (per-socket pipes routed by `multicast_if_v4`, making `from_unicast` deterministic); the rest is documentation and CHANGELOG. - -**Tech Stack:** Rust (stable; nightly only for the final `-Zbuild-std=core` verification), cargo, GNU `nm`. - -**Spec:** `docs/simple_someip/plans/2026-06-10-pr1-124-followups-design.md` - -**Working tree:** the `simple_someip-pr0` worktree, branch `feature/pr1_124_followups` (created off `feature/pr0_measurement_harness` at the design commit). PR base = `feature/pr0_measurement_harness`. - ---- - -### Task 1: Verify preconditions - -**Files:** none (git only) - -- [ ] **Step 1: Confirm branch and clean tree** - -Run: `git branch --show-current && git status --short` -Expected: `feature/pr1_124_followups`, no output from status (clean tree; the design doc commit `da3e02c` or later is HEAD). - -- [ ] **Step 2: Confirm the baseline is green** - -Run: `cargo test --features server-tokio,client-tokio,bare_metal --tests --lib -- --test-threads=1 2>&1 | grep -E "^test result" | grep -v "0 failed" || echo ALL_GREEN` -Expected: `ALL_GREEN` - ---- - -### Task 2: ctx argument on `NonSdRequestCallback` (breaking) - -Red first: rewrite the test-side callbacks and the positive test to the NEW signature (compile failure is the failing test), then change the library, then green. - -**Files:** -- Modify: `tests/bare_metal_server.rs:365-385` (statics + record fns), `:416-492` (positive test) -- Modify: `src/server/mod.rs:645` (type alias), `:285-293` (ServerDeps field), `:409-417` (builder), `:519-521` (ServerHandles field), `:629-634` (Server field) -- Modify: `src/server/runtime.rs:443`, `:588` (params), `:543-545` (invocation) - -- [ ] **Step 1: Update the test-side statics and record fns to the new shape** - -Replace the two statics and two fns at `tests/bare_metal_server.rs:367-379` with: - -```rust -static OBSERVED_SOME: OnceLock, SocketAddrV4)>>> = OnceLock::new(); - -fn record_some(ctx: usize, data: &[u8], source: SocketAddrV4) { - let slot = OBSERVED_SOME.get_or_init(|| Mutex::new(None)); - *slot.lock().unwrap() = Some((ctx, data.to_vec(), source)); -} -``` - -(`OBSERVED_NONE` / `record_none` are deleted here; Task 4 replaces the test that used them. If the file temporarily fails to compile because the old negative test still references them, stub the references by deleting the body of `non_sd_observer_none_preserves_ignore_behavior` down to `let _ = ();` — Task 4 rewrites it entirely.) - -- [ ] **Step 2: Update the positive test to the new signature + ctx round-trip** - -In `non_sd_observer_some_receives_unicast_method_request`: - -```rust - non_sd_observer: Some((record_some as NonSdRequestCallback, 0xC0FF_EE00)), -``` - -and replace the result-destructuring + asserts at the end with: - -```rust - let (got_ctx, got_data, got_src) = OBSERVED_SOME - .get() - .unwrap() - .lock() - .unwrap() - .clone() - .expect("callback fired"); - assert_eq!( - got_ctx, 0xC0FF_EE00, - "callback must receive the registered ctx word verbatim" - ); - assert_eq!( - got_data, payload, - "callback must receive the full raw datagram bytes" - ); - assert_eq!(got_src, src, "callback must receive the original source"); -``` - -- [ ] **Step 3: Verify it fails to compile (red)** - -Run: `cargo test --no-run --features server,bare_metal --test bare_metal_server 2>&1 | tail -5` -Expected: FAIL — type mismatch (`Option` vs the tuple) — the library hasn't changed yet. - -- [ ] **Step 4: Change the type alias** - -`src/server/mod.rs:636-645` — replace the alias and its doc: - -```rust -/// Callback invoked by the server's `recv_loop` for every non-SD -/// unicast datagram received on the service's port (i.e. method -/// requests / fire-and-forget calls to the offered services). The -/// payload is the full raw datagram bytes; the caller is responsible -/// for re-parsing the SOME/IP header (and applying any E2E check) on -/// the consumer side. -/// -/// `ctx` is an opaque caller-owned context word, registered alongside -/// the callback as a `(NonSdRequestCallback, usize)` pair and passed -/// back verbatim on every invocation. It is deliberately `usize` -/// rather than `*mut c_void`: a stored raw pointer would make -/// [`Server`] `!Send` and break [`Server::run`]'s declared `+ Send` -/// bound, while `usize` is trivially `Send + Sync` and matches the -/// `uintptr_t` an FFI caller holds anyway. No `unsafe` enters this -/// crate — the cast back to a pointer (and its safety justification) -/// lives in the consumer's callback body, the only place that knows -/// the pointee's lifetime and thread-safety. Rust-native users that -/// need no context pass `0`. `fn` pointers are -/// `Copy + Send + Sync + 'static`, so the pair can be stored on the -/// `Server` and captured by the run-future without adding a new -/// generic. -pub type NonSdRequestCallback = fn(ctx: usize, data: &[u8], source: core::net::SocketAddrV4); -``` - -- [ ] **Step 5: Change the three struct fields and the builder** - -`src/server/mod.rs:287-293` (`ServerDeps` field — replace doc + type): - -```rust - /// Optional `(callback, ctx)` pair invoked from the server's receive - /// loop for every non-SD **unicast** datagram (method requests / - /// fire-and-forget calls to offered services). `None` reproduces the - /// historical "non-SD ignored" behavior. The callback receives the - /// opaque `ctx` word back verbatim, plus the full raw datagram bytes - /// and the source `SocketAddrV4`; the consumer is responsible for - /// re-parsing the SOME/IP header and any E2E check. - pub non_sd_observer: Option<(NonSdRequestCallback, usize)>, -``` - -`src/server/mod.rs:409-417` (`ServerDeps::with_non_sd_observer`): - -```rust - /// Register a `(callback, ctx)` pair invoked for every non-SD unicast - /// datagram (method requests / fire-and-forget calls to offered - /// services). The opaque `ctx` word is passed back verbatim on every - /// invocation — FFI callers stash a pointer here as `usize`; - /// pure-Rust callers that need no context pass `0`. Passing `None` - /// (the default if unset) preserves the historical "ignore non-SD" - /// behavior. - #[must_use] - pub fn with_non_sd_observer( - mut self, - observer: Option<(NonSdRequestCallback, usize)>, - ) -> Self { - self.non_sd_observer = observer; - self - } -``` - -`src/server/mod.rs:519-521` (`ServerHandles` field): - -```rust - /// Optional `(callback, ctx)` pair for non-SD unicast datagrams - /// (method requests). `None` reproduces the default "non-SD - /// ignored" behavior. - pub non_sd_observer: Option<(NonSdRequestCallback, usize)>, -``` - -`src/server/mod.rs:629-634` (`Server` field — keep the existing doc sentence about halo's HWP1 dispatch, change the type): - -```rust - non_sd_observer: Option<(NonSdRequestCallback, usize)>, -``` - -All `non_sd_observer: None` initializers and the struct-update copies (`non_sd_observer: self.non_sd_observer`, `non_sd_observer: deps_non_sd_observer`, …) compile unchanged — do not touch them. - -- [ ] **Step 6: Thread the pair through `recv_loop`** - -`src/server/runtime.rs:443` and `:588` — both parameter declarations become: - -```rust - non_sd_observer: Option<(super::NonSdRequestCallback, usize)>, -``` - -`src/server/runtime.rs:543-545` — the invocation becomes: - -```rust - if let Some((cb, ctx)) = non_sd_observer { - if let core::net::SocketAddr::V4(src_v4) = addr { - cb(ctx, data, src_v4); - } -``` - -- [ ] **Step 7: Verify green** - -Run: `cargo test --features server-tokio,client-tokio,bare_metal --tests --lib -- --test-threads=1 2>&1 | grep -E "^test result"` -Expected: all `ok`, 0 failed (the gutted negative test passes vacuously until Task 4). - -- [ ] **Step 8: Commit** - -```bash -git add src/server/mod.rs src/server/runtime.rs tests/bare_metal_server.rs -git commit -m "feat(server)!: NonSdRequestCallback gains an opaque ctx:usize argument - -Stored as (callback, ctx) on ServerDeps/ServerHandles/Server and -passed back verbatim from recv_loop. usize over *mut c_void keeps -Server: Send (run's declared + Send bound survives) with no unsafe -in this crate; the pointer cast lives in the consumer's callback. - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 3: Per-socket mock pipes in `tests/bare_metal_server.rs` - -Both mock sockets currently pop the SAME `inbound` queue, so which select -arm wins (and therefore `from_unicast`) depends on the alternating -`select_biased!` bias — latent nondeterminism, and a blocker for Task 4's -multicast test. Route by `SocketOptions`: the server binds its SD socket -with `multicast_if_v4 = Some(..)` (`src/server/mod.rs:880`), the unicast -socket with plain `SocketOptions::new()`. - -**Files:** -- Modify: `tests/bare_metal_server.rs:53-77` (factory), all four test constructors - -- [ ] **Step 1: Split the factory** - -Replace `MockFactory` and its `bind`: - -```rust -#[derive(Clone)] -struct MockFactory { - /// Handed to sockets bound WITHOUT multicast options — the - /// server's unicast service socket. - unicast_pipe: Arc, - /// Handed to sockets bound WITH `multicast_if_v4` set — the - /// server's SD socket. Per-socket queues make `recv_loop`'s - /// `from_unicast` flag deterministic: with a single shared queue, - /// whichever select arm polled first stole the datagram, so - /// routing depended on the alternating `select_biased!` bias. - sd_pipe: Arc, - next_port: Arc>, -} - -impl TransportFactory for MockFactory { - type Socket = MockSocket; - type BindFuture<'a> = - core::pin::Pin> + Send + 'a>>; - fn bind<'a>(&'a self, addr: SocketAddrV4, options: &'a SocketOptions) -> Self::BindFuture<'a> { - let pipe = if options.multicast_if_v4.is_some() { - Arc::clone(&self.sd_pipe) - } else { - Arc::clone(&self.unicast_pipe) - }; - // Mock: assign port deterministically. If caller asked for 0, - // hand out an incrementing fake ephemeral port. - let port = if addr.port() == 0 { - let mut p = self.next_port.lock().unwrap(); - let next = *p + 1; - *p = next; - 40000 + next - } else { - addr.port() - }; - let local = SocketAddrV4::new(*addr.ip(), port); - Box::pin(async move { Ok(MockSocket { pipe, local }) }) - } -} -``` - -- [ ] **Step 2: Update every factory construction site** - -In all four tests (`server_constructible_without_server_tokio_feature`, -`passive_server_constructible_without_server_tokio_feature`, -`non_sd_observer_some_receives_unicast_method_request`, and the gutted -negative test), replace: - -```rust - let pipe = Arc::new(MockPipe::default()); - let factory = MockFactory { - pipe: Arc::clone(&pipe), - next_port: Arc::new(Mutex::new(0)), - }; -``` - -with: - -```rust - let unicast_pipe = Arc::new(MockPipe::default()); - let sd_pipe = Arc::new(MockPipe::default()); - let factory = MockFactory { - unicast_pipe: Arc::clone(&unicast_pipe), - sd_pipe: Arc::clone(&sd_pipe), - next_port: Arc::new(Mutex::new(0)), - }; -``` - -In the positive test, the datagram pushes change `pipe.` → `unicast_pipe.` -(both the `inbound` push and the `inbound_waker` wake), and DELETE the -now-false comment block above the push ("Queue a non-SD unicast … relying -on the `select_biased!`'s prefer-unicast tick…") — replace it with: - -```rust - // Queue a non-SD unicast method-request datagram on the unicast - // socket's own pipe; per-socket pipes make `from_unicast = true` - // deterministic. -``` - -- [ ] **Step 3: Verify green** - -Run: `cargo test --features server,bare_metal --test bare_metal_server -- --test-threads=1 2>&1 | grep -E "^test result"` -Expected: `ok`, 0 failed. - -- [ ] **Step 4: Commit** - -```bash -git add tests/bare_metal_server.rs -git commit -m "test(server): per-socket mock pipes routed by multicast_if_v4 - -Removes the shared-queue nondeterminism (from_unicast depended on -select_biased bias order) and enables deterministic SD-socket -injection for the negative observer tests. - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 4: Live-witness negative observer tests - -The old `non_sd_observer_none_preserves_ignore_behavior` could not fail: -its witness callback was never registered, so no routing regression could -populate `OBSERVED_NONE`. Replace it with two tests whose observer IS -registered and must NOT fire, plus a slim no-panic `None` case. - -**Files:** -- Modify: `tests/bare_metal_server.rs` (new helper + statics, replace the negative test) - -- [ ] **Step 1: Add the SD datagram builder next to `build_method_request`** - -```rust -/// Build a minimal, well-formed SOME/IP-SD datagram: SD message id -/// (0xFFFF / 0x8100), then an SD payload with flags + reserved and -/// ZERO entries / options. Routing-wise a legitimate (if vacuous) SD -/// message — `recv_loop` must hand it to SD handling, never to the -/// non-SD observer, regardless of which socket it arrived on. -fn build_sd_message() -> Vec { - let mut buf = Vec::with_capacity(28); - buf.extend_from_slice(&0xFFFFu16.to_be_bytes()); // message_id (high): SD service - buf.extend_from_slice(&0x8100u16.to_be_bytes()); // message_id (low): SD method - buf.extend_from_slice(&20u32.to_be_bytes()); // length = header(8) + sd payload(12) - buf.extend_from_slice(&0u32.to_be_bytes()); // request_id - buf.push(1); // protocol_version - buf.push(1); // interface_version - buf.push(2); // message_type = Notification (0x02) - buf.push(0); // return_code = OK - buf.push(0x80); // SD flags: reboot - buf.extend_from_slice(&[0, 0, 0]); // reserved - buf.extend_from_slice(&0u32.to_be_bytes()); // entries array length = 0 - buf.extend_from_slice(&0u32.to_be_bytes()); // options array length = 0 - buf -} -``` - -- [ ] **Step 2: Add per-test witness statics + record fns (next to `OBSERVED_SOME`)** - -Separate statics per test — they share the process under parallel `cargo test`. - -```rust -static OBSERVED_SD_UNICAST: OnceLock, SocketAddrV4)>>> = - OnceLock::new(); -static OBSERVED_MULTICAST: OnceLock, SocketAddrV4)>>> = - OnceLock::new(); - -fn record_sd_unicast(ctx: usize, data: &[u8], source: SocketAddrV4) { - let slot = OBSERVED_SD_UNICAST.get_or_init(|| Mutex::new(None)); - *slot.lock().unwrap() = Some((ctx, data.to_vec(), source)); -} - -fn record_multicast(ctx: usize, data: &[u8], source: SocketAddrV4) { - let slot = OBSERVED_MULTICAST.get_or_init(|| Mutex::new(None)); - *slot.lock().unwrap() = Some((ctx, data.to_vec(), source)); -} -``` - -- [ ] **Step 3: Replace the gutted negative test with three tests** - -```rust -/// A registered observer must NOT fire for an SD message arriving on -/// the unicast socket — SD-formatted unicast traffic (e.g. unicast -/// FindService) routes to SD handling. Unlike the pre-PR-1 negative -/// test, the witness callback IS registered, so a routing regression -/// (SD datagrams leaking to the observer) trips the assertion. -#[tokio::test] -async fn non_sd_observer_ignores_sd_message_on_unicast_socket() { - let unicast_pipe = Arc::new(MockPipe::default()); - let sd_pipe = Arc::new(MockPipe::default()); - let factory = MockFactory { - unicast_pipe: Arc::clone(&unicast_pipe), - sd_pipe: Arc::clone(&sd_pipe), - next_port: Arc::new(Mutex::new(0)), - }; - - let e2e_handle: Arc> = Arc::new(Mutex::new(E2ERegistry::new())); - let config = ServerConfig::new(0x1234, 1) - .with_interface(Ipv4Addr::LOCALHOST) - .with_local_port(30702); - - let deps: ServerDeps>, MockSubscriptions> = - ServerDeps { - factory, - timer: MockTimer, - e2e_registry: e2e_handle, - subscriptions: MockSubscriptions::default(), - non_sd_observer: Some((record_sd_unicast as NonSdRequestCallback, 7)), - }; - - let (_server, _handles, run): ( - Server>, MockSubscriptions>, - _, - _, - ) = Server::new_with_deps(deps, config, false) - .await - .expect("Server::new_with_deps must succeed"); - let handle = tokio::spawn(run); - - let src = SocketAddrV4::new(Ipv4Addr::new(192, 0, 2, 102), 40002); - unicast_pipe - .inbound - .lock() - .unwrap() - .push_back((build_sd_message(), src)); - if let Some(w) = unicast_pipe.inbound_waker.lock().unwrap().take() { - w.wake(); - } - - // No positive completion signal exists for "was ignored" — give - // the run-future a generous processing window, then assert. - for _ in 0..50 { - tokio::task::yield_now().await; - } - tokio::time::sleep(Duration::from_millis(10)).await; - - let observed = OBSERVED_SD_UNICAST.get().and_then(|m| m.lock().unwrap().clone()); - assert!( - observed.is_none(), - "observer must NOT fire for SD messages; got {observed:?}" - ); - handle.abort(); - let _ = handle.await; -} - -/// A registered observer must NOT fire for a non-SD datagram arriving -/// on the SD/multicast socket — the observer contract is unicast-only -/// (`from_unicast == true`). -#[tokio::test] -async fn non_sd_observer_ignores_non_sd_on_multicast_socket() { - let unicast_pipe = Arc::new(MockPipe::default()); - let sd_pipe = Arc::new(MockPipe::default()); - let factory = MockFactory { - unicast_pipe: Arc::clone(&unicast_pipe), - sd_pipe: Arc::clone(&sd_pipe), - next_port: Arc::new(Mutex::new(0)), - }; - - let e2e_handle: Arc> = Arc::new(Mutex::new(E2ERegistry::new())); - let config = ServerConfig::new(0x1234, 1) - .with_interface(Ipv4Addr::LOCALHOST) - .with_local_port(30703); - - let deps: ServerDeps>, MockSubscriptions> = - ServerDeps { - factory, - timer: MockTimer, - e2e_registry: e2e_handle, - subscriptions: MockSubscriptions::default(), - non_sd_observer: Some((record_multicast as NonSdRequestCallback, 9)), - }; - - let (_server, _handles, run): ( - Server>, MockSubscriptions>, - _, - _, - ) = Server::new_with_deps(deps, config, false) - .await - .expect("Server::new_with_deps must succeed"); - let handle = tokio::spawn(run); - - let src = SocketAddrV4::new(Ipv4Addr::new(192, 0, 2, 103), 40003); - sd_pipe - .inbound - .lock() - .unwrap() - .push_back((build_method_request(0x1234, 0x0001), src)); - if let Some(w) = sd_pipe.inbound_waker.lock().unwrap().take() { - w.wake(); - } - - for _ in 0..50 { - tokio::task::yield_now().await; - } - tokio::time::sleep(Duration::from_millis(10)).await; - - let observed = OBSERVED_MULTICAST.get().and_then(|m| m.lock().unwrap().clone()); - assert!( - observed.is_none(), - "observer must NOT fire for non-unicast datagrams; got {observed:?}" - ); - handle.abort(); - let _ = handle.await; -} - -/// With `non_sd_observer: None`, a non-SD unicast datagram is processed -/// without panicking (historical "ignore" behavior). This is all the -/// `None` case can actually prove — there is no callback to witness. -#[tokio::test] -async fn non_sd_observer_none_preserves_ignore_behavior() { - let unicast_pipe = Arc::new(MockPipe::default()); - let sd_pipe = Arc::new(MockPipe::default()); - let factory = MockFactory { - unicast_pipe: Arc::clone(&unicast_pipe), - sd_pipe: Arc::clone(&sd_pipe), - next_port: Arc::new(Mutex::new(0)), - }; - - let e2e_handle: Arc> = Arc::new(Mutex::new(E2ERegistry::new())); - let config = ServerConfig::new(0x1234, 1) - .with_interface(Ipv4Addr::LOCALHOST) - .with_local_port(30701); - - let deps: ServerDeps>, MockSubscriptions> = - ServerDeps { - factory, - timer: MockTimer, - e2e_registry: e2e_handle, - subscriptions: MockSubscriptions::default(), - non_sd_observer: None, - }; - - let (_server, _handles, run): ( - Server>, MockSubscriptions>, - _, - _, - ) = Server::new_with_deps(deps, config, false) - .await - .expect("Server::new_with_deps must succeed"); - let handle = tokio::spawn(run); - - let src = SocketAddrV4::new(Ipv4Addr::new(192, 0, 2, 101), 40001); - unicast_pipe - .inbound - .lock() - .unwrap() - .push_back((build_method_request(0x1234, 0x0001), src)); - if let Some(w) = unicast_pipe.inbound_waker.lock().unwrap().take() { - w.wake(); - } - - for _ in 0..50 { - tokio::task::yield_now().await; - } - tokio::time::sleep(Duration::from_millis(10)).await; - - assert!( - !handle.is_finished(), - "run-future must keep running (no panic / no error) after \ - ignoring a non-SD datagram with no observer registered" - ); - handle.abort(); - let _ = handle.await; -} -``` - -- [ ] **Step 4: Run the new tests — verify they pass, then verify they CAN fail** - -Run: `cargo test --features server,bare_metal --test bare_metal_server -- --test-threads=1 2>&1 | grep -E "^test result"` -Expected: `ok`, 0 failed. - -Sanity-check the witness is live: temporarily change `else if from_unicast` -at `src/server/runtime.rs` to `else if true`, rerun -`cargo test --features server,bare_metal --test bare_metal_server non_sd_observer_ignores_non_sd_on_multicast -- --test-threads=1`, -expect FAIL (observer fired); **revert the temporary change** and rerun to -green before committing. - -- [ ] **Step 5: Commit** - -```bash -git add tests/bare_metal_server.rs -git commit -m "test(server): live-witness negative tests for the non-SD observer - -The old None-case test could not fail (its witness was never -registered). Replace with: registered observer must not fire for SD -messages on the unicast socket nor for non-SD datagrams on the SD -socket; the None case shrinks to its real guarantee (no panic). -Refutation-checked by inverting the from_unicast branch. - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 5: Eager-`Ready` timing docs (spec item 2, doc-only) - -**Files:** -- Modify: `src/server/subscription_manager.rs:321-341` (trait method docs), `:495-502` (impl comment) - -- [ ] **Step 1: Trait method notes** - -On `SubscriptionHandle::subscribe` (after the "Idempotent…" paragraph, before `fn subscribe`): - -```rust - /// Timing note: implementations whose critical section is fully - /// synchronous (e.g. `StaticSubscriptionHandle`) may perform the - /// mutation when the future is *constructed*, deferring only the - /// result delivery to the poll. Callers must not assume that - /// constructing the returned future is free of side effects. -``` - -On `unsubscribe` (after "Remove a subscriber from an event group."): - -```rust - /// Same construction-time-mutation caveat as [`Self::subscribe`]. -``` - -- [ ] **Step 2: Impl-side sentence** - -In the comment block above `type SubscribeFuture` in -`impl SubscriptionHandle for StaticSubscriptionHandle` -(`src/server/subscription_manager.rs:496-502`), append after -"…satisfying any `Send`-checked run path.": - -```rust - // Consequence (documented on the trait): the mutation runs - // eagerly at future construction; only the result is delivered - // through the poll. The in-tree caller awaits immediately, so - // the difference from the lazy boxed impls is unobservable - // there. -``` - -- [ ] **Step 3: Verify docs build + commit** - -Run: `cargo doc --no-deps --features server,bare_metal 2>&1 | tail -2` — expect `Finished`. - -```bash -git add src/server/subscription_manager.rs -git commit -m "docs(server): document eager-at-construction timing of Ready-based subscribe - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 6: `announce_only_future` rationale (spec item 3, doc-only) - -**Files:** -- Modify: `src/server/mod.rs:1286-1296` (doc comment) - -- [ ] **Step 1: Append to the doc comment** - -After "…competing for inbound datagrams." and before "The returned future -loops forever…", insert: - -```rust - /// Design note: this partially reintroduces the split-future shape - /// phase 21 removed — deliberately. An announce-only future never - /// touches the receive path, so the invariant that motivated the - /// phase-21 combined run-future (no two futures racing the same - /// sockets and SD session counter) is preserved: the [`Self::run`] - /// path is still guarded by the first-poll `started` latch, and - /// supplementary announce loops only ever *send* on the shared SD - /// socket. - /// -``` - -- [ ] **Step 2: Verify + commit** - -Run: `cargo doc --no-deps --features server,bare_metal 2>&1 | tail -2` — expect `Finished`. - -```bash -git add src/server/mod.rs -git commit -m "docs(server): record why announce_only_future may split the run shape - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 7: `UDP_BUFFER_SIZE` rustdoc trues-up (spec item 5) - -**Files:** -- Modify: `src/lib.rs:145-150` - -- [ ] **Step 1: Confirm the claim is stale before editing** - -Run: `grep -rn "alloc::vec\|Vec::with_capacity\|vec!" src/server/runtime.rs src/protocol/sd/ --include="*.rs" | grep -v test` -Expected: no production-path `std`/`alloc` `Vec` hits (heapless only). `send_ack`/`send_nack` build into `[0u8; crate::UDP_BUFFER_SIZE]` (`src/server/runtime.rs`, inside `send_ack`); the `server,bare_metal` rlib `nm`-audits to zero allocator symbols (CI since PR 0). If a real heap `Vec` shows up in an outbound SD path, STOP — the doc is not stale and the spec is wrong; report back. - -- [ ] **Step 2: Rewrite the stale sentence** - -Replace (in the `UDP_BUFFER_SIZE` doc): - -```text -Paths that return early before -attempting serialization (e.g. `publish_event` when there are no -subscribers) are not affected. Other outbound SD paths (announcement -builders, `SubscribeAck` / `SubscribeNack`) currently still use -heap `Vec` buffers and are not capped by this constant — that is a -known gap, planned alongside the bare-metal `no_alloc` refactor. -``` - -with: - -```text -Paths that return early before -attempting serialization (e.g. `publish_event` when there are no -subscribers) are not affected. The remaining outbound SD paths -(`OfferService` announcements, `SubscribeAck` / `SubscribeNack`) -serialize into stack buffers of this same size — PR #124's no-alloc -server work removed the former heap `Vec` buffers, so every outbound -path is capped by this constant. -``` - -- [ ] **Step 3: Verify + commit** - -Run: `cargo doc --no-deps 2>&1 | tail -2` — expect `Finished`. - -```bash -git add src/lib.rs -git commit -m "docs: UDP_BUFFER_SIZE rustdoc — outbound SD paths are stack-buffered since #124 - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 8: CHANGELOG - -**Files:** -- Modify: `CHANGELOG.md` (0.8.0 section — add a `#### Breaking` block after the existing GAT one, matching its style) - -- [ ] **Step 1: Add the entry** - -````markdown -#### Breaking — `NonSdRequestCallback` gains an opaque `ctx: usize` first argument - -```rust -// before -pub type NonSdRequestCallback = fn(data: &[u8], source: SocketAddrV4); -// after -pub type NonSdRequestCallback = fn(ctx: usize, data: &[u8], source: SocketAddrV4); -``` - -The observer is now registered as a `(callback, ctx)` pair -(`ServerDeps::non_sd_observer: Option<(NonSdRequestCallback, usize)>`), -and `ctx` is passed back verbatim on every invocation. FFI consumers -stash their state pointer as `usize`; pure-Rust callers pass `0`. -`usize` (rather than a stored `*mut c_void`) keeps `Server: Send` — -and therefore `Server::run`'s declared `+ Send` bound — with no -`unsafe` in this crate; the pointer cast lives in the consumer's -callback body. - -##### Migration - -`non_sd_observer: Some(my_cb)` → `non_sd_observer: Some((my_cb, 0))`, -and add the leading `ctx: usize` parameter to the callback. -```` - -- [ ] **Step 2: Commit** - -```bash -git add CHANGELOG.md -git commit -m "docs: changelog for the ctx-carrying NonSdRequestCallback break - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 9: Spec correction, full verification, push, PR - -**Files:** -- Modify: `docs/simple_someip/plans/2026-06-10-pr1-124-followups-design.md` (one word) - -- [ ] **Step 1: Fix the builder owner in the design doc** - -The spec says `ServerConfig::with_non_sd_observer`; the builder lives on -**ServerDeps**. Replace that one occurrence with -`ServerDeps::with_non_sd_observer` and commit: - -```bash -git add docs/simple_someip/plans/2026-06-10-pr1-124-followups-design.md -git commit -m "docs: with_non_sd_observer lives on ServerDeps, not ServerConfig - -Co-Authored-By: Claude Fable 5 " -``` - -- [ ] **Step 2: Full local verification (mirrors CI)** - -```bash -cargo fmt --check -cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic -cargo clippy --no-default-features -- -D warnings -D clippy::pedantic -cargo test --features server-tokio,client-tokio,bare_metal --tests --lib -- --test-threads=1 -cargo test --doc -cargo +nightly build --no-default-features --features client,bare_metal -Zbuild-std=core --target thumbv7em-none-eabihf -cargo +nightly build --no-default-features --features server,bare_metal -Zbuild-std=core --target thumbv7em-none-eabihf -cargo +nightly build --no-default-features --features client,server,bare_metal -Zbuild-std=core --target thumbv7em-none-eabihf -cargo clean -p simple-someip --target thumbv7em-none-eabihf -cargo build --target thumbv7em-none-eabihf --no-default-features --features server,bare_metal -nm -A target/thumbv7em-none-eabihf/debug/libsimple_someip.rlib | grep -c -E '__rust_alloc|__rg_alloc' -``` - -Expected: fmt/clippy clean; tests green; doc tests green; three `Finished`; -`nm` count `0`. The PR 0 future-size witnesses run inside the test step — -the `(fn, usize)` capture adds 8 bytes to the server run-future, far -inside the 25% budget headroom; if a witness trips, STOP and investigate -rather than raising a budget. - -- [ ] **Step 3: Push and open the PR (stacked on PR 0)** - -```bash -git push -u origin feature/pr1_124_followups -gh pr create --base feature/pr0_measurement_harness \ - --title "PR 1: #124 follow-ups — ctx-carrying NonSdRequestCallback + doc trues-up (#125 stack)" \ - --body "$(cat <<'EOF' -Second PR of the #125 / phase-22 stack -(docs/simple_someip/plans/2026-06-10-pr1-124-followups-design.md). -Stacked on PR 0 (#127); retargets when that merges. - -- **BREAKING:** `NonSdRequestCallback` gains an opaque `ctx: usize` - first argument, registered as a `(callback, ctx)` pair. Keeps - `Server: Send` with zero `unsafe` in-crate; FFI casts its pointer - in the callback body. Migration in CHANGELOG. -- Negative observer tests now have a live witness (the old None-case - test could not fail); mock transport gets per-socket pipes so - `from_unicast` is deterministic. -- Doc trues-up: eager-at-construction timing of the `Ready`-based - subscribe/unsubscribe; `announce_only_future` split-shape rationale; - `UDP_BUFFER_SIZE` outbound-SD claim (stack-buffered since #124). - -**Merge gate:** Feliciano gets the callback-signature heads-up first -so no further halo FFI builds on the bare `fn` shape. - -🤖 Generated with [Claude Code](https://claude.com/claude-code) -EOF -)" -``` - ---- - -## Self-review notes (already applied) - -- Spec coverage: item 1 → Task 2; item 2 → Task 5; item 3 → Task 6; - item 4 → Tasks 3+4 (the mock split is the enabling mechanic the spec - deferred to this plan); item 5 → Task 7; CHANGELOG → Task 8; the - spec's own ServerConfig/ServerDeps naming slip → Task 9. -- The `None` initializers needing no edit (Task 2 Step 5) was verified - against all 20 `non_sd_observer:` sites — only `Some(...)` sites and - type declarations change; `examples/*` and `tests/bare_metal_e2e.rs` - all pass `None` and compile unchanged. -- Task 4's refutation step (invert `from_unicast`, watch the test fail, - revert) guards against rebuilding another vacuous test. - ---- - -## Recorded deviation (2026-06-11, post-execution) - -Task 2's callback shape was superseded after execution by the -cross-branch contract reconciliation: the final signature is the -union `fn(ctx, source, service_id, method_id, payload, e2e_status)` -(library parses; e2e_status 0 = unchecked today). See the design -doc's §1 revision note. Applied as a follow-up commit rather than -rewriting this plan's task history. diff --git a/docs/simple_someip/plans/2026-06-11-128-embassy-union-rebase.md b/docs/simple_someip/plans/2026-06-11-128-embassy-union-rebase.md deleted file mode 100644 index 3bd6c2b0..00000000 --- a/docs/simple_someip/plans/2026-06-11-128-embassy-union-rebase.md +++ /dev/null @@ -1,360 +0,0 @@ -# PR #128 Rebase — Union Callback Adoption Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Rebase `feat/embassy-mem-channel-cap` (PR #128, Feliciano's draft) onto the merged #124→#127→#129 spine, dropping its three #124-duplicate commits and adopting the union `NonSdRequestCallback` contract for the runtime's `DispatchFn`, then capture the fresh size baseline that becomes PR 2's "before". - -**Architecture:** A 9-commit rebase with a small, fully-enumerated conflict surface (dry-run executed 2026-06-11 — see inventory below), followed by a mechanical union-adoption pass on the new `bare_metal_runtime` files (2 compile errors, both at `runtime.rs:345`), a `DISPATCH_CTX` parallel static, and ctx/source threading through `event_rx_dispatch_future`. - -**Tech Stack:** git rebase, Rust (nightly for the runtime's `#![feature]` and `-Zbuild-std`), cargo, GNU `nm`. - -**Ownership note:** `feat/embassy-mem-channel-cap` is Feliciano's draft PR. This plan produces a **preview branch**; force-pushing his branch happens only with his explicit ack (Task 8). - ---- - -## Dry-run inventory (executed 2026-06-11, preview preserved) - -A complete dry run exists: branch `preview/embassy_union_rebase` (tip `35ba14f`) in worktree `/tmp/embassy_rebase_preview`, rebased onto the PR 1 tip `8e41b0e`. Tasks 1–2 below are **already done on that branch** — an executor can resume from it (start at Task 3) or replay from scratch using the recipes. Per-commit results: - -| #128 commit | Result on rebase | -|---|---| -| `0901274`, `45a4630`, `1a4ca83` | **Dropped by construction** (rewritten duplicates of #124's `be292bb`/`3dafd3e`/`64fcc08`; patch-ids drifted so git will NOT auto-skip them — rebase from `1a4ca83`, not from the branch base) | -| `16e5b27` pre-bound sockets | clean | -| `1124531` host rx notify | clean | -| `7623829` payload/config sizes | clean | -| `63e549f` CLIENT_SOCKET_CHANNEL_CAP | 1 trivial conflict: `src/client/socket_manager.rs` import adjacency — keep BOTH lines | -| `0f48c16` ARENA cap cuts | clean | -| `d56a691` co-offered Subscribe | 1 conflict: `src/server/runtime.rs` — new `accept_subscribe` fn inserted above `handle_sd_message`; take embassy's insertion AND the chain's backticked doc line (`` `FindService` ``) | -| `77cf725` SD codec + helpers | 5 hunks in `src/server/mod.rs` / `src/server/runtime.rs` / `tests/bare_metal_server.rs` — ALL are union-vs-4-param of the same content; **keep HEAD (the union side) in every hunk**. The commit's new files (`src/sd_codec.rs`, helper fns) apply clean | -| `3f7d7d1` run_someip | clean | -| `223bf40` reusable runtime | clean (new files) — but **semantically un-adopted**: leaves exactly 2 compile errors at `src/bare_metal_runtime/runtime.rs:345` (E0308 `Some(fn)` vs `Option<(fn, usize)>`, E0605 4-param→6-param fn cast). Tasks 3–5 fix this | - ---- - -### Task 1: Preconditions and base selection - -**Files:** none (git only) - -- [ ] **Step 1: Pick the rebase target** - -The real target is `feature/phase21_api_symmetry` AFTER the spine (#124 → #127 → #129) merges. Until then, the PR 1 tip is content-identical: `origin/feature/pr1_124_followups` (`8e41b0e`). The preserved preview was built against the PR 1 tip. If the spine has merged since, replay Task 2 against the merged phase21 instead of resuming the preview — the inventory above still applies unless #129 was amended in review (check: `git log 8e41b0e..origin/feature/phase21_api_symmetry --oneline -- src/server/` — if #129 landed with changes beyond `8e41b0e`, re-verify the `77cf725` resolutions). - -- [ ] **Step 2: Resume or replay?** - -Run: `git -C /tmp/embassy_rebase_preview log --oneline -1 2>/dev/null` -If it prints `35ba14f feat(bare-metal): reusable runtime …`, resume from the preview (skip Task 2). Otherwise replay Task 2. - ---- - -### Task 2: The rebase (replay recipe — skip if resuming the preview) - -**Files:** conflict resolutions only, per the inventory. - -- [ ] **Step 1: Create the worktree and rebase from above the duplicates** - -```bash -git worktree add /tmp/embassy_rebase_preview -b preview/embassy_union_rebase origin/feat/embassy-mem-channel-cap -cd /tmp/embassy_rebase_preview -git rebase --onto 1a4ca83 -``` - -`` = `origin/feature/pr1_124_followups` (pre-spine-merge) or `origin/feature/phase21_api_symmetry` (post-merge). Rebasing from `1a4ca83` drops the three duplicates by construction. - -- [ ] **Step 2: Resolve `63e549f` (socket_manager.rs)** - -One hunk: HEAD's `use crate::log::{…}` vs embassy's `use super::CLIENT_SOCKET_CHANNEL_CAP;` at the same insertion point. Keep both (CLIENT_SOCKET_CHANNEL_CAP line first, then the log line). `git add` + `git rebase --continue`. - -- [ ] **Step 3: Resolve `d56a691` (runtime.rs)** - -One hunk: embassy inserts `async fn accept_subscribe(…)` (a ~95-line function) above `handle_sd_message`; HEAD's side of the hunk is only the doc line `/// Handle a Service Discovery message (Subscribe / \`FindService\` etc.).`. Resolution: take the entire embassy insertion, and replace its trailing un-backticked `FindService` doc line with the backticked HEAD version. `git add` + continue. - -- [ ] **Step 4: Resolve `77cf725` (3 files, 5 hunks)** - -Every hunk is the union contract (HEAD) vs the 4-param contract (embassy) of the SAME semantic content — the union strictly supersedes (it has everything plus `ctx` + `source`). Keep the HEAD side of **every** hunk; do NOT use whole-file `checkout --ours` (it would discard the commit's cleanly-merged additions elsewhere in those files). `git add -u` + continue. - -- [ ] **Step 5: Confirm completion** - -`3f7d7d1` and `223bf40` apply clean. Expected: `Successfully rebased`. Then `cargo +nightly check --no-default-features --features bare-metal-runtime,client 2>&1 | grep -c "^error"` → exactly 2 (both at `runtime.rs:345`) — that's the Task 3–5 worklist, not a failure. - ---- - -### Task 3: Union adoption — `DispatchFn`, `DISPATCH_CTX`, trampoline - -**Files:** -- Modify: `src/bare_metal_runtime/runtime.rs` (~line 63 alias; ~line 174 statics; ~line 263 trampoline; ~line 345 registration; ~line 441 init store; the init config struct — locate fields with `grep -n "pub dispatch\|dispatch:" src/bare_metal_runtime/runtime.rs`) - -- [ ] **Step 1: Reshape the alias** (~line 63) - -```rust -/// Platform dispatch sink for inbound messages (decoded by the runtime). -/// Same shape as [`crate::server::NonSdRequestCallback`] — the union -/// contract — so a platform can use one handler for both the server's -/// non-SD observer and the runtime's notification RX path. `ctx` is the -/// opaque word registered at [`init`]; `source` is the sender; -/// `e2e_status` is real on the RX path (Profile-5 check) and `0` -/// (unchecked) on the server-request path. -pub type DispatchFn = fn( - ctx: usize, - source: core::net::SocketAddrV4, - service_id: u16, - method_id: u16, - payload: &[u8], - e2e_status: u8, -); -``` - -- [ ] **Step 2: Add the parallel ctx static** (next to `static DISPATCH`, ~line 174) - -```rust -static DISPATCH: AtomicUsize = AtomicUsize::new(0); // DispatchFn as usize -static DISPATCH_CTX: AtomicUsize = AtomicUsize::new(0); // opaque ctx word for DISPATCH -``` - -- [ ] **Step 3: Register ctx at init** - -The init config struct (the one whose fields feed `SEND_FN`/`NOW_FN`/`DISPATCH` stores at ~line 439-441) gains a field: - -```rust - /// Opaque context word passed back verbatim as the first argument of - /// every `dispatch` invocation (FFI: stash a pointer as `usize`). - pub dispatch_ctx: usize, -``` - -and beside `DISPATCH.store(...)` (~line 441): - -```rust - DISPATCH_CTX.store(config.dispatch_ctx, Ordering::Release); -``` - -**Flag for Feliciano:** if that struct is `#[repr(C)]` consumed from C, this is a C-side ABI addition — field at the END of the struct, and the C header updates with it. - -- [ ] **Step 4: Reshape the trampoline** (~line 263) - -```rust -/// Forwards a parsed inbound message to the platform dispatch callback. -/// The `_ctx` received from the caller is ignored: the runtime's real -/// ctx lives in [`DISPATCH_CTX`] (registered at [`init`], possibly -/// re-registered later), so loading it here keeps late re-registration -/// coherent — callers register/pass `0`. -fn dispatch( - _ctx: usize, - source: core::net::SocketAddrV4, - service_id: u16, - method_id: u16, - payload: &[u8], - e2e_status: u8, -) { - let raw = DISPATCH.load(Ordering::Acquire); - if raw == 0 { - return; - } - // SAFETY: stored from a valid DispatchFn in `init`. - let f: DispatchFn = unsafe { core::mem::transmute::(raw) }; - f( - DISPATCH_CTX.load(Ordering::Acquire), - source, - service_id, - method_id, - payload, - e2e_status, - ); -} -``` - -- [ ] **Step 5: Fix the registration** (~line 345) - -```rust - non_sd_observer: Some((dispatch as crate::server::NonSdRequestCallback, 0)), -``` - -(`0` because the trampoline injects `DISPATCH_CTX` itself — see Step 4 doc.) - -- [ ] **Step 6: Verify the two errors are gone** - -Run: `cargo +nightly check --no-default-features --features bare-metal-runtime,client 2>&1 | grep -E "^error" | head` -Expected: errors at `runtime.rs:345` gone; remaining errors (if any) are in `bare_metal_tasks.rs` — Task 4's worklist. - ---- - -### Task 4: Thread ctx + source through `event_rx_dispatch_future` - -**Files:** -- Modify: `src/bare_metal_tasks.rs` (~lines 95-125, plus its callers — locate with `grep -n "event_rx_dispatch_future" src/`) - -- [ ] **Step 1: Reshape the helper** - -The fn's `dispatch` parameter is currently an inline 4-param fn type. It becomes the union shape plus a pass-through `ctx`, and the receive captures the datagram's source (`ReceivedDatagram.source` is already there — only `bytes_received` was being kept): - -```rust -pub async fn event_rx_dispatch_future<'a, S, R>( - rx_socket: &'a S, - e2e: &'a R, - e2e_enabled: bool, - dispatch: crate::bare_metal_runtime::DispatchFn, - ctx: usize, - buf: &'a mut [u8], -) where - S: TransportSocket, - R: E2ERegistryHandle, -{ - loop { - let (n, source) = match rx_socket.recv_from(&mut *buf).await { - Ok(d) => (d.bytes_received, d.source), - Err(_) => continue, - }; - let Some(parsed) = parse_someip_datagram(&buf[..n]) else { - continue; - }; - let (status, body) = if e2e_enabled { - check_parsed_e2e(e2e, &parsed) - } else { - (E2ECheckStatus::Unchecked, parsed.payload) - }; - dispatch( - ctx, - source, - parsed.service_id, - parsed.method_id, - body, - e2e_status_code(status), - ); - } -} -``` - -NOTE on the `dispatch` param type: if `bare_metal_tasks` must stay decoupled from the `bare-metal-runtime` feature (check the cfg on `bare_metal_runtime`'s module declaration in lib.rs), keep an inline fn type with the same six params instead of naming `DispatchFn`. External (non-runtime) callers pass their real callback + ctx and get verbatim forwarding; the runtime passes its trampoline + `0`. - -- [ ] **Step 2: Update the callers** - -`run_someip` (in `bare_metal_tasks.rs`) and any direct caller pass the extra `ctx` argument — the runtime's composition passes `0` (trampoline injects). Locate: `grep -n "event_rx_dispatch_future(" src/`. - -- [ ] **Step 3: Sweep for leftover 4-param shapes** - -Run: `grep -rn "fn(service_id: u16, method_id: u16" src/` -Expected: zero hits (every dispatch-shaped type is now the union). - -- [ ] **Step 4: Full check** - -Run: `cargo +nightly check --no-default-features --features bare-metal-runtime,client 2>&1 | tail -2` → `Finished`. - -- [ ] **Step 5: Commit** (one commit on the preview branch for Tasks 3+4) - -```bash -git add src/bare_metal_runtime/runtime.rs src/bare_metal_tasks.rs -git commit -m "feat(bare-metal): DispatchFn adopts the union callback contract - -Same six-param shape as NonSdRequestCallback (decision 2026-06-11): -ctx + source + decoded fields + e2e_status. DISPATCH_CTX parallel -static carries the opaque word; the dispatch trampoline injects it so -late re-registration stays coherent; event_rx_dispatch_future threads -ctx/source through (e2e_status stays REAL on this path). - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 5: CHANGELOG + sd_codec visibility note - -**Files:** -- Modify: `CHANGELOG.md` - -- [ ] **Step 1: Behavior notes under the 0.8.0 section** - -Append to the existing `#### Breaking — NonSdRequestCallback…` block's vicinity (match file style): - -- `PENDING_RESPONSES_CAP` 64→8 is a **global** bound (also tokio): more than 8 outstanding request-response pairs now returns `Err(Error::Capacity(…))`. Sized for the embedded target; raise the const if a host consumer genuinely needs more in flight. (`REQUEST_QUEUE_CAP` 32→4 is NOT consumer-visible: the feeding control channel was always depth 4 on both paths — verified `BoundedSender` + tokio `channel(N)`.) -- The bare-metal runtime's `DispatchFn` now matches `NonSdRequestCallback` (union shape); the init config gains `dispatch_ctx`. - -- [ ] **Step 2: sd_codec visibility — decision recorded, not changed** - -`sd_codec::parse_someip_datagram` stays at its current visibility (the runtime's RX path uses it; halo's FFI may too). Narrowing to `pub(crate)` is Feliciano's call on his own PR — leave a PR-comment question, don't change it here. - -- [ ] **Step 3: Commit** - -```bash -git add CHANGELOG.md -git commit -m "docs: changelog for ARENA cap bounds + DispatchFn union shape - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 6: Full verification - -- [ ] **Step 1: fmt the conflict resolutions** - -Run: `cargo fmt` then `git diff --stat` — the Task 2 resolutions (esp. `accept_subscribe`) may need reflow; if fmt changed files, amend them into the rebase HEAD: `git add -u && git commit --amend --no-edit` is WRONG here (HEAD is the Task 5 commit) — instead commit fmt separately: `git commit -m "style: rustfmt over rebase resolutions"`. - -- [ ] **Step 2: The matrix** - -```bash -cargo fmt --check -cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic -cargo clippy --no-default-features -- -D warnings -D clippy::pedantic -cargo test --features server-tokio,client-tokio,bare_metal --tests --lib -- --test-threads=1 -cargo test --doc -cargo check -p simple-someip-embassy-net --tests -cargo +nightly build --no-default-features --features client,bare_metal -Zbuild-std=core --target thumbv7em-none-eabihf -cargo +nightly build --no-default-features --features server,bare_metal -Zbuild-std=core --target thumbv7em-none-eabihf -cargo +nightly build --no-default-features --features client,server,bare_metal -Zbuild-std=core --target thumbv7em-none-eabihf -cargo +nightly build --no-default-features --features bare-metal-runtime,client -Zbuild-std=core --target thumbv7em-none-eabihf -cargo clean -p simple-someip --target thumbv7em-none-eabihf -cargo build --target thumbv7em-none-eabihf --no-default-features --features server,bare_metal -nm -A target/thumbv7em-none-eabihf/debug/libsimple_someip.rlib | grep -c -E '__rust_alloc|__rg_alloc' -``` - -Expected: all clean; nm prints `0`. The fourth build-std line is NEW (the runtime feature under halo's constraint) — if it fails on `embassy-executor` deps, record the failure verbatim; it's a finding about #128, not about this rebase. The future-size witnesses run inside the test step and must PASS (the cap cuts shrink futures; budgets are upper bounds). If the `#128` clippy surface has pre-existing pedantic warnings the chain's gate now catches (the chain enforces `--workspace --all-features`), fix mechanically and commit as `style:`. - -- [ ] **Step 3: Probe-mirror check (`7623829` touched payload/option sizes)** - -`tools/size_probe`'s `ProbePayload` mirrors `TestPayload` (`src/protocol/sd/test_support.rs`) field-for-field. `7623829` changed `heapless_payload.rs` / `sd/options.rs` / `static_channels` — verify `TestPayload`/`TestSdHeader` themselves are untouched (`git diff 1a4ca83..HEAD -- src/protocol/sd/test_support.rs` → empty means the mirror holds). `MAX_CONFIGURATION_STRING_LENGTH` changes WILL shift captured layouts — that's expected and handled by Step 4's re-capture, not an error. - -- [ ] **Step 4: Fresh baseline — this is PR 2's "before"** - -```bash -tools/capture_type_sizes.sh -``` - -Copy the new numbers into a new committed baseline `docs/simple_someip/plans/baselines/post-128-size-baseline.md` (same format as `pr0-size-baseline.md`, with a header noting: captured on the #128-rebased tree; supersedes pr0 baseline as PR 2's "before"; the deltas vs pr0 quantify #128's cap cuts — record them, they're the first real measured win of the stack). Also re-run the host witnesses with `--nocapture` and record the `FUTURE_SIZE` lines. - -```bash -git add docs/simple_someip/plans/baselines/post-128-size-baseline.md -git commit -m "docs: post-#128 size baseline (PR 2's before; quantifies the cap cuts) - -Co-Authored-By: Claude Fable 5 " -``` - ---- - -### Task 7: Witness-budget tightening decision (optional, record either way) - -The PR 0 witness budgets are `pr0-baseline × 1.25`. After #128's cuts the real sizes drop well below those budgets, leaving slack that could mask a future regression up to the OLD budget. Either tighten the budget consts (`src/client/mod.rs`, `tests/bare_metal_e2e.rs`) to `post-128-baseline × 1.25` in this branch, or record in the new baseline doc that tightening lands with PR 2. **Default: tighten now** — it's two consts per file and the witnesses exist to be tight. - ---- - -### Task 8: Handoff (Feliciano coordination — DO NOT force-push his branch unilaterally) - -- [ ] **Step 1: Push the preview** - -```bash -git push -u origin preview/embassy_union_rebase -``` - -- [ ] **Step 2: PR comment on #128** - -Summarize: rebase preview ready; 3 duplicate commits dropped; his 9 commits survive with authorship intact (rebase preserves author); union contract adopted for `DispatchFn` + `DISPATCH_CTX` + init-config `dispatch_ctx` field (C-side ABI addition flagged); the conflict inventory + this plan's path; ask him to either `git reset --hard origin/preview/embassy_union_rebase && git push --force-with-lease` on his branch, or cherry-pick at his leisure. Include the open question: `sd_codec` visibility (keep `pub` for halo FFI, or narrow?). - -- [ ] **Step 3: Sequencing reminder** - -This branch can only MERGE after the spine (#124→#127→#129) lands in phase21 — it contains the spine. If #129 gets amended in review, re-run Task 1 Step 1's check and rebase the preview again (cheap: the inventory holds). - ---- - -## Self-review notes (already applied) - -- The dry run IS the spec-coverage check: every #128 commit is accounted for in the inventory table; the 2 compile errors are closed by Tasks 3–4; sweep step (Task 4 Step 3) catches any 4-param stragglers. -- `e2e_status` semantics differ by path and both docs say so: REAL on the RX/notification path (`check_parsed_e2e`), `0` on the server-request path — the union docs on both aliases carry the distinction. -- The trampoline-injects-ctx design (register `(dispatch, 0)`) was chosen over registering the real ctx because `DISPATCH`/`DISPATCH_CTX` support late re-registration; the server's stored copy would go stale. Documented on the trampoline. -- `event_rx_dispatch_future` gets ctx as a pass-through parameter (not a static read) because it's a public spawnable helper — non-runtime callers need verbatim forwarding. diff --git a/docs/simple_someip/plans/2026-06-17-pr126-polled-port-handoff.md b/docs/simple_someip/plans/2026-06-17-pr126-polled-port-handoff.md deleted file mode 100644 index 0c90dd05..00000000 --- a/docs/simple_someip/plans/2026-06-17-pr126-polled-port-handoff.md +++ /dev/null @@ -1,67 +0,0 @@ -# Handoff: port #126 (polled bare-metal) onto the #125 stack - -**For:** Feliciano (owner of the polled module) -**Branch:** `feat/polled-port-onto-125` — branched off the #133 tip (`82be01d`), the top of the completed #125 stack (`#131 → #127 → #129 → #132 → #133`). This is your port target; it builds clean. -**Why a port, not a rebase:** `#126`'s `polled.rs` is built on your *own* divergent refactor of the alloc-free server, the `E2ERegistry`, and a new mutex — and the #125 stack reworked those same areas differently. A `git rebase` of `#126` onto #133 hits 15 conflict hunks across `server/mod.rs`/`runtime.rs`/`subscription_manager.rs` and replays commits that duplicate (and contradict) the stack. So the clean path is: branch off #133, bring `polled.rs` over, and adapt it to the stack's APIs. - -## Decision already made (Justin, 2026-06-17): take the union callback - -`NonSdRequestCallback` is **already the agreed union** on the stack — no change needed there. At `src/server/mod.rs:666`: -```rust -pub type NonSdRequestCallback = fn( - ctx: usize, source: core::net::SocketAddrV4, - service_id: u16, method_id: u16, payload: &[u8], e2e_status: u8, -); -``` -Library-side header parse happens at the call site (`runtime.rs:619`, `view.header()`) — the consumer never hand-rolls parsing (MISRA/ASIL). `ctx: usize` (not `*mut c_void`) keeps `Server: Send`; `source` keeps reply-routing; `e2e_status` is live on the client path. halo uses neither `ctx` nor `source`, but they stay for other consumers. **Your `#126` commit `4c2531d "remove non-SD observer callback"` is therefore dropped — we keep the observer.** Apply this same shape to the runtime `DispatchFn` when you bring it over. - -## Your `#126` commits — keep / drop / adapt - -| commit | what | action | -|---|---|---| -| `4faac4a` make ...alloc-free, add `NonSdRequestCallback` | server/mod, runtime, subscription_manager | **DROP** — superseded by `#131` (stack's alloc-free server) | -| `4c2531d` remove non-SD observer | server/mod (−28), runtime (−13) | **DROP** — superseded by the union decision | -| `929962a` const-generic `E2ERegistry` | e2e/registry, subscription_manager (+164), transport, event_publisher | **DON'T adopt into the stack** — see "E2E" below; adapt polled instead | -| `372c7d4` `SingleContextRawMutex` | new file, subscription_manager, transport | **your call** — bring the primitive if polled needs it (see "mutex") | -| `25ba82b` sync SOME/IP helpers | new `src/polled.rs` (+285), header.rs | **KEEP** — port | -| `3b8e483` polled integration | `polled.rs` (+464) | **KEEP** — port | -| `e7c956c` multi-offer builders | `polled.rs` (+114) | **KEEP** — port | - -Bring the polled sources over with: -```bash -git checkout feat/polled-port-onto-125 -git checkout origin/feat/polled-bared-metal -- src/polled.rs -# (and src/single_context_mutex.rs if you decide to keep the mutex) -# then wire `mod polled;` into src/lib.rs and adapt — see below. -``` - -## Target API the stack provides (what to adapt `polled.rs` against) - -- **`NonSdRequestCallback`** — the union above (`src/server/mod.rs:666`); registered via `ServerDeps`/`ServerStorage.non_sd_observer: Option<(NonSdRequestCallback, usize)>` and invoked at `runtime.rs:619`. -- **`E2ERegistry`** (`src/e2e/registry.rs`) is a **plain, non-const-generic** struct: `register(key, profile) -> Result<(), E2ERegistryFull>`, `contains_key(&key) -> bool`, `check(...)`, `protect(...)`. The handle trait `E2ERegistryHandle` and `StaticE2EHandle` live in `src/transport.rs` as the stack left them (PR 2/PR 3). **Your `929962a` changed these to a const-generic (`E2E_CAP`) shape** — `polled.rs` (`check_parsed_e2e`, the `E2E_CAP` params) and its `use crate::transport::E2ERegistryHandle` / `use crate::StaticE2EHandle` depend on that. Adapt polled's E2E usage to the stack's non-const-generic handle surface. -- **Server send/run** (PR 3 — caller-buffer model): `Server::run_with_buffers(unicast_buf, sd_buf, recv_send_buf, announce_send_buf: &mut [u8])`; new public `Server::announce_only_with_buffer(&mut [u8])`; `EventPublisher::publish_event_with_buffers(.., msg_buf, protected_buf)` / `publish_raw_event_with_buffers(.., buf)`. The SD send helpers in `runtime.rs`/`sd_state.rs` now take caller scratch and bound on `buf.len()`. If polled re-implements SD datagram building (`build_multi_offer_service_datagram`), reconcile against these. -- **`subscription_manager`** caps: `EVENT_GROUPS_CAP = 32`, `SUBSCRIBERS_PER_GROUP = 16`; backed by `FnvIndexMap`. `929962a` reshaped this (+164) — adapt polled's `` usage to the stack's. - -## Adaptation list (the divergent-API touch points in `polled.rs`) - -1. **E2E:** `use crate::transport::E2ERegistryHandle`, `use crate::StaticE2EHandle`, `use crate::E2ECheckStatus`, and `check_parsed_e2e` (`polled.rs:341`) — reconcile with the stack's non-const-generic `E2ERegistry`/handle traits. **Do NOT pull `929962a` into the stack** to satisfy this (see rationale below); adapt polled. -2. **Mutex:** any `SingleContextRawMutex` use — decide whether to bring `372c7d4`'s primitive (it's small and arguably generally useful) or use the stack's existing mutex abstraction. -3. **Server/observer:** polled's references to the removed observer / your alloc-free server APIs — retarget to the stack's union `NonSdRequestCallback` + `#131`/PR 3 server surface. -4. **SD builders:** `build_multi_offer_service_datagram` / `build_multi_stop_offer_service_datagram` (`polled.rs:145/158`) — confirm they encode against the same SD wire format the stack's `sd_state`/`runtime` helpers use (wire format is unchanged across the #125 stack). - -## Why the const-generic `E2ERegistry` (`929962a`) is NOT being adopted into the stack - -Adopting it would re-open the audited E2E code that PR 2/PR 3 just reworked and reviewed (the E2E-OOB fixes + `buf.len()` guards), force a public `E2ERegistry` API migration on other consumers (e.g. dft, which uses the E2E/`e2e_status` surface), and add per-instantiation monomorphization/flash cost the flash-constrained TC4/halo target doesn't want — all to host one module. If the const-generic registry is worth it on its own merits, it should be its own PR with its own review + dft-impact call, not a polled dependency. So: `polled.rs` (a consumer) adapts to the crate's reviewed E2E surface. - -## Suggested order - -1. `git checkout origin/feat/polled-bared-metal -- src/polled.rs` onto this branch; wire `mod polled;` into `lib.rs`. -2. Compile under `--no-default-features --features `; fix the E2E-handle + mutex + observer references against the target APIs above. -3. Re-apply the union `NonSdRequestCallback` shape to the runtime `DispatchFn`. -4. Run `cargo build --target thumbv7em-none-eabihf ...` for the polled feature combos + the no-alloc witness. -5. Clear the open `#126` review punch list (mutex feature-unification footgun, missing tests/CI, Ack-on-failure) as part of this. -6. Open the PR based on `feature/pr3_125_server_buffers` (keep it stacked; never merge-down). - -`#128` (embassy-mem-channel-cap) is still a draft and stacks *after* this once it lands. - -Backups of the original tips: `backup/feat-polled-bared-metal-20260617-prestack2`, `backup/feat-embassy-mem-channel-cap-20260617-prestack2`. diff --git a/docs/simple_someip/plans/2026-06-17-pr2-125-client-async-state-reduction.md b/docs/simple_someip/plans/2026-06-17-pr2-125-client-async-state-reduction.md deleted file mode 100644 index c7fc5d25..00000000 --- a/docs/simple_someip/plans/2026-06-17-pr2-125-client-async-state-reduction.md +++ /dev/null @@ -1,767 +0,0 @@ -# PR 2 — #125 Client Async-State Reduction Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Move the client's per-socket `[u8; UDP_BUFFER_SIZE]` buffers out of the spawned socket-loop futures and into caller-sized pooled storage, and flatten the control-message handler, so the client's Embassy-arena (TaskStorage) footprint drops and becomes consumer-sized — closing the client half of issue #125. - -**Architecture:** Three moves. (1) Add a `BufferPool` static-storage primitive and a `BufferProvider` trait that mirror the existing channel-pool machinery (`OneshotPool`/`MpscPool` + `ChannelFactory`); a claim returns a RAII `BufferLease` that derefs to `&'static mut [u8]` and returns its slot on drop. (2) Thread a `BufferProvider` through `ClientDeps` → `BindDispatch` → `SocketManager::bind_*`; the socket loop receives its buffer by slice instead of owning a stack array, so the buffer lives in consumer `.bss` (or the tokio heap pool), not in the future. (3) Flatten `handle_control_message` into a synchronous decode/decide returning a small action value, with awaits hoisted to shallow helpers — kept only where the PR-0 witnesses show it moves the number. - -**Tech Stack:** Rust (edition 2024, crate stays stable-buildable), `heapless`, `embassy-sync` (bare-metal channel backend), `critical-section` (no_std slot synchronization), `tokio` (std path only), cargo, `cargo-nextest`. - -**Spec:** `docs/simple_someip/plans/2026-06-09-phase22-125-memory-reduction-design.md` (PR 2 section) and its rev-2 caller-sized-buffers decision. - -## Global Constraints - -- **Crate stays stable-buildable.** No nightly-only features in `simple-someip` itself. (CI may use nightly only for `-Zprint-type-sizes` / `-Zbuild-std` measurement jobs.) -- **Wire format untouched.** No change to any byte emitted or parsed. -- **Public tokio/std API unchanged.** Callers using the tokio-defaulted `ClientDeps` constructor see no signature change; the buffer pool is provisioned internally (8 × `UDP_BUFFER_SIZE`). -- **Only these behavioral changes are allowed** (all from the design doc's PR-2 section; everything else is behavior-preserving): - - (a) An inbound datagram larger than the claimed receive buffer is **dropped with a log**, not truncated/panicked. - - (b) The oversize-send rejection compares against the **claimed buffer length** (`buf.len()`), not the `UDP_BUFFER_SIZE` constant. -- **Existing suite stays green on every commit** (~543 tests as of #114 plus #124's additions; the `client,bare_metal` no-alloc witness and the embassy-net loopback live-wire test included). -- **Every memory change is validated against PR-0's future-size witnesses** (`tests/bare_metal_e2e.rs`). A change that does not move a witness number is dropped, not merged on faith. -- **No `&'static mut` aliasing.** A buffer slot is handed out to exactly one lease at a time; the pool enforces this and is covered by a witness test. -- **Exact constants (copy verbatim):** `UDP_BUFFER_SIZE = 1500` (`src/lib.rs:158`); `UNICAST_SOCKETS_CAP = 8` (`src/client/inner.rs:40`). - ---- - -## File Structure - -| File | Action | Responsibility | -|---|---|---| -| `src/static_channels/buffer_pool.rs` | Create | `BufferPool` static primitive + `BufferLease` RAII handle (claim/release, no aliasing). | -| `src/static_channels/mod.rs` | Modify | `mod buffer_pool; pub use buffer_pool::{BufferPool, BufferLease};` | -| `src/transport.rs` | Modify | `BufferProvider` trait (`claim(&self) -> Option`), next to the `*Pooled` traits. | -| `src/client/socket_manager.rs` | Modify | `socket_loop_future` takes the buffer by lease; oversize-send check keys off `buf.len()`; inbound-oversize drop+log; `bind_*` claim a buffer before spawn. | -| `src/client/bind_dispatch.rs` | Modify | Thread the `BufferProvider` from `SpawnerDispatch` into the `SocketManager::bind_*` calls. | -| `src/client/inner.rs` | Modify | Hold the provider; flatten `handle_control_message` into a sync decide + hoisted awaits. | -| `src/client/mod.rs` | Modify | Add `buffer_provider` field/generic to `ClientDeps`; rewrite the `:1-30` memory-footprint doc. | -| `src/tokio_transport.rs` | Modify | Heap-backed `BufferProvider` (leak a `[u8; UDP_BUFFER_SIZE] × 8` store) wired into the tokio-defaulted `ClientDeps` constructor. | -| `tests/buffer_pool.rs` | Create | Unit + witness tests for claim-to-exhaustion, release-on-drop, no double-claim. | -| `tests/bare_metal_e2e.rs` | Modify | Extend/retighten the future-size witnesses after extraction. | - ---- - -### Task 1: `BufferPool` static primitive + `BufferLease` - -**Files:** -- Create: `src/static_channels/buffer_pool.rs` -- Modify: `src/static_channels/mod.rs` -- Test: `tests/buffer_pool.rs` - -**Interfaces:** -- Produces: - - `pub struct BufferPool` with `pub const fn new() -> Self` and `pub fn claim(&'static self) -> Option`. - - `pub struct BufferLease { /* private */ }` implementing `Deref`, `DerefMut`, `Drop` (returns the slot), and `Send`. - -- [ ] **Step 1: Write the failing test** - -```rust -// tests/buffer_pool.rs -use simple_someip::static_channels::BufferPool; - -static POOL: BufferPool<2, 4> = BufferPool::new(); - -#[test] -fn claim_returns_distinct_zeroed_slices_until_exhausted() { - let mut a = POOL.claim().expect("slot 0"); - let b = POOL.claim().expect("slot 1"); - assert_eq!(a.len(), 4); - assert_eq!(&*b, &[0u8; 4]); // freshly handed-out slot is zeroed - a[0] = 0xAB; // writable - assert_eq!(a[0], 0xAB); - assert!(POOL.claim().is_none(), "pool of 2 must refuse a 3rd claim"); -} - -#[test] -fn dropping_a_lease_returns_its_slot() { - let a = POOL.claim().expect("slot"); - drop(a); - assert!(POOL.claim().is_some(), "slot must be reusable after the lease drops"); -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cargo test --test buffer_pool` -Expected: FAIL — `BufferPool` is not found / unresolved import. - -- [ ] **Step 3: Write the implementation** - -```rust -// src/static_channels/buffer_pool.rs -//! Fixed-capacity pool of `&'static mut [u8]` buffers with claim/release -//! semantics, mirroring the channel pools in this module. A `BufferPool` -//! is declared as a `static` by the consumer; each `claim()` hands out one -//! slot as a [`BufferLease`] that returns the slot to the pool on drop. -//! -//! Synchronization uses `critical-section` so the same code is valid on the -//! bare-metal (single-core, no atomics-guarantee) target and on std. - -use core::cell::UnsafeCell; -use core::ops::{Deref, DerefMut}; - -/// Backing storage for a pool: `SLOTS` independent `LEN`-byte buffers plus a -/// claimed-flag per slot. -pub struct BufferPool { - // `UnsafeCell` because `claim()` hands out `&'static mut` into this store; - // the `claimed` flags guarantee at most one live `&mut` per slot. - store: UnsafeCell<[[u8; LEN]; SLOTS]>, - claimed: UnsafeCell<[bool; SLOTS]>, -} - -// SAFETY: all access to `store`/`claimed` is funneled through a -// `critical_section::with`, which provides mutual exclusion on the targets -// we support; a slot is only aliased once (its `claimed` flag gates it). -unsafe impl Sync for BufferPool {} - -impl BufferPool { - #[must_use] - pub const fn new() -> Self { - Self { - store: UnsafeCell::new([[0u8; LEN]; SLOTS]), - claimed: UnsafeCell::new([false; SLOTS]), - } - } - - /// Claim a free slot, or `None` if all `SLOTS` are in use. The returned - /// buffer is zeroed before hand-out so a reused slot never leaks the - /// previous tenant's bytes. - pub fn claim(&'static self) -> Option { - critical_section::with(|_| { - // SAFETY: inside the critical section we hold exclusive access to - // both arrays; we take a raw pointer and only form one `&mut` for - // the chosen, not-yet-claimed slot. - let claimed = unsafe { &mut *self.claimed.get() }; - let idx = claimed.iter().position(|&c| !c)?; - claimed[idx] = true; - let store = unsafe { &mut *self.store.get() }; - let slot: &'static mut [u8; LEN] = unsafe { &mut *(&mut store[idx] as *mut [u8; LEN]) }; - slot.fill(0); - Some(BufferLease { - buf: slot.as_mut_slice(), - claimed_flag: claimed.as_mut_ptr(), - idx, - }) - }) - } -} - -impl Default for BufferPool { - fn default() -> Self { - Self::new() - } -} - -/// RAII handle to one claimed buffer. Derefs to the `&'static mut [u8]`; -/// returns the slot to its pool on drop. -pub struct BufferLease { - buf: &'static mut [u8], - claimed_flag: *mut bool, - idx: usize, -} - -// SAFETY: `BufferLease` owns exclusive access to its slot (gated by the -// pool's `claimed` flag) and the backing store is `'static`. -unsafe impl Send for BufferLease {} - -impl Deref for BufferLease { - type Target = [u8]; - fn deref(&self) -> &[u8] { - self.buf - } -} - -impl DerefMut for BufferLease { - fn deref_mut(&mut self) -> &mut [u8] { - self.buf - } -} - -impl Drop for BufferLease { - fn drop(&mut self) { - critical_section::with(|_| { - // SAFETY: `claimed_flag` points into the owning pool's `'static` - // `claimed` array; only this lease writes `idx`'s flag. - unsafe { - *self.claimed_flag.add(self.idx) = false; - } - }); - } -} -``` - -```rust -// src/static_channels/mod.rs — add near the other `mod`/`pub use` lines -mod buffer_pool; -pub use buffer_pool::{BufferLease, BufferPool}; -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `cargo test --test buffer_pool` -Expected: PASS (both tests). - -- [ ] **Step 5: Confirm no_std-clean and lint-clean** - -Run: `cargo clippy -p simple-someip --no-default-features --features client,bare_metal -- -D warnings -D clippy::pedantic` -Expected: no warnings. (`critical-section` is already a transitive dep via `embassy-sync` per `Cargo.toml`; if the bare-metal build cannot find it, add `critical-section = { version = "1", optional = true }` and include it in the `bare_metal` feature.) - -- [ ] **Step 6: Commit** - -```bash -git add src/static_channels/buffer_pool.rs src/static_channels/mod.rs tests/buffer_pool.rs -git commit -m "feat(static_channels): BufferPool + BufferLease claim/release primitive (#125)" -``` - ---- - -### Task 2: `BufferProvider` trait + static & tokio impls - -**Files:** -- Modify: `src/transport.rs` (next to `OneshotPooled`/`BoundedPooled`, ~`:1342`) -- Modify: `src/tokio_transport.rs` (after the `TokioChannels` `*Pooled` impls, ~`:550`) -- Test: `tests/buffer_pool.rs` - -**Interfaces:** -- Consumes: `BufferLease`, `BufferPool` (Task 1). -- Produces: - - `pub trait BufferProvider: Clone + Send + Sync + 'static { fn claim(&self) -> Option; }` - - `pub struct StaticBufferProvider(pub &'static BufferPool);` impl `BufferProvider`. - - `pub struct TokioBufferProvider;` impl `BufferProvider` (heap-backed, `UDP_BUFFER_SIZE`-sized). - -- [ ] **Step 1: Write the failing test** - -```rust -// tests/buffer_pool.rs (append) -use simple_someip::static_channels::{BufferPool, BufferLease}; -use simple_someip::transport::{BufferProvider, StaticBufferProvider}; - -static PROV_POOL: BufferPool<2, 8> = BufferPool::new(); - -#[test] -fn static_provider_claims_through_a_shared_pool() { - let prov = StaticBufferProvider(&PROV_POOL); - let _a = prov.claim().expect("first"); - let _b = prov.claim().expect("second"); - assert!(prov.claim().is_none(), "provider exposes the pool's capacity"); -} -``` - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cargo test --test buffer_pool static_provider_claims_through_a_shared_pool` -Expected: FAIL — `BufferProvider` / `StaticBufferProvider` unresolved. - -- [ ] **Step 3: Write the trait and static impl** - -```rust -// src/transport.rs — near the *Pooled traits -use crate::static_channels::{BufferLease, BufferPool}; - -/// Source of `&'static mut [u8]` receive/scratch buffers for the client's -/// socket loops. Mirrors [`ChannelFactory`]'s role for channels: the -/// bare-metal path is backed by a consumer-declared `static BufferPool`; -/// the tokio path is heap-backed and provisioned internally. -pub trait BufferProvider: Clone + Send + Sync + 'static { - /// Claim one buffer, or `None` when the pool is exhausted. - fn claim(&self) -> Option; -} - -/// `BufferProvider` backed by a `'static` [`BufferPool`] (bare-metal path). -#[derive(Clone, Copy, Debug)] -pub struct StaticBufferProvider( - pub &'static BufferPool, -); - -impl BufferProvider - for StaticBufferProvider -{ - fn claim(&self) -> Option { - self.0.claim() - } -} -``` - -```rust -// src/tokio_transport.rs — heap-backed provider -use crate::static_channels::{BufferLease, BufferPool}; -use crate::transport::BufferProvider; -use crate::UDP_BUFFER_SIZE; - -/// Tokio-path buffer provider: a single `Arc`-backed `BufferPool` sized at -/// `UNICAST_SOCKETS_CAP + 1 (discovery) + 1 (release-lag slack)` × -/// `UDP_BUFFER_SIZE`. `Arc`-backed, NOT leaked — the pool is freed when the -/// last provider/lease drops; the API hides it entirely from callers. -/// (Rev: the original sketch used `Box::leak`; the merged implementation is -/// `Arc`-backed so dynamically-created clients don't leak a pool each.) -#[derive(Clone, Debug)] -pub struct TokioBufferProvider(alloc::sync::Arc>); - -impl TokioBufferProvider { - #[must_use] - pub fn new() -> Self { - Self(alloc::sync::Arc::new(BufferPool::new())) - } -} - -impl Default for TokioBufferProvider { - fn default() -> Self { - Self::new() - } -} - -impl BufferProvider for TokioBufferProvider { - fn claim(&self) -> Option { - self.0.claim() - } -} -``` - -- [ ] **Step 4: Run test to verify it passes** - -Run: `cargo test --test buffer_pool static_provider_claims_through_a_shared_pool` -Expected: PASS. - -- [ ] **Step 5: Commit** - -```bash -git add src/transport.rs src/tokio_transport.rs tests/buffer_pool.rs -git commit -m "feat(transport): BufferProvider trait + static/tokio impls (#125)" -``` - ---- - -### Task 3: `socket_loop_future` consumes the buffer; oversize behaviors - -**Files:** -- Modify: `src/client/socket_manager.rs:551` (signature), `:569` (drop local `buf`), `:447-458` (oversize-send check), the receive path (inbound-oversize drop+log) -- Test: `tests/bare_metal_e2e.rs` (a focused inbound-oversize test) - -**Interfaces:** -- Consumes: `BufferLease` (Task 1). -- Produces: `socket_loop_future(socket, rx_tx, tx_rx, e2e_registry, buf: BufferLease)` — the buffer is now an explicit parameter. - -- [ ] **Step 1: Write the failing test** (inbound datagram larger than the claimed buffer is dropped with a log, loop survives) - -```rust -// tests/bare_metal_e2e.rs (new test; reuse the file's existing harness types) -#[tokio::test] -async fn inbound_datagram_larger_than_claimed_buffer_is_dropped_not_fatal() { - // Claim a deliberately tiny 8-byte buffer for the loop, deliver a 64-byte - // datagram, then deliver a valid small one. The oversized datagram must be - // dropped (no panic, loop still running) and the valid one delivered. - let outcome = run_socket_loop_with_buffer_len(8, &[ - Datagram::raw(vec![0xFF; 64]), // oversized -> dropped - Datagram::valid_small(), // must still arrive - ]) - .await; - assert_eq!(outcome.delivered.len(), 1, "only the in-budget datagram is delivered"); - assert!(outcome.loop_alive, "loop must survive an oversized datagram"); -} -``` - -*(If `run_socket_loop_with_buffer_len` / `Datagram` helpers don't exist, add a thin harness in this test module that builds a `SocketManager` over a mock `TransportSocket` whose `recv` yields the scripted datagrams and a `BufferPool<1, 8>` for the lease. Model it on the existing `future_size_witness_bare_metal_channels` setup at `tests/bare_metal_e2e.rs:600`.)* - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e inbound_datagram_larger` -Expected: FAIL — signature mismatch (`socket_loop_future` takes no buffer) / helper missing. - -- [ ] **Step 3: Change the signature and drop the local array** - -```rust -// src/client/socket_manager.rs:551 — add the buffer parameter -#[allow(clippy::too_many_lines)] -async fn socket_loop_future( - socket: T, - rx_tx: C::BoundedSender, Error>, 16>, - mut tx_rx: C::BoundedReceiver, 16>, - e2e_registry: R, - mut buf: crate::static_channels::BufferLease, // ← was `let mut buf = [0u8; UDP_BUFFER_SIZE];` -) where - T: TransportSocket + 'static, - R: E2ERegistryHandle, -{ - const MAX_CONSECUTIVE_RECV_ERRORS: u32 = 16; - let mut consecutive_recv_errors: u32 = 0; - // (delete the old `let mut buf = [0u8; UDP_BUFFER_SIZE];` at :569) -``` - -- [ ] **Step 4: Inbound-oversize drop+log in the receive path** - -In the receive arm (where `socket.recv(&mut buf)` is awaited), the transport already truncates to `buf.len()`; add the guard that a datagram reported larger than `buf.len()` is dropped with a log rather than parsed from a truncated buffer: - -```rust -// receive path, after a successful recv reporting `n` bytes: -let n = match socket.recv(&mut buf).await { - Ok(n) => n, - Err(e) => { /* existing consecutive-error handling, unchanged */ } -}; -if n > buf.len() { - crate::log::warn!( - "inbound datagram ({n} B) exceeds claimed buffer ({} B); dropping", - buf.len() - ); - continue; -} -let datagram = &buf[..n]; -// ... existing parse/forward of `datagram`, unchanged -``` - -- [ ] **Step 5: Oversize-send check keys off `buf.len()`** - -```rust -// src/client/socket_manager.rs:447 — was `if required > UDP_BUFFER_SIZE {` -if required > buf.len() { - warn!( - "outgoing message size {required} exceeds claimed buffer ({}); rejecting with Capacity(\"udp_buffer\")", - buf.len() - ); - return Err(Error::Capacity("udp_buffer")); -} -``` - -For the E2E `protected` scratch at `:632` (`let mut protected = [0u8; UDP_BUFFER_SIZE];`): leave it for **Task 5** (measurement-gated). For now, keep it as a local so this task stays focused on the receive buffer + the two oversize behaviors. - -- [ ] **Step 6: Run the focused test + the full client/server suite** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e inbound_datagram_larger` -Expected: PASS. -Run: `cargo nextest run --no-default-features --features client-tokio,server-tokio` -Expected: all existing client/server tests still green (behavior-preserving except the two allowed changes). - -- [ ] **Step 7: Commit** - -```bash -git add src/client/socket_manager.rs tests/bare_metal_e2e.rs -git commit -m "feat(client): socket loop receives buffer by lease; oversize drop/reject on buf.len() (#125)" -``` - ---- - -### Task 4: Thread `BufferProvider` through deps → bind → spawn - -**Files:** -- Modify: `src/client/mod.rs:278-296` (`ClientDeps` gains a `buffer_provider` + generic `BP`), and the tokio-defaulted constructor -- Modify: `src/client/bind_dispatch.rs:34-117` (`BindDispatch` + `SpawnerDispatch` carry/forward the provider) -- Modify: `src/client/socket_manager.rs:248-249,355-397` (claim a buffer before spawn; pass it into `socket_loop_future`) -- Modify: `src/client/inner.rs` (store the provider; nothing extra at eviction — release is RAII on loop exit) -- Test: `tests/bare_metal_e2e.rs` (bind-to-capacity claims/releases) - -**Interfaces:** -- Consumes: `BufferProvider` (Task 2), `socket_loop_future(.., buf)` (Task 3). -- Produces: `ClientDeps` with field `pub buffer_provider: BP`. - -- [ ] **Step 1: Write the failing test** (binding N unicast sockets claims N buffers; closing a socket releases its buffer) - -```rust -// tests/bare_metal_e2e.rs -#[tokio::test] -async fn each_bound_socket_claims_one_buffer_and_releases_on_close() { - // Pool with exactly 2 slots; provider shared into ClientDeps. - static POOL: simple_someip::static_channels::BufferPool<2, 1500> = - simple_someip::static_channels::BufferPool::new(); - let provider = simple_someip::transport::StaticBufferProvider(&POOL); - - let client = build_test_client_with_buffer_provider(provider).await; - client.bind_unicast_for_test(40000).await.expect("1st bind claims slot 0"); - client.bind_unicast_for_test(40001).await.expect("2nd bind claims slot 1"); - // 3rd bind must fail: pool exhausted. - let third = client.bind_unicast_for_test(40002).await; - assert!(matches!(third, Err(Error::Capacity("udp_buffer")))); - - client.close_unicast_for_test(40000).await; // releases slot 0 - client.bind_unicast_for_test(40002).await.expect("slot freed -> bind succeeds"); -} -``` - -*(`build_test_client_with_buffer_provider` / `bind_unicast_for_test` / `close_unicast_for_test` are thin test shims over `new_with_deps` + the existing control-message API; build them in the test module mirroring `tests/bare_metal_client.rs:257`.)* - -- [ ] **Step 2: Run test to verify it fails** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e each_bound_socket_claims` -Expected: FAIL — `ClientDeps` has no `buffer_provider`. - -- [ ] **Step 3: Add the provider to `ClientDeps`** - -```rust -// src/client/mod.rs:278 — add generic BP and field -pub struct ClientDeps -where - F: TransportFactory, - Tm: Timer, - R: E2ERegistryHandle, - I: InterfaceHandle, - BP: crate::transport::BufferProvider, -{ - pub factory: F, - pub timer: Tm, - pub e2e_registry: R, - pub interface: I, - pub spawner: Sp, - /// Source of `&'static mut [u8]` socket-loop buffers (caller-sized on - /// bare-metal; internally heap-provisioned on the tokio path). - pub buffer_provider: BP, -} -``` - -Update `Client::new_with_deps` and the `Inner` constructor to store `buffer_provider` (alongside `dispatch`/`spawner`). The tokio-defaulted constructor (the phase-21 `ClientDeps`/`Deps` convenience builder) sets `buffer_provider: TokioBufferProvider::new()` so tokio callers are unaffected. - -- [ ] **Step 4: Forward the provider through `bind_dispatch` and claim at bind** - -In `SpawnerDispatch` add a `buffer_provider: BP` field; in its `BindDispatch::bind_unicast`/`bind_discovery` impls (`src/client/bind_dispatch.rs:87-117`), claim a buffer and pass it to the `SocketManager::bind_*` calls. In `SocketManager::bind_with_transport` / `bind_discovery_seeded_with_transport` (`socket_manager.rs:355-397` / `:248`), accept the lease and forward it into the spawned loop: - -```rust -// src/client/bind_dispatch.rs — bind_unicast impl -fn bind_unicast(&self, port: u16, e2e_registry: R) -> impl Future, Error>> + '_ { - async move { - let buf = self - .buffer_provider - .claim() - .ok_or(Error::Capacity("udp_buffer"))?; // pool exhausted -> typed error - SocketManager::::bind_with_transport( - &self.factory, - &self.spawner, - port, - e2e_registry, - buf, // ← moved into the loop - ) - .await - } -} -``` - -```rust -// src/client/socket_manager.rs:389 — pass buf into the spawned future -let fut = Self::socket_loop_future(socket, rx_tx, tx_rx, e2e_registry, buf); -spawner.spawn(fut); -``` - -Release needs **no** new code: the lease is owned by the loop future, so when a socket closes and the loop returns, the future drops, dropping the lease and freeing the slot — the eviction at `inner.rs:639` already removes the handle. (Add a one-line comment there noting the buffer is released via the loop future's drop.) - -- [ ] **Step 5: Run the test + the no-alloc witness** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e each_bound_socket_claims` -Expected: PASS. -Run: `cargo test --features client,bare_metal --test no_alloc_witness` -Expected: PASS — the buffer pool introduces no allocator symbols on the client path. - -- [ ] **Step 6: Commit** - -```bash -git add src/client/mod.rs src/client/bind_dispatch.rs src/client/socket_manager.rs src/client/inner.rs tests/bare_metal_e2e.rs -git commit -m "feat(client): thread BufferProvider through deps/bind; release on loop drop (#125)" -``` - ---- - -### Task 5: E2E scratch buffer — measurement-gated - -**Files:** -- Modify: `src/client/socket_manager.rs:629-632` (the `protected` E2E buffer) -- Reference: `tests/bare_metal_e2e.rs` witnesses - -**Interfaces:** consumes the per-loop receive `BufferLease` (Task 3/4). - -The design doc leaves this as measure-and-decide. Two candidate implementations; pick by the witness numbers. - -- [ ] **Step 1: Capture the current `bm_client_socket_loop` witness number** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e future_size_witness_bare_metal_channels -- --nocapture | grep FUTURE_SIZE` -Record the `bm_client_socket_loop` value (post-Task-4 baseline). - -- [ ] **Step 2: Implement Option A — reuse the loop's receive buffer for E2E protect** - -The receive buffer and the E2E send-scratch are never live at the same instant (receive and send are distinct `select` arms processed one at a time). Reuse the single leased `buf` for protection instead of a second 1500-byte array: - -```rust -// src/client/socket_manager.rs:629 — was `let mut protected = [0u8; UDP_BUFFER_SIZE];` -// Reuse the loop's leased buffer; nothing inbound is pending across a send. -let protected = &mut buf[..]; -let protected_len = e2e_registry.protect(&key, &outgoing, protected)?; // adjust to actual protect() sig -socket.send_to(&protected[..protected_len], dest).await?; -``` - -If `protect()` requires the input and output to be disjoint slices (it may, depending on its signature), fall back to **Option B**: claim a second `BufferLease` from the same provider at send time (`provider.claim()`), use it for `protected`, and let it drop at the end of the send arm. Decide A-vs-B by which keeps `bm_client_socket_loop` smaller in the witness. - -- [ ] **Step 3: Re-measure and verify the suite** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e -- --nocapture | grep FUTURE_SIZE` -Expected: `bm_client_socket_loop` ≤ the Step-1 number (strictly smaller if the second array was the dominant term). -Run: `cargo nextest run --no-default-features --features client-tokio,server-tokio` -Expected: green, including any E2E send test. - -- [ ] **Step 4: Commit** - -```bash -git add src/client/socket_manager.rs -git commit -m "perf(client): eliminate the second E2E scratch buffer from the socket loop (#125)" -``` - ---- - -### Task 6: Flatten `handle_control_message` (secondary, keep only if it moves the number) - -**Files:** -- Modify: `src/client/inner.rs:661-970` (`handle_control_message`), `:1034-1235` (`run_future`) -- Test: existing control-message tests + the run-future witness - -**Interfaces:** introduces a private `enum ControlAction` describing the post-decode work; awaits are hoisted to `run_future`'s top level. - -- [ ] **Step 1: Capture the current `bm_client_run_future` witness number** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e future_size_witness_bare_metal_channels -- --nocapture | grep bm_client_run_future` -Record the value. - -- [ ] **Step 2: Add the action enum and split decode from await** - -Split the 10-variant `match` (`inner.rs:661-970`) into (i) a synchronous `decide_control_action(&mut self, msg) -> ControlAction` that does all the lock/registry mutation and returns a small value, and (ii) shallow `async` helpers invoked from `run_future` for the arms that must await (`bind_*`, `SendToService`, `SendSD`, `Subscribe`, `QueryRebootFlag`): - -```rust -// src/client/inner.rs — new private enum -enum ControlAction { - None, - BindDiscovery(C::OneshotSender>), - BindUnicastThenSend { service_id: u16, instance_id: u16, message: /*…*/, /* + responders */ }, - SendSd { target: SocketAddrV4, header: SdHeader, response: /*…*/ }, - Subscribe { /* the Subscribe fields */ }, - QueryRebootFlag(C::OneshotSender>), - // …one variant per arm that currently awaits -} -``` - -```rust -// run_future loop tail (was `self.handle_control_message().await;` at :1234) -let action = self.decide_control_action(); // synchronous; drops all locals before awaiting -match action { - ControlAction::None => {} - ControlAction::BindDiscovery(resp) => { - let r = self.bind_discovery().await; // shallow, single await - let _ = resp.send(r); - } - ControlAction::BindUnicastThenSend { .. } => { /* hoisted await */ } - // … -} -``` - -The awaited sub-futures still appear in `run_future`'s layout; the win is that the per-variant locals (decoded headers, buffers, responder handles) are no longer held across the awaits, and the variants overlap better. **This task is kept only if Step 4 shows the witness moved.** - -- [ ] **Step 3: Run the full control-message suite (behavior must be identical)** - -Run: `cargo nextest run --no-default-features --features client-tokio,server-tokio` -Expected: all green — every control path (bind/unbind, send, subscribe, reboot-flag query, set-interface) behaves exactly as before. - -- [ ] **Step 4: Re-measure the run-future witness** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e -- --nocapture | grep bm_client_run_future` -Expected: value ≤ Step-1. **If it did not drop, revert this task** (`git checkout -- src/client/inner.rs`) per the "dropped, not merged on faith" constraint, and note it in the PR description. - -- [ ] **Step 5: Commit (only if kept)** - -```bash -git add src/client/inner.rs -git commit -m "perf(client): flatten control-message handler to shrink run_future state (#125)" -``` - ---- - -### Task 7: Tighten witness budgets + rewrite the footprint doc - -**Files:** -- Modify: `tests/bare_metal_e2e.rs:596-598` (budgets) -- Modify: `src/client/mod.rs:1-30` (doc) - -- [ ] **Step 1: Set budgets to the new sizes + 25% headroom** - -Using the post-Task-6 `FUTURE_SIZE` prints, set each budget to `ceil64(measured × 1.25)`: - -```rust -// tests/bare_metal_e2e.rs:596 — replace with the new measured baselines -const BM_CLIENT_RUN_FUTURE_BUDGET: usize = /* ceil64(new bm_client_run_future × 1.25) */; -const BM_CLIENT_SOCKET_LOOP_BUDGET: usize = /* ceil64(new bm_client_socket_loop × 1.25) */; -const BM_SERVER_RUN_FUTURE_BUDGET: usize = 9664; // unchanged — server is PR 3 -``` - -- [ ] **Step 2: Rewrite the memory-footprint doc** (`src/client/mod.rs:1-30`) to describe pooled, caller-sized buffers instead of the old "12 KiB always-live / 24 KiB peak in-future" math: - -```rust -//! SOME/IP client. -//! -//! # Memory footprint -//! -//! The client's `Inner` state is allocated inline. The per-socket -//! `UDP_BUFFER_SIZE` receive buffers are **not** part of the spawned -//! socket-loop futures: each loop claims a `&'static mut [u8]` from a -//! [`BufferProvider`] at bind and releases it when the socket closes. On -//! the bare-metal path the consumer declares the backing `BufferPool` as a -//! `static`, choosing both the slot count and the per-slot length (e.g. -//! 2 × 512 B), so the buffer budget lives in `.bss` and is sized by the -//! caller rather than fixed at `UNICAST_SOCKETS_CAP × UDP_BUFFER_SIZE`. On -//! `std + tokio` the provider is heap-backed and provisioned internally -//! (`UDP_BUFFER_SIZE`-sized slots), invisible to callers. -//! -//! See `docs/simple_someip/plans/2026-06-09-phase22-125-memory-reduction-design.md`. -``` - -- [ ] **Step 3: Verify witnesses pass at the new budgets + docs build** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e` -Expected: PASS at the tightened budgets. -Run: `RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --no-default-features --features client` -Expected: no broken intra-doc links (the `[`BufferProvider`]` link resolves). - -- [ ] **Step 4: Commit** - -```bash -git add tests/bare_metal_e2e.rs src/client/mod.rs -git commit -m "docs+test(client): pooled-buffer footprint doc + tightened future-size budgets (#125)" -``` - ---- - -### Task 8: Full verification + before/after numbers - -**Files:** none (verification + PR description) - -- [ ] **Step 1: Full suite, all shipped feature combos** - -```bash -cargo nextest run --no-default-features --features client-tokio,server-tokio -cargo test --features client,bare_metal --test no_alloc_witness -cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic -cargo clippy -p simple-someip --no-default-features --features client,bare_metal -- -D warnings -D clippy::pedantic -cargo build --target thumbv7em-none-eabihf --no-default-features --features client,bare_metal -``` -Expected: all green; client+bare_metal still alloc-free. - -- [ ] **Step 2: Capture authoritative thumb numbers** (if `tools/capture_type_sizes.sh` from PR 0 exists) - -Run: `tools/capture_type_sizes.sh` -Record the client run-future / socket-loop TaskStorage rows. - -- [ ] **Step 3: Record before/after** in the PR description — the PR-0 baseline vs. the post-PR-2 `FUTURE_SIZE` prints and thumb table, calling out the arena bytes moved to caller `.bss`. - -- [ ] **Step 4: Finish the branch** - -Announce: "I'm using the finishing-a-development-branch skill to complete this work." Then follow superpowers:finishing-a-development-branch — verify the suite, push `feature/pr2_125_client_async_state`, and open the draft PR **based on `feature/pr1_124_followups`** (keep it stacked, never merge-down). - ---- - -## Self-Review - -**Spec coverage (design doc PR-2 section):** -- Buffer extraction via claim/release pool, `&'static mut [u8]`, caller-sized → Tasks 1, 2, 3, 4. ✓ -- Tokio path provisions internally, API unchanged → Task 2 (`TokioBufferProvider`) + Task 4 (defaulted constructor). ✓ -- Inbound-oversize drop+log; oversize-send keyed on `buf.len()` → Task 3. ✓ -- E2E `protected` buffer pooled-or-restructured, measured → Task 5. ✓ -- Handler-tree flattening, kept only if numbers move → Task 6. ✓ -- Doc debt rewrite (`mod.rs:12-30`) → Task 7. ✓ -- Validate against PR-0 numbers; drop changes that don't move it → Steps in Tasks 5, 6 + budget retighten in Task 7. ✓ -- Deferred readiness-split receive → out of scope, recorded in the design doc; not a task here. ✓ - -**Placeholder scan:** Task 5/6 contain measurement-derived values (budgets, the A/B E2E choice) that are *intentionally* resolved at implementation time against live witness output — each has an explicit capture-then-decide step, not a vague "TBD." The `protect()` call shape in Task 5 and the `ControlAction` field lists in Task 6 must be matched to the real signatures in `inner.rs`/the E2E registry when those tasks run; flagged inline. - -**Type consistency:** `BufferPool`/`BufferLease` (Task 1) → `BufferProvider`/`StaticBufferProvider`/`TokioBufferProvider` (Task 2) → `ClientDeps.buffer_provider: BP` (Task 4) → `socket_loop_future(.., buf: BufferLease)` (Task 3) are consistent across tasks. `Error::Capacity("udp_buffer")` is reused verbatim from the existing send-path error (`socket_manager.rs:447`) for the pool-exhaustion case. - -**Risk note carried from the design doc:** the pool hands out `&'static mut [u8]`; Task 1's tests cover claim-to-exhaustion, release-on-drop, and no-double-claim. If `critical-section` is not already satisfiable on the bare-metal build, Task 1 Step 5 adds it to the `bare_metal` feature. diff --git a/docs/simple_someip/plans/2026-06-17-pr3-125-server-buffer-extraction.md b/docs/simple_someip/plans/2026-06-17-pr3-125-server-buffer-extraction.md deleted file mode 100644 index 0c2d9522..00000000 --- a/docs/simple_someip/plans/2026-06-17-pr3-125-server-buffer-extraction.md +++ /dev/null @@ -1,325 +0,0 @@ -# PR 3 — #125 Server Buffer Extraction + Final Numbers Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Move the server's 6 future-resident `[u8; UDP_BUFFER_SIZE]` send-path buffers out of the run/publish futures into caller-provided scratch — matching the server's existing caller-provided *receive* buffer model (`run_with_buffers`) — then capture issue #125's final client+server before/after numbers and close it. - -**Architecture:** The server runs a single combined future (`run_combined` = `recv_loop` + `announce_loop` via `select`) plus an app-driven publish path (`EventPublisher`); it does NOT spawn per-socket loops, so there is no buffer *pool* — buffers are caller-provided fixed scratch (as the receive buffers already are). The SD/Subscribe/offer send helpers and `EventPublisher::publish_*` stop stack-allocating `[u8; UDP_BUFFER_SIZE]` and instead take `&mut [u8]` scratch; the run path is fed two scratch buffers (recv-path + announce-path, which can be mid-send concurrently); the tokio path heap-allocates them internally so the public tokio API is unchanged. - -**Tech Stack:** Rust (edition 2024, crate stays stable-buildable), `heapless`, `embassy-sync`, `tokio` (std path only), cargo, `cargo-nextest`. - -**Spec:** `docs/simple_someip/plans/2026-06-09-phase22-125-memory-reduction-design.md` (PR 3 section — note its `recv_loop`/`announce_loop` *receive* extraction is already done via `run_with_buffers`; the real targets are the send paths below). - -**Base:** stacked on PR 2 (`feature/pr2_125_client_async_state`, tip `6b0aaa3`). - -## Global Constraints - -- **Crate stays stable-buildable**; no nightly-only features. **`server,bare_metal` stays alloc-free** where it is today (the new scratch params must not pull `alloc` into the bare-metal path — tokio-only heap allocation goes behind the `_alloc`/`server-tokio` gate). -- **Wire format untouched.** No change to any byte emitted. -- **Public tokio/std API unchanged** for `Server::new`/`run` and `EventPublisher::publish_*` callers: the tokio path allocates the new scratch internally (mirroring how `run_inner` already heap-allocates the 65535-byte receive buffers). -- **Only behavioral change allowed:** an encode or E2E-protect output that exceeds the *provided scratch length* is rejected with `Error::Capacity("udp_buffer")` (today it's checked against the `UDP_BUFFER_SIZE` constant, which is correct only because the buffers are currently full-size). No other behavioral change. -- **Existing suite green on every commit** (514 nextest `client-tokio,server-tokio`; `bare_metal_e2e` 6/6; no-alloc witness; all clippy + doc gates). -- **Every memory change validated against the `bm_server_run_future` witness** (`tests/bare_metal_e2e.rs`); a change that doesn't move the number is dropped. -- **Exact constants (verbatim):** `UDP_BUFFER_SIZE = 1500` (`src/lib.rs:158`); `BM_SERVER_RUN_FUTURE_BUDGET = 9664` (measured 7696; `tests/bare_metal_e2e.rs:606`). - ---- - -## File Structure - -| File | Action | Responsibility | -|---|---|---| -| `src/server/runtime.rs` | Modify | `send_unicast_offer`/`send_subscribe_ack_from_view`/`send_subscribe_nack_from_view` take `buf: &mut [u8]`; `recv_loop`/`announce_loop`/`run_combined` thread the two send-scratch buffers to them. | -| `src/server/sd_state.rs` | Modify | `SdStateManager::send_offer_service` takes `buf: &mut [u8]`; bounds on `buf.len()`. | -| `src/server/event_publisher.rs` | Modify | `publish_event` takes `msg_buf`/`protected_buf: &mut [u8]`; `publish_raw_event` takes `buf: &mut [u8]`; all encode + E2E bounds on `buf.len()`. | -| `src/server/mod.rs` | Modify | Extend `run_with_buffers` with the two send buffers; `run_inner` (tokio) heap-allocates them; tokio `EventPublisher` wrapper allocates publish scratch internally. | -| `tests/bare_metal_e2e.rs` | Modify | Server send-path regression tests (small-scratch → `Capacity`, not panic) + retighten `BM_SERVER_RUN_FUTURE_BUDGET`. | - ---- - -### Task 1: SD send helpers take caller scratch (`runtime.rs`) - -**Files:** Modify `src/server/runtime.rs` (`send_unicast_offer` ~`:29-78`, `send_subscribe_ack_from_view` ~`:81-129`, `send_subscribe_nack_from_view` ~`:132-182`); Test `tests/bare_metal_e2e.rs`. - -**Interfaces:** -- Produces: each helper gains a leading `buf: &mut [u8]` parameter (replacing its internal `let mut buffer = [0u8; UDP_BUFFER_SIZE]`). - -- [ ] **Step 1: Write the failing test** (a too-small scratch rejects, doesn't panic/OOB) - -```rust -// tests/bare_metal_e2e.rs — drives send_subscribe_ack via the public Subscribe path -// with a deliberately tiny send-scratch buffer; the encoded SD-ACK exceeds it. -#[tokio::test] -async fn server_send_with_undersized_scratch_returns_capacity_not_panic() { - // Harness: build the server with a send-scratch buffer of, say, 24 bytes — - // big enough for the 16-byte header but not the SD-ACK payload — drive a - // Subscribe, and assert the run loop surfaces Capacity("udp_buffer") and - // does NOT panic / OOB. (Model on the existing bare_metal_e2e server harness.) - let outcome = run_server_subscribe_with_send_scratch_len(24).await; - assert!(matches!(outcome, Err(Error::Capacity("udp_buffer")))); -} -``` - -- [ ] **Step 2: Run it — expect FAIL** (helpers don't take a buffer yet / no harness) - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e server_send_with_undersized_scratch` -Expected: FAIL (signature mismatch / harness missing). - -- [ ] **Step 3: Change the three helpers to take `buf` and bound on `buf.len()`** - -Pattern (apply to all three — `send_unicast_offer`, `send_subscribe_ack_from_view`, `send_subscribe_nack_from_view`): - -```rust -// was: fn send_subscribe_ack_from_view(... sd_socket, ...) { let mut buffer = [0u8; UDP_BUFFER_SIZE]; ... } -async fn send_subscribe_ack_from_view( - buf: &mut [u8], // ← caller scratch (was the local array) - /* existing params... */ -) -> Result<(), Error> { - if buf.len() < 16 { - return Err(Error::Capacity("udp_buffer")); - } - let sd_data_len = sd_payload.encode_to_slice(&mut buf[16..])?; // encode_to_slice already errors if the slice is too small; map/propagate to Capacity if it surfaces a different error - let someip_header = SomeIpHeader::new_sd(sid, sd_data_len); - someip_header.encode_to_slice(&mut buf[..16])?; - let total_len = 16 + sd_data_len; - if total_len > buf.len() { // defensive: should already be caught by encode_to_slice - return Err(Error::Capacity("udp_buffer")); - } - sd_socket.send_to(&buf[..total_len], subscriber_v4).await?; - Ok(()) -} -``` -Note: `encode_to_slice` into `&mut buf[16..]` already fails on a too-small slice — verify which error it returns and ensure the helper surfaces `Error::Capacity("udp_buffer")` for the over-capacity case (add the explicit `buf.len()` guards above so the contract is the typed capacity error, not a generic encode error). - -- [ ] **Step 4: Run the test — expect PASS**, then the full server suite. - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e server_send_with_undersized_scratch` → PASS -Run: `cargo nextest run --no-default-features --features client-tokio,server-tokio` → still green. - -- [ ] **Step 5: Commit** - -```bash -git add src/server/runtime.rs tests/bare_metal_e2e.rs -git commit -m "feat(server): SD send helpers take caller scratch, bound on buf.len() (#125)" -``` - ---- - -### Task 2: `SdStateManager::send_offer_service` takes caller scratch (`sd_state.rs`) - -**Files:** Modify `src/server/sd_state.rs:~219-240`. - -**Interfaces:** Consumes nothing new. Produces: `send_offer_service` gains a `buf: &mut [u8]` parameter. - -- [ ] **Step 1: Change the signature and bound on `buf.len()`** (same pattern as Task 1; this is the `announce_loop`'s send path): - -```rust -// was: pub(crate) async fn send_offer_service(&self, config, socket) { let mut buffer = [0u8; UDP_BUFFER_SIZE]; ... } -pub(crate) async fn send_offer_service( - &self, - buf: &mut [u8], // ← caller scratch - config: &ServerConfig, - socket: &impl TransportSocket, -) -> Result<(), Error> { - if buf.len() < 16 { return Err(Error::Capacity("udp_buffer")); } - let sd_data_len = sd_payload.encode_to_slice(&mut buf[16..])?; - let someip_header = SomeIpHeader::new_sd(sid, sd_data_len); - someip_header.encode_to_slice(&mut buf[..16])?; - let total_len = 16 + sd_data_len; - if total_len > buf.len() { return Err(Error::Capacity("udp_buffer")); } - let multicast_addr = SocketAddrV4::new(sd::MULTICAST_IP, sd::MULTICAST_PORT); - socket.send_to(&buf[..total_len], multicast_addr).await?; - Ok(()) -} -``` - -- [ ] **Step 2: Build (callers updated in Task 3) — verify it compiles in isolation by temporarily building after Task 3 wires the caller.** (This task's deliverable is verified together with Task 3, since `send_offer_service`'s only caller is `announce_loop`.) - -- [ ] **Step 3: Commit** (fold into Task 3's commit if cleaner — `send_offer_service` has no standalone caller). - -```bash -git add src/server/sd_state.rs -git commit -m "feat(server): send_offer_service takes caller scratch, bound on buf.len() (#125)" -``` - ---- - -### Task 3: Thread two send-scratch buffers through the run path (`runtime.rs`, `server/mod.rs`) - -**Files:** Modify `src/server/runtime.rs` (`run_combined` ~`:587-642`, `recv_loop` ~`:435-571`, `announce_loop` ~`:397-430`); `src/server/mod.rs` (`run_with_buffers` ~`:1260-1312`, `run_inner` ~`:1403-1453`). - -**Interfaces:** -- Consumes: the `buf`-taking helpers (Tasks 1–2). -- Produces: `Server::run_with_buffers(unicast_buf, sd_buf, recv_send_buf, announce_send_buf: &mut [u8])` — two new send-scratch params. `recv_loop` and `announce_loop` each receive their send-scratch buffer. - -**Why two:** `run_combined` drives `recv_loop` and `announce_loop` concurrently via `select`; both can be suspended at a `send_to().await` at the same time, so a single shared send buffer would alias. `recv_loop` itself handles one inbound message at a time (one send in flight), so it needs exactly one; `announce_loop` needs exactly one. - -- [ ] **Step 1: Extend `run_with_buffers` + `run_combined` + the loops to pass the buffers down** - -```rust -// server/mod.rs — run_with_buffers gains two params -pub fn run_with_buffers<'a>( - &self, - unicast_buf: &'a mut [u8], - sd_buf: &'a mut [u8], - recv_send_buf: &'a mut [u8], // ← new: recv_loop's send scratch - announce_send_buf: &'a mut [u8], // ← new: announce_loop's send scratch -) -> impl core::future::Future> + 'a + use<'a, F, Tm, R, Sub, H, Hsd, Hep> { /* forward all four into run_combined */ } -``` -```rust -// runtime.rs — run_combined forwards recv_send_buf into recv_loop, announce_send_buf into announce_loop; -// recv_loop passes its buffer to send_unicast_offer / send_subscribe_ack_from_view / send_subscribe_nack_from_view; -// announce_loop passes announce_send_buf to sd_state.send_offer_service. -``` - -- [ ] **Step 2: tokio `run_inner` allocates the two send buffers internally** (API unchanged for `Server::run` callers): - -```rust -// server/mod.rs run_inner (tokio) — alongside the existing two recv vecs -let mut unicast_buf = alloc::vec![0u8; 65535]; -let mut sd_buf = alloc::vec![0u8; 65535]; -let mut recv_send_buf = alloc::vec![0u8; crate::UDP_BUFFER_SIZE]; // ← new -let mut announce_send_buf = alloc::vec![0u8; crate::UDP_BUFFER_SIZE]; // ← new -// ...run_with_buffers(&mut unicast_buf, &mut sd_buf, &mut recv_send_buf, &mut announce_send_buf).await -``` - -- [ ] **Step 3: Update bare-metal callers** (`examples/bare_metal_server`, any `run_with_buffers` test caller) to declare and pass the two send buffers. Grep for `run_with_buffers(` and fix each call site. - -- [ ] **Step 4: Verify** — full suite + bare-metal builds: - -Run: `cargo nextest run --no-default-features --features client-tokio,server-tokio` → green -Run: `cargo build --target thumbv7em-none-eabihf --no-default-features --features server,bare_metal` → builds; `cargo build ... --features client,server,bare_metal` → builds -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e` → 6/6 + Task 1's new test - -- [ ] **Step 5: Commit** - -```bash -git add src/server/runtime.rs src/server/sd_state.rs src/server/mod.rs examples/ tests/ -git commit -m "feat(server): thread recv+announce send-scratch buffers through run_with_buffers (#125)" -``` - ---- - -### Task 4: `EventPublisher` publish paths take caller scratch (`event_publisher.rs`, `server/mod.rs`) - -**Files:** Modify `src/server/event_publisher.rs` (`publish_event` ~`:158-218`, `publish_raw_event` ~`:313-344`); `src/server/mod.rs` (tokio publisher wrapper / publish entry). - -**Interfaces:** -- Produces: `publish_event(&self, /* msg */, msg_buf: &mut [u8], protected_buf: &mut [u8])` and `publish_raw_event(&self, /* hdr+payload */, buf: &mut [u8])`. E2E needs `protected_buf` because `E2ERegistry::protect(key, input: &[u8], hdr, output: &mut [u8])` requires disjoint in/out slices. - -- [ ] **Step 1: Write the failing test** (E2E publish on undersized scratch → `Capacity`, mirroring PR 2's client regression): - -```rust -// tests/bare_metal_e2e.rs — register an E2E key, publish an event whose protected -// frame exceeds the provided msg_buf/protected_buf, assert Capacity not panic/OOB. -#[tokio::test] -async fn e2e_publish_with_undersized_scratch_returns_capacity_not_panic() { - let outcome = publish_e2e_event_with_scratch_len(40).await; // 36 fits, +12 P4 protect = 48 > 40 - assert!(matches!(outcome, Err(Error::Capacity("udp_buffer")))); -} -``` - -- [ ] **Step 2: Run it — expect FAIL.** - -- [ ] **Step 3: Change `publish_event` / `publish_raw_event` to take scratch and bound on `buf.len()`** (the E2E guard is the PR-2 lesson applied here — `> msg_buf.len()` / `> protected_buf.len()`, not `> UDP_BUFFER_SIZE`): - -```rust -// publish_event: was two stack arrays; now caller scratch. -pub async fn publish_event(&self, /* message */, msg_buf: &mut [u8], protected_buf: &mut [u8]) -> Result<(), Error> { - let mut message_length = message.encode_to_slice(msg_buf)?; // errors if msg_buf too small - if self.e2e_registry.contains_key(&key) { - let upper_header: [u8; 8] = msg_buf[8..16].try_into().expect("upper header slice"); - let result = self.e2e_registry.protect(key, &msg_buf[16..message_length], upper_header, protected_buf); - if let Some(Ok(protected_len)) = result { - if 16 + protected_len > msg_buf.len() { // ← buf.len(), not UDP_BUFFER_SIZE - return Err(Error::Capacity("udp_buffer")); - } - msg_buf[16..16 + protected_len].copy_from_slice(&protected_buf[..protected_len]); - message_length = 16 + protected_len; - } /* preserve the existing Some(Err)/None arms */ - } - let datagram = &msg_buf[..message_length]; - for addr in &subscribers { /* existing send_to(datagram).await loop, unchanged */ } - Ok(()) -} -``` - -- [ ] **Step 4: tokio publisher keeps the app API ergonomic** — the `server-tokio` publish wrapper allocates `msg_buf`/`protected_buf` (Vecs) internally per publish so existing `publish_event(message)` callers are unchanged; only the bare-metal publish path surfaces the `&mut [u8]` params. Gate the internal allocation behind `_alloc`/`server-tokio`. - -- [ ] **Step 5: Run the test (PASS) + full suite + no-alloc witness.** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e e2e_publish_with_undersized_scratch` → PASS -Run: `cargo nextest run --no-default-features --features client-tokio,server-tokio` → green -Run: `cargo test --features client,bare_metal --test no_alloc_witness` → still alloc-free (publish scratch alloc is tokio-gated) - -- [ ] **Step 6: Commit** - -```bash -git add src/server/event_publisher.rs src/server/mod.rs tests/bare_metal_e2e.rs -git commit -m "feat(server): EventPublisher publish paths take caller scratch; E2E bound on buf.len() (#125)" -``` - ---- - -### Task 5: Measure + retighten the server run-future witness - -**Files:** Modify `tests/bare_metal_e2e.rs:606`. - -- [ ] **Step 1: Capture before/after** - -Run: `cargo test --features client,server,bare_metal --test bare_metal_e2e -- --nocapture | grep bm_server_run_future` -Record the new `bm_server_run_future` (expected to drop from 7696 as the recv-path + announce-path send buffers leave `run_combined`'s state). - -- [ ] **Step 2: Set the budget to `ceil64(new × 1.25)`** - -```rust -// tests/bare_metal_e2e.rs:606 -const BM_SERVER_RUN_FUTURE_BUDGET: usize = /* ceil64(new bm_server_run_future × 1.25) */; -``` - -- [ ] **Step 3: Verify** the witness passes at the tightened budget. If the number did NOT move, the run path's buffers weren't the dominant term — record that and keep the extraction only if it helps (per the binding constraint). - -- [ ] **Step 4: Commit** - -```bash -git add tests/bare_metal_e2e.rs -git commit -m "test(server): retighten server run-future budget after send-buffer extraction (#125)" -``` - ---- - -### Task 6: Final #125 numbers + close-out - -**Files:** none (verification + PR description); optionally `CHANGELOG.md`. - -- [ ] **Step 1: Full gate matrix** - -```bash -cargo nextest run --no-default-features --features client-tokio,server-tokio -cargo test --features client,server,bare_metal --test bare_metal_e2e -cargo test --features client,bare_metal --test no_alloc_witness -cargo clippy --workspace --all-features -- -D warnings -D clippy::pedantic -cargo clippy --no-default-features -- -D warnings -D clippy::pedantic -cargo build --target thumbv7em-none-eabihf --no-default-features --features server,bare_metal -cargo build --target thumbv7em-none-eabihf --no-default-features --features client,server,bare_metal -# nm: server,bare_metal alloc status is documented (server uses alloc today); client,bare_metal stays alloc-free -RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --all-features -cargo test --doc --all-features -``` - -- [ ] **Step 2: Capture authoritative thumb numbers** (if `tools/capture_type_sizes.sh` exists) and assemble issue #125's acceptance table: client (PR 2: socket-loop 2224→776) + server (PR 3: `bm_server_run_future` before→after) TaskStorage sizes + any pool/`nm` symbol deltas. - -- [ ] **Step 3: Record the before/after in the PR description**; note in CHANGELOG that the server run/publish buffers are now caller-sized (breaking the `run_with_buffers` and `publish_*` bare-metal signatures — free pre-0.8.0). - -- [ ] **Step 4: Finish the branch** — announce and use superpowers:finishing-a-development-branch; push `feature/pr3_125_server_buffers` and open the draft PR **based on `feature/pr2_125_client_async_state`** (keep it stacked; never merge-down). - ---- - -## Self-Review - -**Spec coverage:** all 6 future-resident send buffers → Tasks 1 (3 SD helpers), 2 (offer), 4 (publish ×2 incl. E2E `protected`). Threading → Task 3 (run) + Task 4 (publish). E2E `buf.len()` bound → Task 4. Witness retighten → Task 5. Final #125 tables → Task 6. The receive buffers are already caller-provided (no task). ✓ - -**Placeholder scan:** the witness budget (Task 5) and the test harness shims (`run_server_subscribe_with_send_scratch_len`, `publish_e2e_event_with_scratch_len`) are resolved at implementation against the real `bare_metal_e2e` harness; the `encode_to_slice` error-mapping in Tasks 1–2 must be matched to the real return type. Flagged inline. - -**Type consistency:** `buf: &mut [u8]` is the uniform scratch param across Tasks 1/2/4; `run_with_buffers` gains exactly `recv_send_buf`/`announce_send_buf` (Task 3) consumed by the Task 1/2 helpers; `publish_event` gains `msg_buf`/`protected_buf`, `publish_raw_event` gains `buf` (Task 4). `Error::Capacity("udp_buffer")` reused verbatim. - -**Design note for the reviewer:** unlike the client (a `BufferProvider` *pool* for dynamically-spawned per-socket loops), the server uses fixed caller-provided scratch because it runs one combined future + an app publish path — there is no dynamic socket spawning, so claim/release is unnecessary. This is the deliberate, architecture-driven divergence from the client. diff --git a/docs/simple_someip/plans/baselines/pr0-size-baseline.md b/docs/simple_someip/plans/baselines/pr0-size-baseline.md deleted file mode 100644 index 9a721573..00000000 --- a/docs/simple_someip/plans/baselines/pr0-size-baseline.md +++ /dev/null @@ -1,137 +0,0 @@ -# PR 0 size baseline (pre-optimization) - -Captured 2026-06-10 on x86_64-unknown-linux-gnu, before any #125 -optimization work. The "after" tables in PRs 2–3 diff against these -numbers. Regenerate with `tools/capture_type_sizes.sh` (needs nightly -with `rust-src` and `rustup target add thumbv7em-none-eabihf ---toolchain nightly`) and the witness tests: - -```sh -cargo test --features client-tokio,server-tokio,bare_metal future_size_witness -- --nocapture -``` - -(The witnesses are feature-gated; a bare `cargo test future_size_witness` -finds zero tests and exits green.) - -Scope note: the thumb table covers client futures only; the server's -no-alloc probe lands with PR 3 (needs new_with_handles static -plumbing). Server sizes below are host-proxy numbers. - -Payload note: the thumb probe instantiates the client over a -probe-local `ProbePayload` (heapless, fixed-capacity — `RawPayload` -is std-gated), while the host witnesses use `RawPayload`. Thumb and -host rows are therefore NOT comparable layout-for-layout; compare -thumb-to-thumb across captures. Thumb is authoritative for TC4. - -## Toolchains - -- host witnesses: `rustc 1.96.0 (ac68faa20 2026-05-25)` -- thumb capture (`-Zprint-type-sizes`): `rustc 1.96.0-nightly (562dee482 2026-03-21)` - -## Host witness numbers (FUTURE_SIZE lines) - -``` -FUTURE_SIZE tokio_client_run_future 106152 -FUTURE_SIZE tokio_client_socket_loop 6968 -FUTURE_SIZE tokio_server_run_future 7744 -FUTURE_SIZE bm_client_run_future 27208 -FUTURE_SIZE bm_client_socket_loop 2224 -FUTURE_SIZE bm_server_run_future 7696 -``` - -## Capture summary (`target/type-sizes/summary.md`) - -Note: the host-proxy table below comes from compiling only the -`bare_metal_e2e` test target, so it correlates with the `bm_*` -FUTURE_SIZE lines above; the `tokio_*` lines come from lib unit tests -the capture doesn't cover. - -### Type-size capture — rustc 1.96.0-nightly (562dee482 2026-03-21) - -### thumbv7em (authoritative) -| bytes | future | -|---|---| -| 103056 | {async fn body of simple_someip::client::inner::Inner>::run_future()} | -| 12092 | core::mem::MaybeUninit<{async fn body of simple_someip::client::inner::Inner>::handle_control_message()}> | -| 12092 | core::mem::MaybeDangling<{async fn body of simple_someip::client::inner::Inner>::handle_control_message()}> | -| 12092 | core::mem::ManuallyDrop<{async fn body of simple_someip::client::inner::Inner>::handle_control_message()}> | -| 12092 | {async fn body of simple_someip::client::inner::Inner>::handle_control_message()} | -| 5316 | {async fn body of simple_someip::client::socket_manager::SocketManager::socket_loop_future()} | -| 4840 | core::mem::MaybeUninit<{async fn body of simple_someip::client::socket_manager::SocketManager::send()}> | -| 4840 | core::mem::MaybeDangling<{async fn body of simple_someip::client::socket_manager::SocketManager::send()}> | -| 4840 | core::mem::ManuallyDrop<{async fn body of simple_someip::client::socket_manager::SocketManager::send()}> | -| 4840 | {async fn body of simple_someip::client::socket_manager::SocketManager::send()} | -| 2464 | core::mem::MaybeUninit<{async fn body of , simple_someip::client::Error>, 16> as simple_someip::MpscSend, simple_someip::client::Error>>>::send()}> | -| 2464 | core::mem::MaybeDangling<{async fn body of , simple_someip::client::Error>, 16> as simple_someip::MpscSend, simple_someip::client::Error>>>::send()}> | -| 2464 | core::mem::ManuallyDrop<{async fn body of , simple_someip::client::Error>, 16> as simple_someip::MpscSend, simple_someip::client::Error>>>::send()}> | -| 2464 | {async fn body of , simple_someip::client::Error>, 16> as simple_someip::MpscSend, simple_someip::client::Error>>>::send()} | -| 2440 | core::mem::MaybeUninit<{async fn body of , 16> as simple_someip::MpscSend>>::send()}> | -| 2440 | core::mem::MaybeDangling<{async fn body of , 16> as simple_someip::MpscSend>>::send()}> | -| 2440 | core::mem::ManuallyDrop<{async fn body of , 16> as simple_someip::MpscSend>>::send()}> | -| 2440 | {async fn body of , 16> as simple_someip::MpscSend>>::send()} | -| 140 | core::mem::MaybeUninit<{async fn body of simple_someip::client::inner::Inner>::unbind_discovery()}> | -| 140 | core::mem::MaybeDangling<{async fn body of simple_someip::client::inner::Inner>::unbind_discovery()}> | -| 140 | core::mem::ManuallyDrop<{async fn body of simple_someip::client::inner::Inner>::unbind_discovery()}> | -| 140 | {async fn body of simple_someip::client::inner::Inner>::unbind_discovery()} | -| 104 | core::mem::MaybeUninit<{async fn body of simple_someip::client::inner::Inner>::bind_discovery()}> | -| 104 | core::mem::MaybeDangling<{async fn body of simple_someip::client::inner::Inner>::bind_discovery()}> | -| 104 | core::mem::ManuallyDrop<{async fn body of simple_someip::client::inner::Inner>::bind_discovery()}> | -| 104 | {async fn body of simple_someip::client::inner::Inner>::bind_discovery()} | -| 96 | core::mem::MaybeUninit<{async fn body of simple_someip::client::inner::Inner>::bind_unicast()}> | -| 96 | core::mem::MaybeDangling<{async fn body of simple_someip::client::inner::Inner>::bind_unicast()}> | -| 96 | core::mem::ManuallyDrop<{async fn body of simple_someip::client::inner::Inner>::bind_unicast()}> | -| 96 | {async fn body of simple_someip::client::inner::Inner>::bind_unicast()} | -| 92 | core::mem::MaybeUninit<{async fn body of simple_someip::client::socket_manager::SocketManager::bind_discovery_seeded_with_transport()}> | -| 92 | core::mem::MaybeDangling<{async fn body of simple_someip::client::socket_manager::SocketManager::bind_discovery_seeded_with_transport()}> | -| 92 | core::mem::ManuallyDrop<{async fn body of simple_someip::client::socket_manager::SocketManager::bind_discovery_seeded_with_transport()}> | -| 92 | {async fn body of simple_someip::client::socket_manager::SocketManager::bind_discovery_seeded_with_transport()} | -| 80 | core::mem::MaybeUninit<{async fn body of simple_someip::client::socket_manager::SocketManager::bind_with_transport()}> | -| 80 | core::mem::MaybeDangling<{async fn body of simple_someip::client::socket_manager::SocketManager::bind_with_transport()}> | -| 80 | core::mem::ManuallyDrop<{async fn body of simple_someip::client::socket_manager::SocketManager::bind_with_transport()}> | -| 80 | {async fn body of simple_someip::client::socket_manager::SocketManager::bind_with_transport()} | -| 68 | core::mem::MaybeUninit<{async fn body of simple_someip::client::socket_manager::SocketManager::shut_down()}> | -| 68 | core::mem::MaybeDangling<{async fn body of simple_someip::client::socket_manager::SocketManager::shut_down()}> | - -### host x86_64 (proxy) -| bytes | future | -|---|---| -| 35632 | {async block@tests/bare_metal_e2e.rs:604:1: 604:15} | -| 27392 | tokio::runtime::task::core::Cell<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc> | -| 27392 | tokio::runtime::task::core::Cell<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc> | -| 27392 | tokio::runtime::task::core::Cell<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc> | -| 27392 | tokio::runtime::task::core::Cell<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc> | -| 27232 | tokio::runtime::task::core::Core<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc> | -| 27232 | tokio::runtime::task::core::Core<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc> | -| 27224 | {closure@tokio::task::spawn::spawn_inner<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}>::{closure#0}} | -| 27224 | {closure@tokio::runtime::context::current::with_current<{closure@tokio::task::spawn::spawn_inner<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}>::{closure#0}}, tokio::task::JoinHandle<()>>::{closure#0}} | -| 27216 | tokio::runtime::task::core::Stage<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}> | -| 27216 | tokio::runtime::task::core::CoreStage<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}> | -| 27216 | tokio::runtime::task::core::Core<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc> | -| 27216 | tokio::runtime::task::core::Core<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc> | -| 27216 | tokio::loom::std::unsafe_cell::UnsafeCell>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}>> | -| 27216 | std::cell::UnsafeCell>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}>> | -| 27216 | {closure@tokio::runtime::task::core::Core<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc>::set_stage::{closure#0}} | -| 27216 | {closure@tokio::runtime::task::core::Core<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc>::set_stage::{closure#0}} | -| 27208 | std::mem::MaybeUninit<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}> | -| 27208 | std::mem::MaybeDangling<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}> | -| 27208 | std::mem::ManuallyDrop<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}> | -| 27208 | {closure@tokio::task::spawn::spawn_inner<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}>::{closure#0}} | -| 27208 | {closure@tokio::runtime::context::current::with_current<{closure@tokio::task::spawn::spawn_inner<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}>::{closure#0}}, tokio::task::JoinHandle<()>>::{closure#0}} | -| 27208 | {async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()} | -| 27200 | tokio::runtime::task::core::Stage<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}> | -| 27200 | tokio::runtime::task::core::CoreStage<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}> | -| 27200 | tokio::loom::std::unsafe_cell::UnsafeCell>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}>> | -| 27200 | std::cell::UnsafeCell>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}>> | -| 27200 | {closure@tokio::runtime::task::core::Core<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc>::set_stage::{closure#0}} | -| 27200 | {closure@tokio::runtime::task::core::Core<{async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()}, std::sync::Arc>::set_stage::{closure#0}} | -| 27192 | {async fn body of simple_someip::client::inner::Inner>, E2ETestChannels, simple_someip::client::bind_dispatch::SpawnerDispatch>::run_future()} | -| 15616 | tokio::runtime::task::core::Cell<{async block@tests/bare_metal_e2e.rs:487:35: 487:45}, std::sync::Arc> | -| 15616 | tokio::runtime::task::core::Cell<{async block@tests/bare_metal_e2e.rs:487:35: 487:45}, std::sync::Arc> | -| 15424 | tokio::runtime::task::core::Core<{async block@tests/bare_metal_e2e.rs:487:35: 487:45}, std::sync::Arc> | -| 15424 | tokio::runtime::task::core::Core<{async block@tests/bare_metal_e2e.rs:487:35: 487:45}, std::sync::Arc> | -| 15416 | {closure@tokio::task::spawn::spawn_inner<{async block@tests/bare_metal_e2e.rs:487:35: 487:45}>::{closure#0}} | -| 15416 | {closure@tokio::runtime::context::current::with_current<{closure@tokio::task::spawn::spawn_inner<{async block@tests/bare_metal_e2e.rs:487:35: 487:45}>::{closure#0}}, tokio::task::JoinHandle<()>>::{closure#0}} | -| 15408 | tokio::runtime::task::core::Stage<{async block@tests/bare_metal_e2e.rs:487:35: 487:45}> | -| 15408 | tokio::runtime::task::core::CoreStage<{async block@tests/bare_metal_e2e.rs:487:35: 487:45}> | -| 15408 | tokio::loom::std::unsafe_cell::UnsafeCell> | -| 15408 | std::cell::UnsafeCell> | diff --git a/src/client/mod.rs b/src/client/mod.rs index 74a06b6d..5bdeb498 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -34,8 +34,6 @@ //! absolute header minimum: the practical floor is the largest expected //! message (header + payload), realistically one full UDP datagram //! ([`UDP_BUFFER_SIZE`]). -//! -//! See `docs/simple_someip/plans/2026-06-09-phase22-125-memory-reduction-design.md`. mod bind_dispatch; mod error; mod inner; @@ -2337,9 +2335,9 @@ mod tests { /// thumbv7em layouts differ (pointer width/alignment) — the /// authoritative numbers come from `tools/capture_type_sizes.sh`. /// Values are observed-at-capture × 1.25 rounded up to a multiple - /// of 64 (see docs/simple_someip/plans/baselines/pr0-size-baseline.md). + /// of 64. /// If this trips: run the capture script and compare against the - /// baseline before raising the budget — a layout regression in a PR + /// values below before raising the budget — a layout regression in a PR /// is exactly what this witness exists to catch. const TOKIO_CLIENT_RUN_FUTURE_BUDGET: usize = 132736; // = ceil64(106152 × 1.25) /// See [`TOKIO_CLIENT_RUN_FUTURE_BUDGET`] — same proxy-budget rules. diff --git a/src/transport.rs b/src/transport.rs index b2ebefa9..add85525 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -1441,8 +1441,7 @@ impl BufferProvider for StaticBufferProvid /// signatures are implementable without async machinery, (2) /// **layout probing** — `tools/size_probe` instantiates `Client` /// with these on `thumbv7em-none-eabihf` so `-Zprint-type-sizes` -/// reports the real on-target future layouts (see -/// `docs/simple_someip/plans/2026-06-09-phase22-125-memory-reduction-design.md`). +/// reports the real on-target future layouts. /// /// NOT for production use: sockets error, and the spawner panics /// outright — probe code is compiled, never executed, and a loud