From 57d5e57f784d7d4c93218bb899a473181beac815 Mon Sep 17 00:00:00 2001 From: epicsagas Date: Thu, 2 Jul 2026 16:04:25 +0900 Subject: [PATCH 1/2] fix(embedding): drop ort-load-dynamic and make LazyFastembed panic-safe (#50) - Stop enabling ort-load-dynamic on the Linux/Windows fastembed target dep. It forwards to ort-sys/disable-linking, which skips the static-archive download step, making ort-download-binaries a silent no-op and leaving the binary expecting libonnxruntime.so at runtime -> silent deadlock on first .embed(). Default build now statically links ONNX Runtime. - Wrap FastembedProvider::new in catch_unwind so a panicking ONNX init transitions ModelState::Failed and notifies all Condvar waiters, instead of wedging concurrent callers forever. --- CHANGELOG.md | 7 ++ Cargo.lock | 23 ++--- Cargo.toml | 23 ++++- src/embedding/lazy.rs | 207 +++++++++++++++++++++++++++++++++++++++++- 4 files changed, 236 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 300597d..08899a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- **embedding**: stopped force-enabling `ort-load-dynamic` on the Linux/Windows `fastembed` target dependency (#50). `ort-load-dynamic` forwards to `ort-sys/disable-linking`, which makes `ort-sys`'s build script early-return and **skip the static-archive download step entirely** — so `ort-download-binaries-rustls-tls` was a silent no-op and the resulting binary expected `libonnxruntime.so` to be supplied externally at runtime. Since llm-kernel never ships that library, `embedding-fastembed` on Linux deadlocked silently on first `.embed()` instead of failing cleanly. The default build now statically links ONNX Runtime and produces self-contained binaries. We deliberately did **not** add an `embedding-fastembed-dynamic` opt-in feature: cargo feature unification would re-merge `ort-load-dynamic` into the static `fastembed` dep whenever both llm-kernel features were enabled together, silently reintroducing #50. Deployments that truly need dynamic loading should depend on `fastembed` directly in their own crate and enable `ort-load-dynamic` there. +- **embedding**: `LazyFastembedProvider`'s model-load path is now **panic-safe**. A panic during `FastembedProvider::new()` (e.g. a missing `libonnxruntime.so` under dynamic loading) is caught via `catch_unwind` and converted into a `ModelState::Failed(..)` transition that notifies all `Condvar` waiters, so concurrent callers receive a clean error instead of wedging forever on `futex` (confirmed in production via `/proc/PID/wchan`). Guards against any future ort/fastembed init failure mode, not just the dynamic-linking one. + ## [0.11.0] - 2026-07-01 ### Added diff --git a/Cargo.lock b/Cargo.lock index e36b2f9..96cdbc6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -700,9 +700,9 @@ dependencies = [ [[package]] name = "console" -version = "0.16.3" +version = "0.16.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d64e8af5551369d19cf50138de61f1c42074ab970f74e99be916646777f8fc87" +checksum = "4fe5f465a4f6fee88fad41b85d990f84c835335e85b5d9e6e63e0d06d28cba7c" dependencies = [ "encode_unicode", "libc", @@ -2144,9 +2144,9 @@ dependencies = [ [[package]] name = "indicatif" -version = "0.18.5" +version = "0.18.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "993f007684f2e9727160da8b960ec161264703bfd1af084fd2e34d040c9a0dd4" +checksum = "9433806cd6b4ec1aba79c021c7e4c58fb4c3b9977c085062e611ac929998fb0c" dependencies = [ "console", "portable-atomic", @@ -2294,16 +2294,6 @@ dependencies = [ "cc", ] -[[package]] -name = "libloading" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "754ca22de805bb5744484a5b151a9e1a8e837d5dc232c2d7d8c2e3492edc8b60" -dependencies = [ - "cfg-if", - "windows-link", -] - [[package]] name = "libm" version = "0.2.16" @@ -2312,9 +2302,9 @@ checksum = "b6d2cec3eae94f9f509c767b45932f1ada8350c4bdb85af2fcab4a3c14807981" [[package]] name = "libredox" -version = "0.1.17" +version = "0.1.18" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f02ab6bace2054fb888a3c16f990117b579d14a3088e472d63c6011fa185c9d3" +checksum = "c943259e342f1e06ff2da7a83eabdfe7f92ce10262688dbf1895ff0b3e6e4652" dependencies = [ "libc", ] @@ -2957,7 +2947,6 @@ version = "2.0.0-rc.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7de3af33d24a745ffb8fab904b13478438d1cd52868e6f17735ef6e1f8bf133" dependencies = [ - "libloading", "ndarray", "ort-sys", "smallvec", diff --git a/Cargo.toml b/Cargo.toml index 02cffa8..831cade 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -228,9 +228,22 @@ codegen-units = 4 inherits = "release" lto = "thin" -# Windows needs ort-load-dynamic to avoid MSVC linker errors with ort-sys. -# Linux needs ort-load-dynamic because ort's prebuilt static libraries require -# glibc 2.38+ (__isoc23_strtol etc), which is not available on ubuntu-22.04. -# macOS uses static linking via ort-download-binaries (works reliably). +# Statically link ONNX Runtime on every platform via `ort-download-binaries-rustls-tls`: +# ort-sys downloads + links a prebuilt static archive at build time, producing a +# self-contained binary with no runtime dylib resolution. +# +# We deliberately do NOT enable `ort-load-dynamic` here: that feature forwards to +# `ort-sys/disable-linking`, which makes ort-sys's build script early-return and +# SKIP the download step entirely — so `ort-download-binaries-*` becomes a silent +# no-op and the resulting binary expects `libonnxruntime.{so,dll}` to be supplied +# externally at runtime (which llm-kernel never ships). On Linux this previously +# manifested as a silent deadlock rather than a clean missing-library error +# (see #50). +# +# We do not expose an `embedding-fastembed-dynamic` opt-in feature either: cargo +# feature unification would merge `ort-load-dynamic` into the static `fastembed` +# dep whenever both llm-kernel features were enabled together, silently +# re-introducing #50. Deployments that truly need dynamic loading should depend +# on `fastembed` directly in their own crate and enable `ort-load-dynamic` there. [target.'cfg(any(target_os = "windows", target_os = "linux"))'.dependencies] -fastembed = { version = "5", default-features = false, features = ["hf-hub-rustls-tls", "ort-download-binaries-rustls-tls", "ort-load-dynamic"], optional = true } +fastembed = { version = "5", default-features = false, features = ["hf-hub-rustls-tls", "ort-download-binaries-rustls-tls"], optional = true } diff --git a/src/embedding/lazy.rs b/src/embedding/lazy.rs index 2f45481..36f8475 100644 --- a/src/embedding/lazy.rs +++ b/src/embedding/lazy.rs @@ -20,6 +20,7 @@ //! ``` use std::path::PathBuf; +use std::sync::Arc; use std::sync::{Condvar, Mutex}; use std::time::{Duration, Instant}; @@ -30,6 +31,63 @@ use super::fastembed::FastembedProvider; use super::types::text_preview; use super::types::{EmbeddingProvider, EmbeddingResult}; +// --------------------------------------------------------------------------- +// Load hook (panic-safe model instantiation) +// --------------------------------------------------------------------------- + +/// Type of the model-load closure used by [`LazyFastembedProvider`]. +/// +/// Wrapped in `Arc` so the loader can be cheaply cloned out from under the +/// `Inner` lock and invoked without holding the mutex across the +/// (potentially minutes-long) model download/init. +pub(crate) type LoadFn = + Arc anyhow::Result + Send + Sync>; + +/// Default loader: instantiate [`FastembedProvider`] directly. +/// +/// This is a free function (rather than an inline call) so that tests can swap +/// it for an injectable panicking/failing loader via +/// [`LazyFastembedProvider::new_with_loader`] without needing real ONNX weights +/// on disk. +fn default_load(model: EmbeddingModel, cache_dir: PathBuf) -> anyhow::Result { + FastembedProvider::new(model, Some(cache_dir)) +} + +/// Run `load` catching any panic, so a panicking ONNX init (e.g. a missing +/// `libonnxruntime.so` under `ort-load-dynamic`) is surfaced as a clean `Err` +/// instead of unwinding across the `Mutex`/`Condvar` boundary and wedging any +/// waiter parked in [`LazyFastembedProvider::ensure_model`]'s `Loading` branch +/// (see #50). +/// +/// `AssertUnwindSafe` is sound here: on the `Err(panic)` path the closure's +/// captures are discarded entirely, and the values read afterwards +/// (`EmbeddingModel`/`PathBuf`) were copied in before the call — so no torn +/// shared state escapes. +fn load_catching_panics( + load: &LoadFn, + model: EmbeddingModel, + cache_dir: PathBuf, +) -> anyhow::Result { + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| load(model, cache_dir))); + match result { + Ok(inner) => inner, + Err(payload) => { + let msg = panic_message(&payload); + Err(anyhow::anyhow!("model init panicked: {msg}")) + } + } +} + +/// Best-effort string extraction from a panic payload. +fn panic_message(payload: &Box) -> String { + if let Some(s) = payload.downcast_ref::<&'static str>() { + (*s).to_string() + } else if let Some(s) = payload.downcast_ref::() { + s.clone() + } else { + "".to_string() + } +} // --------------------------------------------------------------------------- // ModelState // --------------------------------------------------------------------------- @@ -137,6 +195,9 @@ struct Inner { state: ModelState, provider: Option, last_used: Option, + /// Injectable model loader. Defaults to [`default_load`] (which calls + /// `FastembedProvider::new`); tests substitute a controllable loader. + load_fn: LoadFn, } /// Lazy-loading embedding provider backed by [`FastembedProvider`]. @@ -162,6 +223,20 @@ impl LazyFastembedProvider { /// /// Returns instantly — no model download or ONNX initialisation. pub fn new(model: EmbeddingModel, cache_dir: PathBuf, opts: LazyOpts) -> Self { + Self::new_with_loader(model, cache_dir, opts, Arc::new(default_load)) + } + + /// Create a lazy provider with an injected model loader. + /// + /// Production callers should use [`new`](Self::new); this constructor exists + /// so tests can drive the panic/failure paths of [`ensure_model`](Self::ensure_model) + /// deterministically without real ONNX weights. + pub fn new_with_loader( + model: EmbeddingModel, + cache_dir: PathBuf, + opts: LazyOpts, + load_fn: LoadFn, + ) -> Self { let initial_state = if is_model_cached(model, &cache_dir) { ModelState::Cached } else { @@ -174,6 +249,7 @@ impl LazyFastembedProvider { state: initial_state, provider: None, last_used: None, + load_fn, }), cvar: Condvar::new(), opts: opts.clone(), @@ -253,12 +329,17 @@ impl LazyFastembedProvider { guard.state = ModelState::Loading; let model = guard.model; let cache_dir = guard.cache_dir.clone(); + // Clone the loader out from under the lock so we don't hold the + // mutex across the (potentially minutes-long) model load. + let load = Arc::clone(&guard.load_fn); // Notify waiters that we've started loading (they'll keep waiting) self.cvar.notify_all(); drop(guard); - // Do the actual loading (may download, takes seconds to minutes) - let result = FastembedProvider::new(model, Some(cache_dir)); + // Do the actual loading (may download, takes seconds to minutes). + // Wrapped in `catch_unwind` so a panicking ONNX init surfaces as + // `Failed(..)` + `notify_all()` instead of wedging waiters (#50). + let result = load_catching_panics(&load, model, cache_dir); let mut guard = self.inner.lock().unwrap_or_else(|e| e.into_inner()); match result { @@ -513,4 +594,126 @@ mod tests { ); assert_eq!(provider.state(), ModelState::Cached); } + + // ----- panic-safety regression tests (#50) ----- + // + // These exercise the contract that a panicking (or failing) model loader + // transitions the provider to `Failed` and releases any waiter parked in + // the `Loading` branch, instead of hanging forever. They use the injected + // loader (`new_with_loader`) so no real ONNX weights are required. + // + // NOTE: the production `[profile.release]` sets `panic = "abort"`, under + // which a panic terminates the process directly (no unwind to catch). The + // `catch_unwind` guard therefore matters for panic-unwinding builds + // (debug, and release builds that override `panic`), and these tests run + // under the default test harness which unwinds. The guard is still + // valuable under `panic = "abort"` for the *failure* (non-panic) path and + // documents intent; the abort case is acceptable (a hard crash is a clearer + // failure than the previous silent deadlock). + + fn panicking_loader() -> LoadFn { + Arc::new( + |_model: EmbeddingModel, _cache: PathBuf| -> anyhow::Result { + panic!("simulated ort init failure (missing libonnxruntime.so)") + }, + ) + } + + fn failing_loader(msg: &str) -> LoadFn { + let msg = msg.to_string(); + Arc::new( + move |_model: EmbeddingModel, _cache: PathBuf| -> anyhow::Result { + Err(anyhow::anyhow!("{msg}")) + }, + ) + } + + #[test] + fn panic_during_load_transitions_to_failed() { + let dir = tempfile::tempdir().unwrap(); + let provider = LazyFastembedProvider::new_with_loader( + EmbeddingModel::BGESmallENV15, + dir.path().to_path_buf(), + LazyOpts::default(), + panicking_loader(), + ); + assert_eq!(provider.state(), ModelState::NotLoaded); + + let err = provider.ensure_model().unwrap_err(); + assert!( + err.contains("model init panicked"), + "expected panic message, got: {err}" + ); + match provider.state() { + ModelState::Failed(msg) => assert!( + msg.contains("model init panicked"), + "expected panic in Failed state, got: {msg}" + ), + other => panic!("expected Failed state after panic, got {other:?}"), + } + } + + #[test] + fn failure_during_load_transitions_to_failed() { + let dir = tempfile::tempdir().unwrap(); + let provider = LazyFastembedProvider::new_with_loader( + EmbeddingModel::BGESmallENV15, + dir.path().to_path_buf(), + LazyOpts::default(), + failing_loader("download denied"), + ); + let err = provider.ensure_model().unwrap_err(); + assert!(err.contains("download denied"), "got: {err}"); + assert!(matches!(provider.state(), ModelState::Failed(_))); + } + + #[test] + fn concurrent_waiter_released_when_loader_panics() { + // AC4: a waiter blocked in the `Loading` branch must be released (with + // an `Err`) within a bounded time when the owning loader panics — not + // hang until load_timeout_secs, and not deadlock. + use std::sync::Barrier; + use std::thread; + + let dir = tempfile::tempdir().unwrap(); + + // A loader that waits on a barrier until the waiter is parked, then + // panics — deterministically reproducing the #50 hang scenario. + let barrier = Arc::new(Barrier::new(2)); + let loader_barrier = Arc::clone(&barrier); + let loader: LoadFn = Arc::new(move |_m, _c| { + // Let the waiter reach the `Loading` wait first. + loader_barrier.wait(); + panic!("simulated ort init failure under contention"); + }); + + let opts = LazyOpts { + load_timeout_secs: 30, // generous; we must release well before this + ..LazyOpts::default() + }; + let provider = Arc::new(LazyFastembedProvider::new_with_loader( + EmbeddingModel::BGESmallENV15, + dir.path().to_path_buf(), + opts, + loader, + )); + + // Waiter thread: parks in the `Loading` branch. + let waiter_provider = Arc::clone(&provider); + let waiter = thread::spawn(move || waiter_provider.ensure_model()); + + // Give the loader the chance to run; it waits for the barrier, the + // waiter parks in Loading, then this wait returns and the loader + // panics. The provider's panic guard converts that to Failed + notify. + barrier.wait(); + + // The waiter must terminate (with Err) promptly rather than hang. + let result = waiter.join().expect("waiter thread should not hang"); + assert!(result.is_err(), "waiter should receive Err, not Ok"); + assert!( + matches!(provider.state(), ModelState::Failed(_)), + "provider should be Failed after loader panic, got {:?}", + provider.state() + ); + } } From 499c2718b26895faea846b0f88a09817edf9b078 Mon Sep 17 00:00:00 2001 From: epicsagas Date: Thu, 2 Jul 2026 17:48:53 +0900 Subject: [PATCH 2/2] fix(embedding): restore opt-in dynamic ONNX linking, qualify panic-safety claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Re-add ort-load-dynamic as opt-in embedding-fastembed-dynamic-linking feature; static linking alone doesn't satisfy glibc <2.38 Linux hosts (the original reason ort-load-dynamic existed, per #50) - CHANGELOG: scope the catch_unwind panic-safety guarantee to unwinding builds — this crate's own panic = "abort" release profile can't catch panics, so a panicking init still aborts in release builds - ModelState::Failed now carries a panicked flag (is_panic()) so callers can distinguish a panic-origin failure from an ordinary load error - Add LazyFastembedProvider::reset() so a Failed provider can retry instead of being permanently stuck --- CHANGELOG.md | 5 +- Cargo.lock | 11 ++++ Cargo.toml | 31 +++++++-- README.md | 1 + src/embedding/lazy.rs | 149 +++++++++++++++++++++++++++++++++++++----- 5 files changed, 171 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08899a6..90eda46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- **embedding**: stopped force-enabling `ort-load-dynamic` on the Linux/Windows `fastembed` target dependency (#50). `ort-load-dynamic` forwards to `ort-sys/disable-linking`, which makes `ort-sys`'s build script early-return and **skip the static-archive download step entirely** — so `ort-download-binaries-rustls-tls` was a silent no-op and the resulting binary expected `libonnxruntime.so` to be supplied externally at runtime. Since llm-kernel never ships that library, `embedding-fastembed` on Linux deadlocked silently on first `.embed()` instead of failing cleanly. The default build now statically links ONNX Runtime and produces self-contained binaries. We deliberately did **not** add an `embedding-fastembed-dynamic` opt-in feature: cargo feature unification would re-merge `ort-load-dynamic` into the static `fastembed` dep whenever both llm-kernel features were enabled together, silently reintroducing #50. Deployments that truly need dynamic loading should depend on `fastembed` directly in their own crate and enable `ort-load-dynamic` there. -- **embedding**: `LazyFastembedProvider`'s model-load path is now **panic-safe**. A panic during `FastembedProvider::new()` (e.g. a missing `libonnxruntime.so` under dynamic loading) is caught via `catch_unwind` and converted into a `ModelState::Failed(..)` transition that notifies all `Condvar` waiters, so concurrent callers receive a clean error instead of wedging forever on `futex` (confirmed in production via `/proc/PID/wchan`). Guards against any future ort/fastembed init failure mode, not just the dynamic-linking one. +- **embedding**: stopped force-enabling `ort-load-dynamic` on the Linux/Windows `fastembed` target dependency by default (#50). `ort-load-dynamic` forwards to `ort-sys/disable-linking`, which makes `ort-sys`'s build script early-return and **skip the static-archive download step entirely** — so `ort-download-binaries-rustls-tls` was a silent no-op and the resulting binary expected `libonnxruntime.so` to be supplied externally at runtime. Since llm-kernel never ships that library, `embedding-fastembed` on Linux deadlocked silently on first `.embed()` instead of failing cleanly. The default build now statically links ONNX Runtime and produces self-contained binaries. **Caveat:** ort's prebuilt static archive requires glibc 2.38+, resolved against the executing host's libc at runtime — Linux hosts on glibc <2.38 (e.g. Ubuntu 22.04, Debian 12) will fail to load the statically-linked binary at first ONNX Runtime init. Such deployments should enable the new opt-in `embedding-fastembed-dynamic-linking` feature instead (forwards to `fastembed/ort-load-dynamic`) and ensure `libonnxruntime.{so,dll}` is present on the runtime host. Do not combine `embedding-fastembed-dynamic-linking` with a build that also enables plain `embedding-fastembed`/`full` elsewhere in the same feature graph — Cargo feature unification would re-merge both and silently reintroduce #50. +- **embedding**: `LazyFastembedProvider`'s model-load path is now **panic-safe** in builds that unwind on panic (the default `dev`/`test` profile, and any `release` profile that doesn't override `panic`). A panic during `FastembedProvider::new()` (e.g. a missing `libonnxruntime.so` under dynamic loading) is caught via `catch_unwind` and converted into a `ModelState::Failed(..)` transition that notifies all `Condvar` waiters, so concurrent callers receive a clean error instead of wedging forever on `futex` (confirmed in production via `/proc/PID/wchan`). Guards against any future ort/fastembed init failure mode, not just the dynamic-linking one. **Note:** this crate's own `[profile.release]` sets `panic = "abort"`, under which `catch_unwind` cannot intercept a panic — a panicking init in a release build of this crate still aborts the process rather than transitioning to `Failed`. This is an intentional tradeoff (a hard crash is a clearer failure signal than the previous silent deadlock), but it means the "clean error" guarantee above is scoped to unwinding builds; downstream crates that enable `panic = "abort"` in their own release profile inherit the same limitation. +- **embedding**: a load that fails with `ModelState::Failed` is no longer permanently stuck — `LazyFastembedProvider::reset()` clears a `Failed` state back to `NotLoaded`/`Cached` so a subsequent `ensure_model()` call retries the load (e.g. after a transient network failure during model download). `ModelState::Failed` now also exposes `is_panic()` so callers can distinguish "loader panicked" (ort/global state may be corrupted; retrying on the same process may not be safe) from an ordinary load error (safe to retry). ## [0.11.0] - 2026-07-01 diff --git a/Cargo.lock b/Cargo.lock index 96cdbc6..1793351 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2294,6 +2294,16 @@ dependencies = [ "cc", ] +[[package]] +name = "libloading" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "754ca22de805bb5744484a5b151a9e1a8e837d5dc232c2d7d8c2e3492edc8b60" +dependencies = [ + "cfg-if", + "windows-link", +] + [[package]] name = "libm" version = "0.2.16" @@ -2947,6 +2957,7 @@ version = "2.0.0-rc.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d7de3af33d24a745ffb8fab904b13478438d1cd52868e6f17735ef6e1f8bf133" dependencies = [ + "libloading", "ndarray", "ort-sys", "smallvec", diff --git a/Cargo.toml b/Cargo.toml index 831cade..26330b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -120,6 +120,21 @@ embedding-fastembed-nomic-moe = ["embedding-fastembed", "fastembed/nomic-v2-moe" # DirectML GPU execution provider for FastembedProvider (Windows only) embedding-fastembed-directml = ["embedding-fastembed", "dep:ort"] +# Opt-in dynamic ONNX Runtime linking (loads libonnxruntime.{so,dylib,dll} at +# runtime instead of statically linking a prebuilt archive at build time). +# +# Only use this if your deployment target cannot satisfy the static build's +# requirements — e.g. a glibc <2.38 Linux host (ort's prebuilt static archive +# needs glibc 2.38+; see #50) — and you can guarantee `libonnxruntime.*` is +# present on the runtime host. Do NOT combine this feature with a build that +# also depends on plain `embedding-fastembed` elsewhere in the same binary's +# feature graph: Cargo feature unification merges both `ort-load-dynamic` and +# `ort-download-binaries-*` into the shared `fastembed`/`ort-sys` crate, +# which silently turns the static path into a no-op again (the original #50 +# failure mode). Enable this feature alone, or ensure nothing else in your +# build enables `embedding-fastembed`/`full` at the same time. +embedding-fastembed-dynamic-linking = ["embedding-fastembed", "fastembed/ort-load-dynamic"] + # Knowledge graph with async wrappers (requires tokio) graph-async = ["graph", "dep:tokio"] @@ -228,11 +243,11 @@ codegen-units = 4 inherits = "release" lto = "thin" -# Statically link ONNX Runtime on every platform via `ort-download-binaries-rustls-tls`: +# Statically link ONNX Runtime by default via `ort-download-binaries-rustls-tls`: # ort-sys downloads + links a prebuilt static archive at build time, producing a # self-contained binary with no runtime dylib resolution. # -# We deliberately do NOT enable `ort-load-dynamic` here: that feature forwards to +# We do NOT enable `ort-load-dynamic` by default: that feature forwards to # `ort-sys/disable-linking`, which makes ort-sys's build script early-return and # SKIP the download step entirely — so `ort-download-binaries-*` becomes a silent # no-op and the resulting binary expects `libonnxruntime.{so,dll}` to be supplied @@ -240,10 +255,12 @@ lto = "thin" # manifested as a silent deadlock rather than a clean missing-library error # (see #50). # -# We do not expose an `embedding-fastembed-dynamic` opt-in feature either: cargo -# feature unification would merge `ort-load-dynamic` into the static `fastembed` -# dep whenever both llm-kernel features were enabled together, silently -# re-introducing #50. Deployments that truly need dynamic loading should depend -# on `fastembed` directly in their own crate and enable `ort-load-dynamic` there. +# CAVEAT — static linking does not fully replace ort-load-dynamic: ort's +# prebuilt static archive requires glibc 2.38+ (`__isoc23_strtol` etc.), which +# is resolved against the *executing* host's libc at runtime, not the build +# host's. Linux hosts on glibc <2.38 (e.g. Ubuntu 22.04, Debian 12) will fail +# to load the statically-linked binary at first ONNX Runtime init. Such +# deployments should enable the `embedding-fastembed-dynamic-linking` feature +# instead (see above) and ensure `libonnxruntime.so` is present on the host. [target.'cfg(any(target_os = "windows", target_os = "linux"))'.dependencies] fastembed = { version = "5", default-features = false, features = ["hf-hub-rustls-tls", "ort-download-binaries-rustls-tls"], optional = true } diff --git a/README.md b/README.md index 511aca4..7cd4fe3 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,7 @@ Each module is gated behind a feature flag so you only pay for what you use. | `embedding-fastembed` | Local ONNX embedding via fastembed-rs (44 models) | | | `embedding-fastembed-qwen3` | Qwen3 embedding via candle backend | | | `embedding-fastembed-nomic-moe` | Nomic V2 MoE embedding via candle backend | | +| `embedding-fastembed-dynamic-linking` | Dynamic ONNX Runtime linking (opt-in; for glibc <2.38 Linux hosts, see #50) | | | `vector-index` | TurboQuant compressed vector index — 2-bit/4-bit, SIMD ANN search | | | `qdrant` | Qdrant `AsyncVectorIndex` (`QdrantVectorIndex`) for remote vector search | | | `elastic` | Elasticsearch `AsyncVectorIndex` (`ElasticsearchVectorIndex`) over a hand-rolled reqwest client | | diff --git a/src/embedding/lazy.rs b/src/embedding/lazy.rs index 36f8475..509ff47 100644 --- a/src/embedding/lazy.rs +++ b/src/embedding/lazy.rs @@ -63,17 +63,23 @@ fn default_load(model: EmbeddingModel, cache_dir: PathBuf) -> anyhow::Result anyhow::Result { +) -> (anyhow::Result, bool) { let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| load(model, cache_dir))); match result { - Ok(inner) => inner, + Ok(inner) => (inner, false), Err(payload) => { let msg = panic_message(&payload); - Err(anyhow::anyhow!("model init panicked: {msg}")) + (Err(anyhow::anyhow!("model init panicked: {msg}")), true) } } } @@ -106,7 +112,29 @@ pub enum ModelState { /// Feature disabled by configuration. Disabled, /// Last attempt failed (preserves error). - Failed(String), + /// + /// `panicked` is `true` when the failure came from a caught panic during + /// loader init (e.g. ort/ONNX init panicking on a missing dylib) rather + /// than an ordinary `Err` returned by the loader (e.g. a network error + /// during model download). Callers that want to retry after `Failed` + /// (via [`LazyFastembedProvider::reset`]) should treat a panic-origin + /// failure with more caution — process-global ort state may be corrupted + /// — whereas an ordinary failure is generally safe to retry blindly. + Failed { + /// Human-readable error description. + message: String, + /// `true` if the failure came from a caught panic during loader + /// init, `false` if the loader returned an ordinary `Err`. + panicked: bool, + }, +} + +impl ModelState { + /// Whether this is a `Failed` state that originated from a caught panic + /// (as opposed to an ordinary loader `Err`). + pub fn is_panic(&self) -> bool { + matches!(self, ModelState::Failed { panicked: true, .. }) + } } // --------------------------------------------------------------------------- @@ -266,12 +294,34 @@ impl LazyFastembedProvider { /// Whether the model is loaded and ready for inference. /// /// Cheaper than [`state`](Self::state) for hot-path polling — avoids - /// cloning the `Failed(String)` variant. + /// cloning the `Failed { message, .. }` variant's string. pub fn is_ready(&self) -> bool { let guard = self.inner.lock().unwrap_or_else(|e| e.into_inner()); matches!(guard.state, ModelState::Ready) } + /// Clear a `Failed` state so the next [`ensure_model`](Self::ensure_model) + /// call retries the load, instead of returning the cached error forever. + /// + /// No-op if the provider isn't currently `Failed`. Resets to `Cached` if + /// weights are already on disk, `NotLoaded` otherwise — mirroring the + /// initial-state logic in [`new_with_loader`](Self::new_with_loader). + /// + /// Callers should check [`ModelState::is_panic`] before calling this: + /// retrying after a panic-origin failure re-enters the same loader (and + /// thus the same ort/ONNX init path) that just panicked, which may hit + /// corrupted process-global state rather than a fresh, safe retry. + pub fn reset(&self) { + let mut guard = self.inner.lock().unwrap_or_else(|e| e.into_inner()); + if matches!(guard.state, ModelState::Failed { .. }) { + guard.state = if is_model_cached(guard.model, &guard.cache_dir) { + ModelState::Cached + } else { + ModelState::NotLoaded + }; + } + } + /// Block until the model is ready for inference. /// /// If the model is idle beyond the configured timeout, the ONNX session @@ -296,7 +346,7 @@ impl LazyFastembedProvider { match &guard.state { ModelState::Ready => Ok(()), ModelState::Disabled => Err("model is disabled".into()), - ModelState::Failed(e) => Err(e.clone()), + ModelState::Failed { message, .. } => Err(message.clone()), ModelState::Loading => { // Wait for existing loading thread let timeout = Duration::from_secs(self.opts.load_timeout_secs); @@ -307,7 +357,10 @@ impl LazyFastembedProvider { Ok((mut g, timeout_result)) => { if timeout_result.timed_out() { if matches!(g.state, ModelState::Loading) { - g.state = ModelState::Failed("model loading timed out".into()); + g.state = ModelState::Failed { + message: "model loading timed out".into(), + panicked: false, + }; self.cvar.notify_all(); } return Err("model loading timed out".into()); @@ -320,7 +373,7 @@ impl LazyFastembedProvider { } match &guard.state { ModelState::Ready => Ok(()), - ModelState::Failed(e) => Err(e.clone()), + ModelState::Failed { message, .. } => Err(message.clone()), other => Err(format!("unexpected state after wait: {other:?}")), } } @@ -338,8 +391,8 @@ impl LazyFastembedProvider { // Do the actual loading (may download, takes seconds to minutes). // Wrapped in `catch_unwind` so a panicking ONNX init surfaces as - // `Failed(..)` + `notify_all()` instead of wedging waiters (#50). - let result = load_catching_panics(&load, model, cache_dir); + // `Failed{..}` + `notify_all()` instead of wedging waiters (#50). + let (result, panicked) = load_catching_panics(&load, model, cache_dir); let mut guard = self.inner.lock().unwrap_or_else(|e| e.into_inner()); match result { @@ -351,7 +404,10 @@ impl LazyFastembedProvider { Ok(()) } Err(e) => { - guard.state = ModelState::Failed(e.to_string()); + guard.state = ModelState::Failed { + message: e.to_string(), + panicked, + }; self.cvar.notify_all(); Err(e.to_string()) } @@ -645,12 +701,19 @@ mod tests { "expected panic message, got: {err}" ); match provider.state() { - ModelState::Failed(msg) => assert!( - msg.contains("model init panicked"), - "expected panic in Failed state, got: {msg}" - ), + ModelState::Failed { message, panicked } => { + assert!( + message.contains("model init panicked"), + "expected panic in Failed state, got: {message}" + ); + assert!( + panicked, + "panic-originated failure should set panicked=true" + ); + } other => panic!("expected Failed state after panic, got {other:?}"), } + assert!(provider.state().is_panic()); } #[test] @@ -664,7 +727,58 @@ mod tests { ); let err = provider.ensure_model().unwrap_err(); assert!(err.contains("download denied"), "got: {err}"); - assert!(matches!(provider.state(), ModelState::Failed(_))); + match provider.state() { + ModelState::Failed { panicked, .. } => { + assert!(!panicked, "ordinary Err should not set panicked=true"); + } + other => panic!("expected Failed state, got {other:?}"), + } + assert!(!provider.state().is_panic()); + } + + #[test] + fn reset_after_failure_allows_retry() { + let dir = tempfile::tempdir().unwrap(); + let attempts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let attempts_clone = Arc::clone(&attempts); + let loader: LoadFn = Arc::new(move |_m, _c| { + attempts_clone.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Err(anyhow::anyhow!("transient failure")) + }); + let provider = LazyFastembedProvider::new_with_loader( + EmbeddingModel::BGESmallENV15, + dir.path().to_path_buf(), + LazyOpts::default(), + loader, + ); + + assert!(provider.ensure_model().is_err()); + assert!(matches!(provider.state(), ModelState::Failed { .. })); + assert_eq!(attempts.load(std::sync::atomic::Ordering::SeqCst), 1); + + // Without reset, ensure_model just replays the cached error. + assert!(provider.ensure_model().is_err()); + assert_eq!(attempts.load(std::sync::atomic::Ordering::SeqCst), 1); + + // After reset, ensure_model retries the loader. + provider.reset(); + assert_eq!(provider.state(), ModelState::NotLoaded); + assert!(provider.ensure_model().is_err()); + assert_eq!(attempts.load(std::sync::atomic::Ordering::SeqCst), 2); + } + + #[test] + fn reset_is_noop_when_not_failed() { + let dir = tempfile::tempdir().unwrap(); + let provider = LazyFastembedProvider::new_with_loader( + EmbeddingModel::BGESmallENV15, + dir.path().to_path_buf(), + LazyOpts::default(), + failing_loader("irrelevant"), + ); + assert_eq!(provider.state(), ModelState::NotLoaded); + provider.reset(); + assert_eq!(provider.state(), ModelState::NotLoaded); } #[test] @@ -711,9 +825,10 @@ mod tests { let result = waiter.join().expect("waiter thread should not hang"); assert!(result.is_err(), "waiter should receive Err, not Ok"); assert!( - matches!(provider.state(), ModelState::Failed(_)), + matches!(provider.state(), ModelState::Failed { .. }), "provider should be Failed after loader panic, got {:?}", provider.state() ); + assert!(provider.state().is_panic()); } }