Skip to content

deps(core)(deps): update bincode requirement from 1 to 3 in /src/lib/mlxcel-core - #10

Closed
dependabot[bot] wants to merge 1 commit into
mainfrom
dependabot/cargo/src/lib/mlxcel-core/bincode-3
Closed

dependabot[bot] wants to merge 1 commit into
mainfrom
dependabot/cargo/src/lib/mlxcel-core/bincode-3

Conversation

@dependabot

@dependabot dependabot Bot commented on behalf of github May 18, 2026 •

Copy link
Copy Markdown
Contributor

Updates the requirements on bincode to permit the latest version.

Commits

@dependabot dependabot Bot added area:core mlxcel-core: MLX FFI, primitives, KV cache, layers type:dependency Dependency updates labels May 18, 2026
Updates the requirements on [bincode](https://github.com/bincode-org/bincode) to permit the latest version.
- [Commits](https://github.com/bincode-org/bincode/commits)

---
updated-dependencies:
- dependency-name: bincode
  dependency-version: 3.0.0
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
@dependabot
dependabot Bot force-pushed the dependabot/cargo/src/lib/mlxcel-core/bincode-3 branch from 5a659c2 to 2f14fa3 Compare May 18, 2026 08:29
@inureyes

Copy link
Copy Markdown
Member

Closing — superseded by the postcard migration scheduled for #8.

The mlxcel team has decided to migrate mlxcel-core off bincode
entirely (to postcard 1.x) per the architectural choice in #8,
rather than upgrading to bincode 3.x. Reasons:

  • bincode 1.x → 3.x is not a drop-in upgrade — the API changes
    significantly, and the 2.x / 3.x project is maintained by a
    different team than the original (1.x). Same crate name, different
    project history; carries name-confusion risk.
  • postcard is a stable 1.0+ serde-based alternative with a frozen
    wire format and a much wider embedded-Rust ecosystem footprint,
    which is a better fit for an inference-runtime cache.
  • Once refactor: migrate mlxcel-core off unmaintained bincode 1.x #8 lands, bincode is removed from the dependency tree
    entirely, and the RUSTSEC-2025-0141 ignore in deny.toml is
    removed as part of that PR.

So this auto-generated security update is no longer applicable.

@inureyes inureyes closed this May 18, 2026
@dependabot @github

dependabot Bot commented on behalf of github May 18, 2026

Copy link
Copy Markdown
Contributor Author

OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting @dependabot ignore this major version or @dependabot ignore this minor version. You can also ignore all major, minor, or patch releases for a dependency by adding an ignore condition with the desired update_types to your config file.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

@dependabot
dependabot Bot deleted the dependabot/cargo/src/lib/mlxcel-core/bincode-3 branch May 18, 2026 08:44
inureyes added a commit that referenced this pull request May 18, 2026
…16)

Closes #8.

Migrates `mlxcel-core`'s `TokenLanguageIndex` on-disk cache off the
unmaintained `bincode` 1.x (RUSTSEC-2025-0141 — project permanently
ceased development) to `postcard` 1.x.

## Architectural choice — postcard

- Drop-in serde-based API (`postcard::to_allocvec(&v)?` /
  `postcard::from_bytes::<T>(&bytes)?`) — minimal call-site change.
- 1.0+ stable with frozen wire format — protects production users'
  cache compatibility across future dependency upgrades.
- Embedded Rust ecosystem standard — natural fit for an
  inference-runtime cache.

Alternatives considered: `bitcode` (faster but pre-1.0, wire-format
changes between minors), `rkyv` (zero-copy over-engineering for a
startup-time cache), `bincode` 2.x / 3.x (different maintainer team,
name-confusion risk — Dependabot's auto-PR #10 to bincode 3.x was
explicitly closed in favor of this approach).

## Cache compatibility — graceful degradation

`lang_analyzer/cache.rs` already has corrupt-detection: a
deserialization failure renames the existing bincode-format file to
`*.broken.<epoch>.bak` and rebuilds the cache from source. Users
upgrading from a prior mlxcel release will trip this path
automatically on the first cache read — no explicit migration code
needed.

## Scope (3 files + Cargo.lock)

- `src/lib/mlxcel-core/Cargo.toml` — `bincode = "1"` → `postcard = { version = "1", features = ["alloc"] }`
- `src/lib/mlxcel-core/src/lang_analyzer/cache.rs` — 3 call sites + doc comments
- `src/lib/mlxcel-core/src/lang_analyzer/mod.rs` — error variant `Bincode` → `Postcard` (no external match-arms on it, safe rename)
- `deny.toml` — RUSTSEC-2025-0141 `[advisories.ignore]` entry removed (no longer applicable)

Verified locally: `cargo deny check` clean (advisories ok, bans ok,
licenses ok, sources ok); `cargo tree -p mlxcel-core -i bincode`
returns "package not found" — bincode is fully removed from the
dependency tree.

Self-hosted clippy + test gate is queued behind the PR #14 cold-build
backlog at merge time; will run on main after the runner clears.
@inureyes inureyes self-assigned this May 18, 2026
@inureyes inureyes added the status:done Completed label May 18, 2026
@inureyes inureyes added this to the 0.1 milestone Jun 21, 2026
be-free-snowboy pushed a commit to freestylejs/snowbrain-mlxcel that referenced this pull request Oct 27, 2026
…ablup#16)

Closes lablup#8.

Migrates `mlxcel-core`'s `TokenLanguageIndex` on-disk cache off the
unmaintained `bincode` 1.x (RUSTSEC-2025-0141 — project permanently
ceased development) to `postcard` 1.x.

## Architectural choice — postcard

- Drop-in serde-based API (`postcard::to_allocvec(&v)?` /
  `postcard::from_bytes::<T>(&bytes)?`) — minimal call-site change.
- 1.0+ stable with frozen wire format — protects production users'
  cache compatibility across future dependency upgrades.
- Embedded Rust ecosystem standard — natural fit for an
  inference-runtime cache.

Alternatives considered: `bitcode` (faster but pre-1.0, wire-format
changes between minors), `rkyv` (zero-copy over-engineering for a
startup-time cache), `bincode` 2.x / 3.x (different maintainer team,
name-confusion risk — Dependabot's auto-PR lablup#10 to bincode 3.x was
explicitly closed in favor of this approach).

## Cache compatibility — graceful degradation

`lang_analyzer/cache.rs` already has corrupt-detection: a
deserialization failure renames the existing bincode-format file to
`*.broken.<epoch>.bak` and rebuilds the cache from source. Users
upgrading from a prior mlxcel release will trip this path
automatically on the first cache read — no explicit migration code
needed.

## Scope (3 files + Cargo.lock)

- `src/lib/mlxcel-core/Cargo.toml` — `bincode = "1"` → `postcard = { version = "1", features = ["alloc"] }`
- `src/lib/mlxcel-core/src/lang_analyzer/cache.rs` — 3 call sites + doc comments
- `src/lib/mlxcel-core/src/lang_analyzer/mod.rs` — error variant `Bincode` → `Postcard` (no external match-arms on it, safe rename)
- `deny.toml` — RUSTSEC-2025-0141 `[advisories.ignore]` entry removed (no longer applicable)

Verified locally: `cargo deny check` clean (advisories ok, bans ok,
licenses ok, sources ok); `cargo tree -p mlxcel-core -i bincode`
returns "package not found" — bincode is fully removed from the
dependency tree.

Self-hosted clippy + test gate is queued behind the PR lablup#14 cold-build
backlog at merge time; will run on main after the runner clears.
inureyes added a commit that referenced this pull request Oct 30, 2026
## Confirmed mechanism

The abort is a `ModelProvider` worker thread freeing its model after `main` has returned. Reproduced on this box (kernel 7.0.0, same as the failing runner image) with the CI configuration (debug profile, no `cuda` feature, `MLXCEL_CI_PP_MODEL=qwen3-0.6b-4bit`); gdb with a breakpoint on `__cxa_pure_virtual` caught it on 3 of 3 attempts. Aborting thread (frames condensed):

```
#0  __cxa_pure_virtual ()
#1  mlx::core::allocator::Data::~Data  at mlx/allocator.h:79        (allocator().free(buffer))
#11 mlx::core::array::ArrayDesc::~ArrayDesc  at mlx/array.cpp:333
#22 mlx_cxx::MlxArray::~MlxArray  at cpp/mlx_cxx_bridge.h:27
#38 drop_glue<mlxcel_core::cache::paged::PagedBlockPool>
#49 spawn_model_worker_with_batch_config::{closure#0}::{closure#0}  at src/server/model_worker.rs:893
#55 mlxcel::worker_failfast::run_core_thread_or_abort
```

Main thread at the same instant:

```
#6  blas_thread_shutdown_ / #8 gotoblas_quit   (libopenblas finalizer)
#9  _dl_call_fini / #10 _dl_fini
#11 __run_exit_handlers / #12 exit
```

The worker (still joined by nobody: `Drop for ModelProvider` only sends `ModelRequest::Shutdown`, and its `JoinHandle` sits in the `observe_worker_exit` watcher thread, which gdb shows blocked in `pthread_join` on it) is dropping the scheduler's `PagedBlockPool`. Each array free goes through `allocator().free()`. On the CPU backend `allocator()` is `static CommonAllocator allocator_` in `mlx/backend/no_gpu/allocator.cpp`, a function-local static whose destructor runs from `__run_exit_handlers`; after it, the object's vtable is the abstract `Allocator`'s and `free` is pure virtual. The Metal, CUDA and ROCm allocators are heap-leaked on purpose and never reach this state, which is why only the CPU CI build aborts. The kernel change in the runner image only altered the scheduling that exposes the window; the test never joined the two providers.

## Fix

- `ModelProvider::shutdown_and_wait(timeout)` (`src/server/model_provider.rs`): sends the shutdown signal, waits on the existing `WorkerExitObserver`, returns `Err("server model worker did not exit within 120s")` on expiry (the duration is printed with `Debug`, so the production text is unchanged and a sub-second test timeout reads `500ms`, not `0s`) and `Err("server model worker panicked: ...")` on a panic. Idempotent; an exited worker and a closed channel return `Ok`. `WORKER_EXIT_TIMEOUT` (120 s) moves here as `pub` and is re-exported from `mlxcel::server` so the integration test can use it. `InProcessServer::shutdown` delegates to it. `Drop` stays non-blocking.
- `tests/pipeline_ci_multi_stage_real_models.rs`: the shared helper calls `shutdown_and_wait(WORKER_EXIT_TIMEOUT)` on the coordinator provider, then the dense one, and `expect`s both before shutting the stage handles down. `tests/pipeline_server_remote_real_models.rs` now does the same instead of dropping its two providers (CUDA and Metal only today, where the allocators are leaked, but exposed on a CPU runner).
- Unit tests in `src/server/model_provider_tests.rs`: cooperative worker (`Ok`, observed, second call `Ok`), already-exited worker (`Ok` with a zero timeout), worker panicking during teardown (`Err` carrying the message), worker ignoring shutdown (`Err` after a 1 s timeout, then `Ok` once released).

## Production exposure (per path)

- `mlxcel-server` single-model mode (`start_server`): affected. `serve_http` returning (bind or TLS error) and the five side-model resolution errors after the chat worker started loading all returned with the worker alive. Fixed: everything after the provider is created moved into the function `serve_with_model_provider`, whose result flows through `join_model_worker_before_exit`. That logs the serve error at error level first (`server stopped: ...; waiting for the model worker to exit`), so a bind or TLS failure during a long load is not a silent wait, then calls `shutdown_and_wait` on the blocking pool and logs `model worker exited before process exit`. The serve error is returned unchanged for `main` to print, and a shutdown failure never replaces it. SIGINT/SIGTERM in this mode still die by default disposition (no exit handlers run), unchanged. The audio, embedding, image and rerank side workers are trait objects without an exit observer and are not joined here; the same hazard class applies to them on the CPU backend and is tracked in #2745 (with the sleeping-worker reload on shutdown, the uncancellable load past the join timeout, and the `--pp-peer` signal path).
- Router mode (`run_router_server`): partly affected. The Ctrl-C arm already drained through `shutdown_all`, whose `unload` waits on each worker's exit observer (and the entry operation guard makes it wait for an in-flight `build_model_app` too). The `serve_http` error arm returned without draining, so a bind failure after `load-on-startup` exited under loading workers. Fixed: both arms drain before returning. The listener error is logged before the drain starts, and a Ctrl-C handler that cannot be installed now drains before its error is returned instead of skipping `shutdown_all`. Residual: a worker that ignores shutdown past `ROUTER_SHUTDOWN_TIMEOUT` (150 s) still exits with handlers running; the alternative is hanging.
- `--pp-peer` stage (`serve_remote_pipeline_stage`): not affected. `RemoteStageServiceHandle::shutdown` joins the stage thread, which owns the stage model and its current-thread runtime.
- `mlxcel run` / `chat`: not affected. They already join through `InProcessServer::shutdown` (#2224), which now shares the implementation.

`tests/server_exit_joins_worker.rs` drives a real `mlxcel-server` into the bind-failure exit (the test holds the port) for the single-model and the router `load-on-startup` paths and asserts exit code 1, no signal, no C++ abort text, and the join log line. The router test matches `router: lifecycle shutdown completed attempted=1 completed=1`, since a failed `begin_load` prints the same message with `attempted=0`. Both skip without the `qwen3-0.6b-4bit` checkpoint.

## Before / after

Command, on the debug CPU binary exactly as CI runs it:

```
BIN=target/debug/deps/pipeline_ci_multi_stage_real_models-<hash>
for i in $(seq 50); do MLXCEL_CI_PP_MODEL=qwen3-0.6b-4bit "$BIN" pipeline_multi_stage_two_host_logical_smoke --ignored --nocapture --test-threads=1; done
```

- Before the fix (origin/main 8385f40): `aborts: 44/50` (every abort log carries `pure virtual method called`; the other 6 runs exited cleanly)
- After the fix: `aborts: 0/50` (same binary rebuilt from this branch, same box, loops overlapped in time)

## Test plan

- [x] `cargo test --profile test-fast --features cuda -p mlxcel --lib server::model_provider:: -- --test-threads=1`
- [x] `cargo test --profile test-fast --features cuda -p mlxcel --lib server::in_process:: -- --test-threads=1`
- [x] `tests/server_exit_joins_worker.rs` (both tests, real checkpoint, `MLXCEL_REQUIRE_MODELS=1`)
- [x] the smoke test on the debug CPU binary, 50 consecutive runs
- [x] `cargo check --profile test-fast --features cuda,test-utils -p mlxcel --test pipeline_server_remote_real_models`
- [x] `cargo clippy --profile test-fast --features cuda -p mlxcel --lib --tests -- -D warnings`, `cargo fmt --all -- --check`

## CI stability

Ten consecutive `workflow_dispatch` runs of `pipeline-parallel-ci.yml` on this branch at head `03c1e365`, dispatched one at a time (the concurrency group cancels overlapping runs on the same ref). All ten passed, and all ten ran on `ubuntu24/20261025.353`, the runner image every one of the original aborts ran on. No job log contains `pure virtual method called`.

| # | Run | Head | Result | Runner image |
|---|---|---|---|---|
| 1 | [41045436759](https://github.com/lablup/mlxcel/actions/runs/41045436759) | `03c1e365` | success | `ubuntu24/20261025.353` |
| 2 | [41048002710](https://github.com/lablup/mlxcel/actions/runs/41048002710) | `03c1e365` | success | `ubuntu24/20261025.353` |
| 3 | [41049748107](https://github.com/lablup/mlxcel/actions/runs/41049748107) | `03c1e365` | success | `ubuntu24/20261025.353` |
| 4 | [41051499873](https://github.com/lablup/mlxcel/actions/runs/41051499873) | `03c1e365` | success | `ubuntu24/20261025.353` |
| 5 | [41053271228](https://github.com/lablup/mlxcel/actions/runs/41053271228) | `03c1e365` | success | `ubuntu24/20261025.353` |
| 6 | [41055089949](https://github.com/lablup/mlxcel/actions/runs/41055089949) | `03c1e365` | success | `ubuntu24/20261025.353` |
| 7 | [41056817489](https://github.com/lablup/mlxcel/actions/runs/41056817489) | `03c1e365` | success | `ubuntu24/20261025.353` |
| 8 | [41058570988](https://github.com/lablup/mlxcel/actions/runs/41058570988) | `03c1e365` | success | `ubuntu24/20261025.353` |
| 9 | [41060493226](https://github.com/lablup/mlxcel/actions/runs/41060493226) | `03c1e365` | success | `ubuntu24/20261025.353` |
| 10 | [41062232317](https://github.com/lablup/mlxcel/actions/runs/41062232317) | `03c1e365` | success | `ubuntu24/20261025.353` |

The two `pull_request` runs of the same workflow on this PR also passed on `ubuntu24/20261025.353`: [41024288311](https://github.com/lablup/mlxcel/actions/runs/41024288311) at `e4312b89` and [41045135070](https://github.com/lablup/mlxcel/actions/runs/41045135070) at `03c1e365`.

Closes #2738
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core mlxcel-core: MLX FFI, primitives, KV cache, layers status:done Completed type:dependency Dependency updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant