Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions TECHNICAL_REPORTS/2284-fallible-array-read-20261013.en.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# PR #2284: Propagate MLX array readback failures without aborting

**Date**: 2026-10-13
**Status**: Implemented, independently reviewed, and focused tests passed
**Risk**: Medium (FFI error propagation and speculative scheduler cleanup)

## Summary

PR #2284 makes `array_evaluated_bytes` fallible across the C++/Rust cxx boundary. A failed MLX event now reaches engine and scheduler callers as a normal error instead of allowing a C++ exception to abort the process. Lookahead decoding drains an already-submitted next evaluation, restores speculative row positions, and aborts only unfinished rows in the affected cohort. Unrelated active requests continue and failed rows are not replayed.

## Problem

The previous bridge returned `Vec<u8>` directly. If synchronizing an MLX array surfaced an event error, the C++ exception crossed an infallible cxx boundary and terminated the process. The scheduler also needed a single cleanup path for failures discovered while collecting lookahead tokens: speculative state may include one append for the current lookahead and a second append for a submitted next prime.

## Design

- The cxx declaration now returns `Result<Vec<u8>>`; the C++ implementation keeps its native return type so the generated wrapper translates C++ exceptions into Rust errors.
- Engine token readback returns `Result<Vec<i32>, String>`, including fused-row and prompt-lookup callers.
- Lookahead failure cleanup evaluates any submitted `next.tokens` through the raw fallible path before trimming state, records the primary failure once, restores the correct one- or two-step speculative positions, removes lookaheads, and aborts only unfinished affected requests.
- A C++ fixture attaches a real signalled MLX `Event` error to an evaluated array. Its error object intentionally remains alive for the process because MLX stores a raw pointer. Direct FFI and engine tests consume this event exactly once.
- Scheduler coverage uses a thread-local one-shot readback fault at the collection boundary. Reusing the failed-event fixture as lookahead input would consume its one-shot event error earlier, while the next decoding step is built, and would not exercise scheduler readback cleanup. The seam preserves a valid submitted next step and injects the error at the boundary under test.

## Verification

The source commit is `9622b8c6`. The publication candidate merges base `a1c58265` at local head `d9a8c4ea`; its tree is `67d54aee7acc63c90a5395fda7673c6bc72a680f`. The reviewed 16-path binary diff against that base retains SHA-256 `60e33dd052fdb4405f281377c5875b80700573fdf0146c8bec95d9edd38489e7`.

- `cargo test --locked -p mlxcel-core -p mlxcel --lib --profile test-fast --features cuda -j4 --no-run`: passed. Log: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/build-no-run-restored-final.log`.
- Real failed-event FFI test: 1 passed. Log: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/ffi-failed-event-test-restored-final.log`.
- Engine failed-event propagation test: 1 passed. Log: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/engine-failed-event-test-restored-final.log`.
- Scheduler cleanup with and without a submitted next prime: 1 passed. It also verifies one health failure, restored model-owned offsets, no pending-token emission, no shutdown, and an unrelated survivor completing. Log: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/scheduler-read-failure-test-restored-final.log`.
- `cargo fmt --all -- --check`: passed. Log: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/fmt-check.log`.
- Refreshed repository contract gate: passed after merging current `main`, including version, kernel dtype-key, kernel port-dispatch, llama compatibility, ROCm overlay, 123 Python tooling tests, and 49 WebUI fixtures. Log: `/tmp/mlxcel-auto-20261012T1250/logs/contracts-1791-a1c-refreshed.log`.
- Negative control: the old infallible bridge was compiled in isolation and the direct FFI test ran in a child process. The child exited with `-6`, exactly `-SIGABRT`, proving the regression would abort without this change. Logs: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/negative-old-signature-status.log`, `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/negative-old-signature-child.log`, and `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/negative-old-signature-build.log`.

The focused test binaries built from source commit `9622b8c6`, before the subsequent `main` refreshes that did not touch the 16 owned paths, were `mlxcel_core-be0ee8003b018a01` with SHA-256 `79ae35f3b958baab057da86d3abf2cc85058706a30840b2647cdb440e5dc1413` and `mlxcel-d5e5498c5ad868df` with SHA-256 `e9f19800ae9efc50ffaf2e2f96986a4123b8f601809ae621a76903c19288690a`.

## Review and limits

Independent implementation, security/performance, and static finalization reviews found no remaining issue in scope. The first issue contract gate passed before the final scheduler test-only fault seam was added; the refreshed gate above passed on the final test seam after merging current `main`. CI and the full integration checks remain with the orchestration gate before central merge. Broad workspace tests and Clippy were intentionally left to that gate.
39 changes: 39 additions & 0 deletions TECHNICAL_REPORTS/2284-fallible-array-read-20261013.ko.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# PR #2284: 프로세스를 중단하지 않고 MLX 배열 readback 오류 전파

