Repository navigation
fix(core): make array_evaluated_bytes fallible so GPU faults don't abort #1791
Description
Problem
array_evaluated_bytes is declared without Result in the cxx bridge, so an MLX throw inside it is an uncatchable std::terminate. At the current MLX pin it does throw on a failed command buffer: since ml-explore/mlx#3742 every event a failed buffer signals carries one shared encoder error, and array::eval() rethrows it through Event::check_error(). The production caller is the lookahead decode path, so in mlxcel-server one failed command buffer kills the process and every in-flight request with it.
The fault does not have to be mlxcel's. A command buffer discarded because another process's fault forced a GPU recovery (kIOGPUCommandBufferCallbackErrorInnocentVictim) now throws at this pin where it used to be swallowed, and it aborted the mlxcel-core test binary with SIGABRT during the PR #1777 re-gate. Found during review of PR #1772 (MLX pin bump to 81ba1c6a).
Evidence
- Declaration
src/lib/mlxcel-core/src/lib.rs:194(fn array_evaluated_bytes(arr: &MlxArray) -> Vec<u8>;) andsrc/lib/mlxcel-core/cpp/mlx_cxx_bridge.h:210; bodysrc/lib/mlxcel-core/cpp/mlx_cxx_bridge.cpp:549-576(a.eval()at:555; its non-contiguous fallback to the equally infalliblearray_to_raw_bytesis at:564). - Production caller
lookahead_tokens_to_host,src/server/batch/scheduler/mod.rs:771, used bypipelined_steady_decodeatsrc/server/batch/scheduler/decode_tick.rs:633. - Test callers:
src/models/falcon_ocr_rope_tests.rs(four sites),src/loading/vlm_falcon_ocr_tests.rs:20,tests/falcon_ocr_parity.rs:144. - PR fix(mlx): bump MLX pin to 81ba1c6a so mxfp8 block scales round up #1772 fixed the same class for stashed sampler launches with
stashed_launch_state(mlx_cxx_bridge.cpp:5585), which reads status, event validity,is_signaled()andload_error()without throwing.
Proposed fix
Declare it -> Result<Vec<u8>>, so the cxx try/catch also covers the fallback copy, and make lookahead_tokens_to_host return a Result. On Err, pipelined_steady_decode takes the #822 path the other decode evals already use (decode_tick.rs:884-900): record_eval_outcome (src/server/batch/scheduler/handoff.rs:48), the same lookahead teardown as its finishing branch (decode_tick.rs:654-671), abort_sequence_with_error (src/server/batch/scheduler/run_loop.rs:102) for each row in la.ids, then eval_failures_exhausted(). Update the test callers.
Acceptance criteria
- No non-
Resultbridge call remains on the lookahead read path. - A test builds an array whose event is signalled and carries an error (a test-only hook modelled on
sampling_dispatch_stash_failed_launch_for_test,mlx_cxx_bridge.cpp:5679), passes it toarray_evaluated_bytes, and assertsErr. - A lookahead read failure fails only the rows of that step; the scheduler keeps serving other requests.
Verification
The workspace gate: cargo test --workspace --profile test-fast --features metal,accelerate, cargo clippy --workspace --all-targets -- -D warnings, cargo fmt --all -- --check. With the bridge signature reverted to -> Vec<u8>, the new test must abort the test binary (SIGABRT) instead of passing.
Activity
Merge origin/main into fix/issue-1791-fallible-array-read
docs: add technical reports for PR #2284
fix(core): make array readback fallible (lablup#2284)