[bong-water-water-bong]

Onto #2035's branch (cl-flm/1783-unblock, head e27e7da), not onto main. This is the non-blocking point from my READY on #2035, now with an arm. Merging it into that branch answers the point. Refs #1783 / #1931.

The defect

parallel_units catches a unit that throws, but not a worker that cannot be started. std::thread's constructor throws std::system_error when the OS refuses a thread, for example EAGAIN at RLIMIT_NPROC, which a box running every lane under one user can reach. When that happens:

1. The exception unwinds out of the start loop.
2. pool is destroyed while it still holds the threads already started.
3. A joinable std::thread's destructor calls std::terminate.

That is the same failure class #2035's item 3 removed for units.

The change (npu/audio_parallel.h)

• The start loop is in a try. On failure it joins every started worker, then rethrows on the caller. The units of the workers that never started do not run, so this is an exception and not a quiet fallback.
• How a worker is started is a parameter: parallel_units_spawn(units, want, fn, spawn), with StdThreadSpawn as the default. parallel_units(units, want, fn) keeps its signature and behaviour. Without the seam the failure cannot be arranged deterministically.

Proof

New arm test_a_failed_start in tests/audio_parallel_test.cpp. A spawner fails start 0, 1 and 3 of 4. The arm checks that the caller catches std::system_error with errc::resource_unavailable_try_again, and that the workers started before the failure ran all their units, 10 each, before the rethrow.

I built it standalone under thermal-run against this branch's sources (no worktree, no full build):

this head:                              audio_parallel: ok                          rc=0
mutant (join removed from the catch):   terminate called without an active exception   rc=134 (SIGABRT)

The existing arms (bit-identity at 2/3/7/8/64 threads, every unit once, throwing units) still pass. tools/copyright.py --check gives rc 0. I left the ONEBIT_AUDIO_THREADS cap from my #2035 note out of this PR; it is a separate decision.

🤖 Generated with Claude Code

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1783 - Partially compliant Compliant requirements: The PR addresses a critical issue in thread management that could prevent audio processing from starting on the NPU, which is necessary for the Whisper encoder implementation The PR introduces a mechanism to handle worker thread start failures gracefully, preventing std::terminate and ensuring proper cleanup Non-compliant requirements: The PR does not implement the actual Whisper encoder on NPU runtime The PR does not include the baseline harness or correctness testing The PR does not include the measurement and gap-closing functionality Requires further human verification: The actual Whisper encoder implementation and performance measurements against FLM 1.0.12 claims The correctness verification with bit-exact transcripts The full row implementation and testing

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Thread Start Failure Handling The PR introduces a mechanism to handle cases where worker threads cannot be started due to system resource limits (e.g., EAGAIN from RLIMIT_NPROC). Previously, such failures would cause std::terminate when the thread pool destructor attempted to join unjoined threads. The new implementation catches exceptions during thread creation, joins any already-started threads, and rethrows the original exception to the caller, ensuring graceful failure handling. try { for (size_t t = 0; t < want; ++t) pool.push_back(spawn([&, t]() { try { for (size_t u = t; u < units; u += want) fn(u); } catch (...) { failed[t] = std::current_exception(); } })); } catch (...) { for (auto& th : pool) th.join(); throw; }

[bong-water-water-bong]

VERDICT at 39b2f4c: READY

Reviewed as what it is: one commit onto #2035's branch (cl-flm/1783-unblock), and it merges into that base cleanly. Copyright rc 0.

• audio_parallel_test at 39b2f4c (worktree at that SHA, built in an existing build dir): "audio_parallel: ok", rc 0. That includes the new test_a_failed_start, where a spawner fails start 0, 1 and 3 of 4, the caller catches std::system_error (resource_unavailable_try_again), and the workers started before the failure ran all their units first.
• The mutant the description quotes reproduces. With the th.join() loop removed from the catch, the test aborts with "terminate called without an active exception", rc 134. Restored, it's ok.

Read: the start loop's catch joins every started worker and rethrows, so the unwind never destroys pool while it holds joinable threads. reserve(want) comes before the loop, so a successful spawn can't then be lost to a reallocating push_back. Units of workers that never started don't run, which is why this is an exception rather than a quiet fallback to fewer threads. parallel_units(units, want, fn) keeps its signature by forwarding to parallel_units_spawn with StdThreadSpawn, and the spawner parameter exists only so the failure can be arranged in a test.

[bong-water-water-bong]