**날짜**: 2026-10-13
**상태**: 구현 및 독립 리뷰 완료, 집중 테스트 통과
**위험도**: 중간 (FFI 오류 전파와 speculative scheduler cleanup 변경)

## 요약

PR #2284는 C++/Rust cxx 경계의 `array_evaluated_bytes`를 fallible하게 변경합니다. 이제 실패한 MLX event는 C++ 예외로 프로세스를 중단시키지 않고 engine과 scheduler 호출자에게 일반 오류로 전달됩니다. Lookahead decoding은 이미 제출된 다음 evaluation을 drain하고 speculative row position을 복구한 뒤, 영향을 받은 cohort에서 아직 완료되지 않은 row만 중단합니다. 관련 없는 활성 요청은 계속 처리하고 실패한 row를 replay하지 않습니다.

## 문제

기존 bridge는 `Vec<u8>`를 직접 반환했습니다. MLX 배열을 synchronize할 때 event 오류가 발생하면 C++ 예외가 infallible cxx 경계를 넘어 프로세스를 종료했습니다. Scheduler에도 lookahead token 수집 중 발견한 실패를 일관되게 처리하는 cleanup 경로가 필요했습니다. Speculative state에는 현재 lookahead append 하나와 이미 제출한 next prime의 append 하나가 추가로 들어 있을 수 있습니다.

## 설계

- cxx 선언은 이제 `Result<Vec<u8>>`를 반환합니다. C++ 구현의 native 반환형은 유지해 생성된 wrapper가 C++ 예외를 Rust 오류로 변환하게 합니다.
- Engine token readback은 fused-row와 prompt-lookup 호출자를 포함해 `Result<Vec<i32>, String>`을 반환합니다.
- Lookahead 실패 cleanup은 제출된 `next.tokens`가 있으면 상태를 trim하기 전에 raw fallible 경로로 평가하고, 주 실패를 한 번만 기록하며, speculative position을 한 단계 또는 두 단계 되돌립니다. 그 뒤 lookahead를 제거하고 영향을 받은 요청 중 아직 완료되지 않은 요청만 중단합니다.
- C++ fixture는 실제 오류가 signal된 MLX `Event`를 평가 완료 배열에 연결합니다. MLX가 raw pointer를 저장하므로 오류 객체는 의도적으로 프로세스 수명 동안 유지합니다. Direct FFI와 engine 테스트는 이 event를 정확히 한 번 소비합니다.
- Scheduler 검증은 수집 경계에서 thread-local one-shot readback fault를 사용합니다. 실패 event fixture를 lookahead 입력으로 그대로 사용하면 다음 decoding step을 만드는 더 이른 시점에 one-shot event 오류를 소비해 scheduler readback cleanup을 검증하지 못합니다. 이 seam은 제출된 next step을 유효하게 유지하면서 검증 대상 경계에서 오류를 주입합니다.

## 검증

소스 커밋은 `9622b8c6`입니다. 게시 후보는 base `a1c58265`를 병합한 local head `d9a8c4ea`이며 tree는 `67d54aee7acc63c90a5395fda7673c6bc72a680f`입니다. 이 base를 기준으로 리뷰한 16개 경로의 binary diff SHA-256은 그대로 `60e33dd052fdb4405f281377c5875b80700573fdf0146c8bec95d9edd38489e7`입니다.

