Skip to content

fix(embed): preflight + bundle ONNX Runtime so the embedder works on a clean machine#39

Merged
avifenesh merged 2 commits into
mainfrom
fix/embed-onnxruntime-bundle
May 29, 2026
Merged

fix(embed): preflight + bundle ONNX Runtime so the embedder works on a clean machine#39
avifenesh merged 2 commits into
mainfrom
fix/embed-onnxruntime-bundle

Conversation

@avifenesh

Copy link
Copy Markdown
Contributor

Problem

The embed binary builds with `ort-load-dynamic` (no compile-time ORT CDN dependency). At runtime `ort` loads libonnxruntime lazily — and when the dylib is absent, fastembed's init deadlocks in a futex with zero output. The embedder appears to hang forever on any machine without a system ONNX Runtime. Root-caused via strace: zero `connect()` calls, blocked in `futex_do_wait` — never a network/model-download problem (the prior "tailnet/CDN" theories were red herrings).

Fix

  1. Fail-fast preflight (`resolve_and_preflight_ort` in `model.rs`) runs before fastembed touches `ort`. dlopens a candidate dylib (`ORT_DYLIB_PATH` → next-to-exe → system search), exports `ORT_DYLIB_PATH` on success, and bails with actionable install guidance on failure. No more silent hang.

  2. Bundle ORT in the release tarball (`release.yml`). Ships Microsoft ONNX Runtime 1.24.2 (the ABI `ort-sys 2.0.0-rc.12` targets) beside the embed binary, sha256-pinned per platform. The runtime resolver then finds it as the next-to-exe candidate — zero extra download, zero system install. musl is intentionally unbundled (no MS musl build; a glibc ORT can't dlopen under musl) and fails fast with guidance.

Tests

  • `try_dlopen` unit tests (rejects non-library / missing file), `ort_dylib_name` platform test
  • 46 embed tests pass, clippy clean, fmt clean

Verified locally (dogfood)

  • missing ORT → clean error in <1s (was: infinite hang)
  • bundled-lib-beside-binary, `ORT_DYLIB_PATH` unset → full scan, 3.8MB embeddings
  • end-to-end on a clean repo (consult): init 102 files → embed sidecar built → semantic find ranks correctly
  • MS 1.24.2 lib confirmed ABI-compatible with the ort-sys rc.12 bindings

Companion JS-side fix (bundled-lib resolution + stale-install re-download) is in agent-core.

The embed binary builds with ort-load-dynamic (no compile-time ORT CDN
dependency). At runtime ort loads libonnxruntime lazily; when the dylib
is absent, fastembed's init deadlocks in a futex with zero output - the
embedder appears to hang forever on any machine without a system ORT.

Two fixes:

1. resolve_and_preflight_ort() in model.rs runs BEFORE fastembed touches
   ort. It dlopens a candidate dylib (ORT_DYLIB_PATH -> next-to-exe ->
   system search), exports ORT_DYLIB_PATH on success, and bails with
   actionable install guidance on failure. No more silent hang. Covered
   by try_dlopen unit tests (rejects non-library / missing file) and an
   ort_dylib_name platform test.

2. release.yml bundles the matching Microsoft ONNX Runtime 1.24.2 (the
   ABI ort-sys 2.0.0-rc.12 targets) beside the embed binary in the embed
   tarball, sha256-pinned per platform. The runtime resolver then finds
   it as the next-to-exe candidate - zero extra download, zero system
   install. musl is intentionally unbundled (no MS musl build; a glibc
   ORT cannot dlopen under musl) and fails fast with guidance instead.

Verified locally: missing ORT -> clean error in <1s (was: infinite
hang); bundled-lib-beside-binary -> full scan, 3.8MB embeddings; MS
1.24.2 lib is ABI-compatible with the ort-sys rc.12 bindings.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a pre-flight check to load the ONNX Runtime shared library using libloading before initializing fastembed, preventing silent deadlocks when the library is missing. The reviewer feedback identifies two important issues: first, calling resolve_and_preflight_ort repeatedly can cause a data race and undefined behavior in multi-threaded contexts due to concurrent calls to std::env::set_var, which can be resolved using std::sync::OnceLock. Second, the pre-flight check should handle cases where ORT_DYLIB_PATH points to a directory rather than a file by appending the platform-specific library name.

/// by dlopen-ing the bare library name.
///
/// Returns a clear, actionable error if nothing loads - never a silent hang.
fn resolve_and_preflight_ort() -> Result<()> {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

resolve_and_preflight_ort is called every time FastEmbedder::new is invoked. If a caller creates multiple embedders (e.g., for different model variants) or initializes them concurrently from multiple threads, this will run repeatedly. This leads to:

  1. Multiple concurrent/parallel calls to std::env::set_var, which is a data race and undefined behavior in Rust (especially in multi-threaded contexts).
  2. Unnecessary overhead of re-running the preflight checks and loading/unloading the library.

We should wrap the preflight check in a std::sync::OnceLock to ensure it runs exactly once per process lifetime.

static ORT_INIT: std::sync::OnceLock<Result<(), String>> = std::sync::OnceLock::new();

fn resolve_and_preflight_ort() -> Result<()> {
    ORT_INIT.get_or_init(|| {
        resolve_and_preflight_ort_inner().map_err(|e| e.to_string())
    })
    .clone()
    .map_err(|e| anyhow::anyhow!(e))
}

fn resolve_and_preflight_ort_inner() -> Result<()> {

Comment on lines +195 to +199
if let Ok(p) = std::env::var("ORT_DYLIB_PATH") {
if !p.is_empty() {
candidates.push(Some(PathBuf::from(p)));
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If ORT_DYLIB_PATH is set to a directory (which is a common use case, e.g., pointing to a directory containing the shared library rather than the file itself), p.exists() will be true, but trying to dlopen the directory directly will fail. The ort crate natively supports ORT_DYLIB_PATH pointing to either a directory or a file.

To handle this robustly, we should check if the path is a directory and append the platform-specific library name if so.

if let Ok(p) = std::env::var(

…IB_PATH

Two high-priority review findings on #39:

1. resolve_and_preflight_ort ran on every FastEmbedder::new and called
   std::env::set_var unguarded - a data race / UB if embedders are built
   concurrently or repeatedly. Now wrapped in a OnceLock so it runs exactly
   once per process and the result is cached.

2. ORT_DYLIB_PATH may point at a DIRECTORY (ort supports both dir and file),
   but dlopen needs the file. Normalize dir -> <dir>/<libname> before probing,
   and export the normalized file path.

Tests: added try_dlopen_rejects_a_directory + preflight_runs_once_and_caches
(48 embed tests pass). Dogfood-verified: ORT_DYLIB_PATH set to a directory
now drives a full scan (exit 0, embeddings) - was a guaranteed dlopen failure
before the normalization.
@avifenesh

Copy link
Copy Markdown
Contributor Author

Both addressed in d9a901e:

  • OnceLock guard: resolve_and_preflight_ort now runs exactly once per process via OnceLock; the set_var can no longer race across concurrent FastEmbedder::new calls.
  • Directory ORT_DYLIB_PATH: normalized to <dir>/<libname> before dlopen. Added try_dlopen_rejects_a_directory + preflight_runs_once_and_caches tests; dogfood-verified a directory-form ORT_DYLIB_PATH now drives a full scan.

@avifenesh
avifenesh merged commit 7a85b96 into main May 29, 2026
5 checks passed
@avifenesh
avifenesh deleted the fix/embed-onnxruntime-bundle branch May 29, 2026 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant