fix(embed): preflight + bundle ONNX Runtime so the embedder works on a clean machine#39
Conversation
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.
There was a problem hiding this comment.
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<()> { |
There was a problem hiding this comment.
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:
- Multiple concurrent/parallel calls to
std::env::set_var, which is a data race and undefined behavior in Rust (especially in multi-threaded contexts). - 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<()> {| if let Ok(p) = std::env::var("ORT_DYLIB_PATH") { | ||
| if !p.is_empty() { | ||
| candidates.push(Some(PathBuf::from(p))); | ||
| } | ||
| } |
There was a problem hiding this comment.
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.
|
Both addressed in d9a901e:
|
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
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.
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
Verified locally (dogfood)
Companion JS-side fix (bundled-lib resolution + stale-install re-download) is in agent-core.