Repository navigation
test(cpu): fix flaky Windows ARM64 crash in SPMD int4 parity test - #111
Merged
Merged
Conversation
The `spmd_real_multi_op_int4_is_bit_identical_at_odd_worker_count` test hard-coded DECODE_THREADS=31 and RAYON_NUM_THREADS=31 for both parity children. On the native Windows ARM64 CI runner this grossly oversubscribed the constrained host and intermittently faulted the whole test binary with STATUS_ACCESS_VIOLATION (0xC0000005) — a flaky, environment-level crash, not a correctness bug: ThreadSanitizer finds no data race and macOS arm64 / Windows x64 / Linux never reproduce it. The 31 was also non-deterministic: the effective count is min(31, available_parallelism), so the advertised "odd worker count" invariant actually depended on the runner's core-count parity. Replace 31 with `parity_worker_count()`: the largest ODD count that never exceeds the host CPU count, capped at 15. Because it is <= available_parallelism() it is never clamped down, so the odd-worker sharding coverage is deterministic per host and the children no longer oversubscribe. The ON child now self-verifies (on single-node hosts) that the pool actually ran the requested odd count. The real byte-identical SPMD-vs-flat parity assertion still runs on every platform across all 6 sequential int4 ops — coverage is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #111 +/- ##
=======================================
Coverage 82.47% 82.48%
=======================================
Files 231 231
Lines 99194 99212 +18
Branches 99194 99212 +18
=======================================
+ Hits 81815 81837 +22
+ Misses 14008 14005 -3
+ Partials 3371 3370 -1
🚀 New features to boost your workflow:
|
justinchuby
added a commit
that referenced
this pull request
Jul 24, 2026
…ws ARM64 flaky (#117) ## Summary Fixes a Windows ARM64 CI flaky (hard subprocess crash, `STATUS_ACCESS_VIOLATION` / empty stderr) in the persistent-pool affinity tests. Same class as #111. ## Root cause `run_affinity_defer_child` (test harness in `matmul_nbits.rs`) hard-coded `ONNX_GENAI_CPU_DECODE_THREADS=8` and `RAYON_NUM_THREADS=8` for its subprocess children. The `forced_off` scenario forces the persistent SPMD pool, which spins up ~8 busy-waiting SPMD workers (plus a Rayon pool) **inside an already-spawned child test binary**. On the low-core native Windows ARM64 runner this oversubscribes CPUs and hard-aborts the child before it can print — surfacing as `forced_persistent_pool_ignores_explicit_affinity` failing with truncated stdout and empty stderr. It intermittently fails any ep-cpu PR (hit #115 twice in a row). ## Fix Replace the hard-coded `"8"` with the existing `parity_worker_count()` helper (largest odd ≤ `available_parallelism()`, capped at 15, always ≥ 1) — the exact remedy #111 applied to a sibling test. **Test-harness only; no product/kernel code changed.** ## Why the test stays meaningful - `forced_off` still forces the pool (`PERSISTENT_POOL=1`) and still asserts `pools().is_some()` (`parity_worker_count() ≥ 1` ⇒ a real pool is built). - `auto_*` children still set explicit affinity and assert `pools().is_none()`. - No `#[ignore]`, no weakened assertion, no OS/arch skip — only the thread count is bounded to a runner-safe value. ## Sibling tests Two other forced-pool subprocess tests use small hard-coded counts (MLAS-shard 3/3, MLAS-prefill 3/6) — below the oversubscription threshold and not reported flaky; left unchanged to preserve the prefill test's deliberate `RAYON > workers` row-block invariant. ## Gates (Linux x86_64) - `cargo test -p onnx-runtime-ep-cpu --features mlas` — 903 pass; no-mlas pass; clippy clean both; `cargo fmt --check` clean (only known `mlas-sys:1211` false positive); aarch64 cross-check clean. Reviewed independently 🟡 APPROVE-WITH-NITS (non-author). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 22, 2026
…lane child (#1778) ## A documentation-only PR was failed by a memory fault `decode_spmd::tests::an_explicit_budget_buys_its_full_width_end_to_end` spawns a child test binary per budget. On native Windows ARM64 that child intermittently faults during SPMD pool build/teardown with `STATUS_ACCESS_VIOLATION` (`0xC0000005`) and **completely empty stderr** — no Rust panic, none of our assertions. It killed `Rust (Windows ARM64)` on #1772, which changes **only Markdown**. That is the clearest possible demonstration that no amount of correct code prevents it: ``` budget 4 child failed (exit code: 0xc0000005): stdout: running 1 test test decode_spmd::tests::budget_lane_child ... stderr: onnx-genai: CPU decode budget 4 bounded the global Rayon pool (prefill/MLAS parallelism capped at 4 workers) ``` The child got as far as bounding the pool, then vanished mid-test. 1520 passed, 1 failed, and the one failure was a hard fault in a child process. ## Why this spawner was unprotected `matmul_nbits.rs` already rides through exactly this class for its own child spawners — #111 first, and again in #1747. But `is_environmental_access_violation_crash` was private to that test module. This spawner was added later, in a different file, and never got the same protection. The guard existed; it just was not reachable. Rather than duplicate a four-condition signature test, this moves `is_environmental_access_violation_crash`, `child_status_detail` and `STATUS_ACCESS_VIOLATION` into the crate's existing `#[cfg(test)] test_support` module and has both callers share them. **The moved code is unmodified** — no behaviour change for the `matmul_nbits` callers. ## The retry cannot mask a real failure It fires only when *all* of these hold: the child was unsuccessful, **and** emitted no completion token, **and** printed no `panicked at` / `assertion` text, **and** exited with exactly that NTSTATUS. A genuine assertion failure still fails on the first attempt. The signature never occurs on Linux, so behaviour there is unchanged. ## The one piece of genuinely new logic, and its falsifier The lane child differs from the existing callers in having **two** completion tokens — a result and a skip — and either one means it finished its work, so a fault after either is not the environmental crash. The shared classifier takes a single marker, so `lane_child_crash_is_environmental` requires it to agree across both. That composition is new, so it gets its own test. `lane_child_retry_covers_only_the_environmental_crash` pins every non-signature exit as non-retryable: a real panic with the AV code, a success, exit code 1, no exit code, each completion token on its own, and both interleaved with `--nocapture` harness output. It is mutation-proved, not merely present. Changing `.all` to `.any` in the helper fails it: ``` assertion failed: !lane_child_crash_is_environmental(false, AV, &result, "") ``` So "either token suppresses the retry" is actually tested, and a real regression can never be retried into a false pass. ## Scope Test-only. No production path, kernel, numerics or dispatch change. `decode_spmd.rs` is touched inside `mod tests` only. ## Validation Base `f38a4f0c9`. Full 21-gate matrix **21/21, 0 fail** — Linux, `--features mlas`, no-default-features, no-MLAS-artifact, aarch64 under QEMU, Windows ARM64 source cfg. * `onnx-runtime-ep-cpu` lib suite: **1638 passed, 0 failed**. * Both classifier tests confirmed **executed** (not filtered) in the default, `mlas`, aarch64-QEMU and no-default lanes. * `cargo fmt` clean, `clippy --all-targets -D warnings` clean. cc @sebastian — `decode_spmd.rs` is yours; this is confined to its test module and adds no assertion about decode behaviour. Happy to hand it over if you would rather own the change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the flaky Windows ARM64 CI crash in
spmd_real_multi_op_int4_is_bit_identical_at_odd_worker_count(introduced by #105). Test-only change — no production/kernel code touched.Root cause
The parity test hard-coded
DECODE_THREADS=31+RAYON_NUM_THREADS=31for both child subprocesses. On the constrained native Windows ARM64 runner this oversubscribed the host and intermittently hard-crashed the child with0xC0000005 STATUS_ACCESS_VIOLATION(empty stderr, truncated stdout — a hard abort, not a Rust panic or a byte mismatch). Same main SHA both passed and failed → flaky. macOS arm64 and Windows x64 always passed.Ruled out (with evidence): not an aarch64 kernel bug (
selected_dot_kernel()returns the safe-RustScalarpath on aarch64 — nounsafe, no NEON/SVE), not a data race (full suite TSan-clean at 31 workers), not a numeric/token divergence.Also fixed a determinism defect: effective worker count was
min(31, cores), so "odd worker count" actually depended on the runner's core-count parity.Fix
New
parity_worker_count()= largest odd integer ≤ host CPUs, capped at 15. Never 0, never exceeds host CPUs (no oversubscription), deterministic (depends only onavailable_parallelism()). The ON child self-verifies the odd count; the byte-identical SPMD-vs-flat parity assertion still runs on every platform across all 6 sequential int4 ops. Coverage preserved.Review
🟢 APPROVED by Drake (non-author). Independently confirmed: test-only, always-odd/≥1/≤CPUs, deterministic, parity assertion runs on all platforms and is non-vacuous (byte-XOR perturbation → FAILED → revert → passed), SPMD routing asserted (≥6 dispatches). Non-blocking nit: on 1–2 CPU hosts the uneven-sharding sub-invariant degrades silently (byte-parity + routing asserts still run); optional follow-up to emit a skip-notice.
Gates (x86_64)
cargo test -p onnx-runtime-ep-cpu(default, NO mlas — the failing CI config): 859 passed; parity test 5×: 5/5 no crashcargo test -p onnx-runtime-ep-cpu --features mlas: 888 passed--features mlas(-D warnings): cleancargo fmt --check: clean (the sole mlas-sys:1211 diff is pre-existing/unrelated, a local rustfmt-version artifact)RUSTFLAGS="-D warnings" cargo check --target aarch64-unknown-linux-gnu: cleanCo-authored-by: Copilot 223556219+Copilot@users.noreply.github.com