fix(trie): isolate SHARED_COMMITTER init from the global rayon pool

[flyq]

Summary

Fixes issue #146. cargo test --features test-bucket-resize could permanently deadlock at high libtest concurrency (32-core hosts, default --test-threads). The hang is not the spin-waiter starvation hypothesized in the issue but a work-stealing re-entrancy deadlock in SHARED_COMMITTER's one-time initialization; the same convoy also affects production cold start (bounded, all cores pegged until init completes).

Root cause

SHARED_COMMITTER's initializer runs Committer::new, whose table build is a par_iter over 256 bases on the global rayon pool (banderwagon/src/salt_committer.rs:165 via the iter! macro). Rayon workers blocked at a join point steal other pending jobs on top of their stack. StateRoot::rebuild's parallel closures call StateRoot::new — which dereferences SHARED_COMMITTER — from inside pool workers, so a worker executing an init subtask could steal such a job; that job then waits for the very initialization frozen beneath it on the same stack. Circular wait, no progress ever. spin::Lazy (introduced by the no_std change #120, replacing once_cell) additionally makes every waiter busy-spin at 100% CPU, which produced the GDB signature in the issue, but the cycle exists with any lazy primitive — bounding --test-threads only shrinks the race window.

Fix

Two independent changes, plus a warm-up:

• Init isolated from the global pool (salt/src/trie/trie.rs:61): the table build runs in a dedicated short-lived pool, driven from a fresh OS thread that the initializer joins. The join matters: a global-pool worker blocking on another pool via install() steals from its own pool while waiting (in_worker_cross) and can re-enter the lazy on its own stack — an intermediate version of this fix deadlocked exactly there, caught by the new regression test. This isolation alone makes first touch safe from any thread, including pool workers. Pool/thread creation failure aborts the process with a diagnostic (abort_init, salt/src/trie/trie.rs:91) rather than panicking or falling back to the caller's pool: an initializer panic would poison the LazyLock so every later deref fails without the original cause, and an inline fallback would reopen both deadlock channels. The diagnostic goes through let _ = writeln!(stderr, …) rather than eprintln!, which panics on an unwritable stderr and would reinstate that poisoning path.
• Parking waiters under std (salt/src/lib.rs:14): Lazy<T> is std::sync::LazyLock under std and spin::Lazy only for no_std targets, where parallel (and therefore rayon) is off, so the initializer never fans out onto a pool and always completes on its own thread. PRECOMPUTED_WEIGHTS (salt/src/proof/prover.rs:42) switched likewise; its initializer is sequential, so parking alone suffices there.
• Warm-up before rebuild's parallel region (salt/src/trie/trie.rs:1007): not required for correctness — the isolation above already makes in-pool first touch safe — but it keeps the first wave of chunk jobs from all parking on the one-time initialization.

Testing

Two single-test regression binaries, one per deadlock channel, sharing a run_guarded helper (salt/tests/common/mod.rs) that runs the test body on a worker thread and fails the test via recv_timeout if it does not finish within 120 s — a hang surfaces as a normal libtest panic with a diagnostic message.

• salt/tests/shared_committer_init_os_winner.rs — original channel: OS thread wins init, its parallel build reaches the global pool, flood jobs get stolen by workers helping the build. Deadlocks pre-fix main 5/5 on a 14-core host (and the final harness was re-validated against pre-fix main: reports a clean FAILED after 120 s); passes 30+ consecutive runs with the fix.
• salt/tests/shared_committer_init.rs — initiator-steal channel: first touch inside pool jobs racing OS threads. Caught the broken intermediate version of this fix; passes 60+ consecutive runs with the fix.

Full matrix green: the issue's repro config (NUM_DATA_BUCKETS=2 BUCKET_RESIZE_LOAD_FACTOR_PCT=1, 196 passed / 2 ignored after merging main), default features, --no-default-features (± test-bucket-resize), riscv64imac-unknown-none-elf no_std check, fmt, clippy.

Notes

• The issue's suggested --test-threads=4 CI mitigation is unnecessary after this fix, and switching to a parking lazy alone would not have fixed the deadlock — the cycle is independent of how waiters wait.
• Bundled churn: mutants/suppressions.toml is touched twice to re-pin four line-anchored timeout suppressions for create_node_aligned_chunks — the new code above that function shifts its lines, and the pins are deliberate (they disambiguate same-text mutant twins), so every edit above it must re-pin. Verified with cargo mutants --list + mutation_gate.py orphans: all 38 suppressions match.
• Visible API nuance: PRECOMPUTED_WEIGHTS is pub, so its type changes from spin::Lazy to std::sync::LazyLock under std. Deref-only usage (all known consumers, incl. stateless-validator) is unaffected.
• Pre-existing, unrelated: enable-hugepages + parallel does not compile on Linux (banderwagon/src/salt_committer.rs:107, missing IntoParallelRefIterator import; combination not covered by CI). Worth a follow-up issue.
• Production cold start previously risked N−1 threads busy-spinning through a ~150 ms–1.3 s init on first block validation; with parking waiters plus the dedicated pool this becomes a clean one-time wait.
• The diff-scoped mutation gate cannot structurally exercise the threading fix (cargo-mutants has no "remove the thread wrapper" mutant; the only viable in-diff mutant is killed by existing root assertions) — the regression binaries are the coverage for the fix itself, and they run under the mutation harness with a timeout below the harness minimum, so a reintroduced deadlock reports as a caught failure.

🤖 Generated with Claude Code

[github-actions]

Performance Benchmark Comparison

Compared 5 benchmark(s) against the latest main baseline.

Detailed Comparison

Benchmark	Baseline Throughput (Kelem/s)	New Throughput (Kelem/s)	Change
update 10000 KVs/1 threads	73.06	79.00	+8.14%
update 10000 KVs/2 threads	139.15	145.66	+4.68%
update 10000 KVs/4 threads	258.45	269.66	+4.34%
update 10000 KVs/8 threads	432.99	454.76	+5.03%
update 10000 KVs/16 threads	579.69	603.57	+4.12%

[github-actions]

Mutation testing - PASS

Nothing to test: no viable mutants were generated (2 unviable, 0 timed out).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 061e7b30d5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[mega-putin]

Independent A/B validation on the same 32-logical-CPU x86_64 host where issue #146 was naturally reproduced: PASS.

Revision / configuration	Result
Control (19419f4, before this fix), ordinary cargo test	3/5 hung; each classified by the 300 s watchdog
Control (19419f4, before this fix), NUM_DATA_BUCKETS=2 resize configuration	3/5 hung; each classified by the 300 s watchdog
PR head (ff8442f), ordinary cargo test	15/15 terminated normally
PR head (ff8442f), NUM_DATA_BUCKETS=2 resize configuration	15/15 terminated normally

All A/B runs used default libtest concurrency, with no --test-threads restriction.

The two new dedicated regression-test binaries also passed repeated execution:

• shared_committer_init: 50/50 PASS
• shared_committer_init_os_winner: 50/50 PASS

For first-initialization cost, 20 alternating fresh-process control/PR pairs produced a paired median delta of +0.669%, below the predeclared practical-significance threshold; no perceptible regression was observed.

Each trial ran in an independent process group. Timed-out trials were cleaned up with SIGTERM followed by SIGKILL where needed, with zero residual PIDs. The complete raw record set (298 subprocess records plus a SHA-256 manifest) has been archived and is available on request.

[Troublor]

Fix verified across both deadlock channels (dedicated-pool isolation + parking waiters), and the abort-on-spawn-failure follow-up in ce6cf2e resolves the LazyLock-poisoning concern. Remaining notes are non-blocking hardening: prefer a non-panicking let _ = writeln!(stderr, ...) before abort() (eprintln! panics on an EPIPE stderr and would reinstate the poisoning path under panic=unwind), and the lib.rs alias comment's "no_std targets have no threads" wording is refuted by the no-default-features CI job on hosted runners. Happy to see these picked up in a follow-up.