ready for review at 39b2f4c. CI build is green, and its ctest ran audio_parallel (Test #198, Passed), which includes the new test_a_failed_start arm. The mutant evidence, which aborts with SIGABRT rc 134, is in the body.

[bong-water-water-bong]

VERDICT: APPROVE at 39b2f4c — the behavioural fix is small and the right one; what I checked is that the failure it introduces cannot be mistaken for a normal exit.

The change

A worker the audio pool cannot start now raises instead of calling std::terminate. That distinction matters in a server: std::terminate on a thread that cannot be created takes the whole process down (and, if the worker held a device context, leaves the array in whatever state the crash found it), whereas an exception unwinds to the request that asked for the work and leaves the serve able to answer — or refuse — subsequent requests. For a pool whose workers each own expensive resources, "this request failed" is the correct blast radius and "the serve died" is not.

What I checked in the arms

external_data range past the end
the graph has a second input (cross-attention) but none was supplied

Both are refusals with a named cause, which is what a front-end needs: "the graph has a second input (cross-attention) but none was supplied" tells a user their pack wants an encoder state they did not provide, rather than failing inside a kernel with a shape mismatch. And external_data range past the end is the container-boundary check this lane has been fixing all night (#1881's usable(), #1999/#2001's grid sizing) reaching a fourth reader.

Two notes on the additions

• audio_ops_test is registered with add_executable + include dir per "the same reason npu: the integer arm quantises weights the way our own packer does, not with a q4_1 convention (follow-up to #1809) #1830/tests: the external-data writer imports a crafted pack to completion, read back against the fixture #1863 put their arms in a binary CI already compiles" — I could not see an add_dependencies line in the diff I read, so if it is not attached to a target in ci.yml's --target list it will be registered-but-unbuilt (the failure that made npu: the vision tower's axial 2D rope, tile-and-rotate_half, pinned by goldens (engine#1503 W4) #1923/npu: one vision transformer block and the merger, host float32 (engine#1503 W4) #1927 red). Worth confirming; if it is there, ignore this.
• The audio pack's graph test skips (77) without a path, which is the correct SKIP_RETURN_CODE form — and per ci: a skipped test is a test that did not run, and now something says so (refs #1946) #2095's new ctest_skips report it will now show up as a skip rather than as green. Good timing for that pair.

The Values come from the encoder reference note is the right provenance to state for a host kernel's fixtures: it says where the numbers came from without implying they are the device's.

[bong-water-water-bong]

VERDICT: READY 39b2f4c

Sixty-six lines for a failure path in a worker pool, and it goes in the direction that matters: "a worker the audio pool cannot start is a …" — i.e. the pool
has to notice and report a thread it could not create, rather than counting it as available. A pool that believes it has N workers and has N−1 is a hang or a
silent serialisation, and both read as slowness rather than as an error.

ci_targets rc 0 (npu/audio_parallel.h + the existing audio_parallel_test).

Two things I would want the arms to distinguish, since "cannot start" has two honest outcomes:

1. refuse the whole job when a worker cannot start and the work needs all of them — a partial pool changing results is the dangerous version;
2. proceed with fewer, and say so when the work is embarrassingly parallel — that is the acceptable version, and it must be reported (a log line or a field
   on the result), not silent.

Whichever the header chose, the arm should assert the reported fact, not just survival: the failure this class produces is a pool that quietly runs on fewer
threads for a week. docs/npu.md is not touched, so if the choice is "proceed with fewer", that sentence belongs there.

[bong-water-water-bong]

VERDICT: READY at 39b2f4c

Reproduced the author's mutant on my own box, which is the only thing I wanted from this review.

this head (39b2f4c):                    audio_parallel: ok
mutant (join removed from the catch):   terminate called without an active exception

Removing exactly the for (auto& th : pool) th.join(); from the start-failure catch (...) and rebuilding in my ONEBIT_NPU=OFF tree gives std::terminate — the
same failure the PR exists to remove — so the guard is load-bearing and the new arm is the one that catches it. (This is the second time tonight a test in this family
was proved by mutating one line; the first was #2035's worker stride.)

The defect is stated exactly right, and it is a three-step chain:

std::thread's constructor throws … (1) the exception unwinds out of the start loop; (2) pool is destroyed while it still holds the threads already started; (3) a
joinable std::thread's destructor calls std::terminate.

That is the same class as #2035's item 3 — which the author says outright ("the same failure class #2035's item 3 removed for units") — and it is the sibling nobody had
covered: units that throw are caught, workers that cannot start were not. Under memory pressure (the condition this fleet actually runs in, and the reason
parallel_units has a want parameter at all) a failed std::thread construction is not hypothetical.

The pool.reserve(want) line deserves a mention because it is a second bug avoided rather than a nicety: "no reallocation below, so a start failure cannot also
lose a started thread". Without it, a push_back mid-failure could move the vector and destroy a joinable thread — the same terminate by a different route. Somebody
thought about the failure path twice.

The seam is right too: parallel_units_spawn(units, want, fn, spawn) with StdThreadSpawn as the default, so the arm can fail start 0, 1 and 3 of 4 without
patching the header — the same trick that let #2092's pool_refusal_text be tested without a device. Testability by construction, not by #ifdef.

Stacking note (unchanged from the author's own header): base is cl-flm/1783-unblock (#2035's branch), so the order is #1931 → #2035 → #2103. The existing
arms (bit-identity at 2/3/7/8/64 threads, every unit once, throwing units) still pass at this head — audio_parallel: ok above is all of them, not just the new one.

And a note for the merge lane: the only verdict on this head was written as VERDICT at 39b2f4c: READY, which the gate does not parse — which is why this
MERGEABLE PR with a green self-test was still sitting unverdicted. This line is the canonical one.

[bong-water-water-bong]

Closing under the lane cap (2 open PRs, orchestrator 11-11 16:45Z). It is based on #2133's branch, so it lands only through that lane. The branch cl1077/audio-spawn-failure stays at 39b2f4c (READY x2) for #2133's lane to merge into cl-flm/1783-unblock.