- `cargo test --locked -p mlxcel-core -p mlxcel --lib --profile test-fast --features cuda -j4 --no-run`: 통과. 로그: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/build-no-run-restored-final.log`.
- 실제 failed-event FFI 테스트: 1개 통과. 로그: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/ffi-failed-event-test-restored-final.log`.
- Engine failed-event 전파 테스트: 1개 통과. 로그: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/engine-failed-event-test-restored-final.log`.
- 제출된 next prime이 있는 경우와 없는 경우의 scheduler cleanup: 1개 통과. Health failure가 한 번만 기록되는지, model-owned offset이 복원되는지, pending token이 방출되지 않는지, 서버가 종료되지 않는지, 관련 없는 survivor가 완료되는지도 함께 확인했습니다. 로그: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/scheduler-read-failure-test-restored-final.log`.
- `cargo fmt --all -- --check`: 통과. 로그: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/fmt-check.log`.
- 최신 repository contract gate: 현재 `main`을 병합한 뒤 통과했습니다. Version, kernel dtype-key, kernel port-dispatch, llama 호환성, ROCm overlay, Python tooling 테스트 123개, WebUI fixture 49개를 포함합니다. 로그: `/tmp/mlxcel-auto-20261012T1250/logs/contracts-1791-a1c-refreshed.log`.
- Negative control: 기존 infallible bridge를 격리해 컴파일하고 direct FFI 테스트를 child process에서 실행했습니다. Child가 정확히 `-SIGABRT`인 `-6`으로 종료되어, 이 변경이 없으면 회귀가 프로세스를 중단시킨다는 점을 확인했습니다. 로그: `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/negative-old-signature-status.log`, `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/negative-old-signature-child.log`, `/tmp/mlxcel-auto-20261012T1250/logs/issue-1791/negative-old-signature-build.log`.

16개 소유 경로를 건드리지 않은 후속 `main` 갱신들을 병합하기 전 source commit `9622b8c6`에서 만든 집중 테스트 binary는 SHA-256 `79ae35f3b958baab057da86d3abf2cc85058706a30840b2647cdb440e5dc1413`인 `mlxcel_core-be0ee8003b018a01`과 SHA-256 `e9f19800ae9efc50ffaf2e2f96986a4123b8f601809ae621a76903c19288690a`인 `mlxcel-d5e5498c5ad868df`입니다.

## 리뷰와 제한 사항

독립 구현 리뷰, 보안·성능 리뷰, static finalization 리뷰에서 이 범위에 남은 문제를 발견하지 못했습니다. 첫 번째 이슈 contract gate는 최종 scheduler test-only fault seam을 추가하기 전에 통과했고, 위의 최신 gate는 그 seam과 현재 `main` 병합을 포함한 상태에서도 통과했습니다. CI와 전체 통합 검사는 central merge 전 orchestration gate가 수행합니다. 광범위한 workspace 테스트와 Clippy도 해당 gate에서 수행하도록 의도적으로 남겼습니다.
25 changes: 25 additions & 0 deletions src/lib/mlxcel-core/cpp/mlx_cxx_bridge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,31 @@ rust::Vec<uint8_t> array_evaluated_bytes(const MlxArray& arr) {
return result;
}

std::unique_ptr<MlxArray> array_with_failed_event_for_test() {
auto stream = mlx::core::default_stream(mlx::core::default_device());
mlx::core::Event event(stream);
event.set_value(1);
event.signal(stream);
// Land the real signal before attaching an error: synchronize checks an
// event's error, so doing this afterwards would consume the fixture.
mlx::core::synchronize(stream);

// Event stores a raw Error pointer. This fixture deliberately gives each
// returned array an independent process-lifetime error so one consuming
// read cannot invalidate another test's event.
auto* error = new mlx::core::Error();
error->set_message(std::make_shared<std::string>(
"mlxcel test: synthetic array read failure"));
event.set_error(*error);

const uint32_t zero = 0;
auto result = std::make_unique<MlxArray>(
mlx::core::array(&zero, mlx::core::Shape{1}, mlx::core::uint32));
result->inner.set_status(mlx::core::array::Status::evaluated);
result->inner.attach_event(event);
return result;
}

// Same body as `array_to_raw_bytes`, but declared `-> Result<Vec<u8>>` on the
// Rust side so cxx wraps the call in a try/catch and converts any thrown MLX
// exception (an allocation failure making the contiguous copy, or any deferred
Expand Down
4 changes: 4 additions & 0 deletions src/lib/mlxcel-core/cpp/mlx_cxx_bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@ rust::Vec<uint8_t> try_array_to_raw_bytes(const MlxArray& arr);
// `array_to_raw_bytes` contiguous copy rather than reading past the allocation.
rust::Vec<uint8_t> array_evaluated_bytes(const MlxArray& arr);

// Test-only. Return an evaluated uint32 array whose signalled event carries a
// synthetic backend error. The error outlives the returned event.
std::unique_ptr<MlxArray> array_with_failed_event_for_test();

// Evaluation.
void eval(const MlxArray& arr);
// Fallible counterpart of `eval`; declared `-> Result<()>` in the bridge so cxx
Expand Down
27 changes: 27 additions & 0 deletions src/lib/mlxcel-core/src/array_evaluated_bytes_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2025-2026 Lablup Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use super::*;

/// The fixture's event error is consumable, so this must call the newly
/// fallible bridge directly without probing availability or evaluating first.
#[test]
fn failed_event_readback_returns_an_error() {
let array = array_with_failed_event_for_test();
let err = array_evaluated_bytes(&array).expect_err("failed event must cross as Result::Err");
assert!(
err.to_string().contains("synthetic array read failure"),
"unexpected bridge error: {err}"
);
}
11 changes: 11 additions & 0 deletions src/lib/mlxcel-core/src/engine/eval_fault_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,17 @@ fn readback_helpers_return_errors() {
);
}

#[test]
fn engine_token_readback_propagates_a_failed_event() {
// Use a fresh fixture: Event::check_error consumes its attached error.
let tokens = crate::array_with_failed_event_for_test();
let err = try_tokens_to_host(&tokens).expect_err("failed event must reach the engine");
assert!(
err.contains("synthetic array read failure"),
"unexpected engine readback error: {err}"
);
}

#[test]
fn failed_readbacks_unwind_to_the_committed_prefix() {
for sampling in [
Expand Down
15 changes: 9 additions & 6 deletions src/lib/mlxcel-core/src/engine/rows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,10 @@ pub fn sample_and_finish<H: StepRowHooks>(
if let Err(err) = crate::try_eval(&tokens) {
return fail_batch(rows, &err.to_string());
}
let host = tokens_to_host(&tokens);
let host = match tokens_to_host(&tokens) {
Ok(host) => host,
Err(err) => return fail_batch(rows, &err),
};
if host.len() != rows.len() {
// A short readback would let `zip` drop rows whose KV was
// appended but which are never finished or advanced.
Expand Down Expand Up @@ -383,11 +386,12 @@ pub fn row_biases<'r, H: StepRowHooks>(rows: &'r [StepRow<'_, H>]) -> Vec<&'r To
/// next forward is already scheduled on the same stream before this read,
/// and a fresh `contiguous()` op would queue behind it and collapse the
/// overlap. This reader waits only on the token array's own completion.
pub fn tokens_to_host(tokens: &MlxArray) -> Vec<i32> {
ffi::array_evaluated_bytes(tokens)
pub fn tokens_to_host(tokens: &MlxArray) -> Result<Vec<i32>, String> {
Ok(ffi::array_evaluated_bytes(tokens)
.map_err(|err| err.to_string())?
.chunks_exact(4)
.map(|c| i32::from_ne_bytes([c[0], c[1], c[2], c[3]]))
.collect()
.collect())
}

/// `logits` sliced to a `[1, 1, vocab]` row, the shape the per-row chain
Expand All @@ -399,8 +403,7 @@ pub fn row_logits(logits: &MlxArray, i: usize) -> UniquePtr<MlxArray> {
/// Wait through the fallible boundary before the overlap-preserving byte read.
pub fn try_tokens_to_host(tokens: &MlxArray) -> Result<Vec<i32>, String> {
crate::eval_faults::check(crate::eval_faults::FaultPoint::Readback)?;
crate::try_eval(tokens).map_err(|e| e.to_string())?;
Ok(tokens_to_host(tokens))
tokens_to_host(tokens)
}

/// Read a per-row token without letting an asynchronous backend error escape.
Expand Down
2 changes: 1 addition & 1 deletion src/lib/mlxcel-core/src/engine/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ fn submit_then_finish_rows_is_one_pipelined_step() {
let biases: Vec<&crate::sampling::TokenBiasMap> =
rows.iter().map(|r| &r.sampling.token_bias).collect();
let tokens = engine.submit(&batch, &params, &biases).unwrap();
let host = tokens_to_host(&tokens);
let host = tokens_to_host(&tokens).expect("test token readback must succeed");
assert_eq!(host, vec![2, 7]);
// The speculative appends are in place until the caller decides.
assert_eq!(seq_len(&engine, a), 1);
Expand Down
11 changes: 10 additions & 1 deletion src/lib/mlxcel-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,12 @@ mod ffi {
/// `fused_sample` output); a non-contiguous array falls back to the safe
/// `array_to_raw_bytes` copy rather than reading past the allocation.
/// Used by: the #632 lookahead decode pipeline.
fn array_evaluated_bytes(arr: &MlxArray) -> Vec<u8>;
fn array_evaluated_bytes(arr: &MlxArray) -> Result<Vec<u8>>;

/// Test-only. Build an evaluated uint32 array whose real, signalled
/// event carries a synthetic backend error. Each call owns an
/// independent error so tests can exercise consuming read boundaries.
fn array_with_failed_event_for_test() -> UniquePtr<MlxArray>;

/// Fallible counterpart of [`array_to_raw_bytes`].
///
Expand Down Expand Up @@ -4059,6 +4064,10 @@ pub(crate) mod test_support;
#[path = "ffi_tests.rs"]
mod ffi_tests;

#[cfg(test)]
#[path = "array_evaluated_bytes_tests.rs"]
mod array_evaluated_bytes_tests;

// Affine 1-bit quantization (issue #1370): fused Metal qmv / qmm parity against
// a host f64 oracle, the dequantize reference, load validation and layer
// routing.
Expand Down
1 change: 1 addition & 0 deletions src/loading/vlm_falcon_ocr_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use super::*;

fn read(array: &mlxcel_core::MlxArray) -> Vec<f32> {
mlxcel_core::array_evaluated_bytes(array)
.expect("test tensor readback must succeed")
.chunks_exact(4)
.map(|c| f32::from_le_bytes([c[0], c[1], c[2], c[3]]))
.collect()
Expand Down
Loading
Loading