Skip to content

Commit 60d952e

Browse files
ortyclaude
andauthored
Fix/magika new session hangs on windows (#928)
## Description Brief description of changes and motivation. Fixes #(issue number) ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Change 1 - Change 2 - Change 3 ## Testing Describe the tests you ran to verify your changes: - [ ] Unit tests pass (`pytest`) - [ ] Linting passes (`ruff check .`) - [ ] Type checking passes (`mypy headroom`) - [ ] New tests added for new functionality - [ ] Manual testing performed ## Test Output ``` # Paste relevant test output here pytest -v tests/test_your_feature.py ``` ## Checklist - [ ] My code follows the project's style guidelines - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable ## Screenshots (if applicable) Add screenshots to help explain your changes. ## Additional Notes Any additional information that reviewers should know. <!-- headroom-maintainer-template-completion:start --> ## Description This PR prepares `Fix/magika new session hangs on windows` for review by documenting the intended change, validation evidence, and remaining merge-readiness context. Linked issues: None declared. ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Documentation - [ ] Refactor - [ ] Tests only ## Changes Made - Commit: fix(magika): bound ONNX session init with configurable timeout to pre… - Commit: Merge branch 'main' into fix/magika-new-session-hangs-on-windows - Touches `crates/headroom-core/src/transforms/magika_detector.rs` - Touches `headroom/proxy/handlers/openai.py` ## Testing - [x] GitHub checks reviewed - [x] Metadata/template validation - [ ] Local functional testing ### Test Output ```text gh pr view 928 --repo chopratejas/headroom --json statusCheckRollup - CI / changes: SUCCESS - Init E2E / docker-init-e2e: SUCCESS - PR Governance / label: SUCCESS - Wrap E2E / docker-wrap-e2e: SUCCESS - rust / test (ubuntu): SUCCESS - CI / commitlint: SUCCESS - rust / wheels (x86_64-unknown-linux-gnu): SUCCESS - rust / wheels (aarch64-apple-darwin): SUCCESS - CI / lint: SUCCESS - rust / audit: SUCCESS - rust / parity (nightly, allowed to fail during Phase 0): SKIPPED - CI / build-wheel: SUCCESS ``` ## Real Behavior Proof - Environment: GitHub PR metadata and checks for `chopratejas/headroom` PR #928. - Exact command / steps: Reviewed PR title, commits, changed files, linked issues, labels, and check rollup; appended this maintainer template completion block without replacing the author's original description. - Observed result: PR body now contains all required governance sections, checked readiness fields, and a non-placeholder validation evidence block. - Not tested: This pass updated PR metadata only; code validation remains represented by the linked GitHub checks and any author-provided evidence above. ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review <!-- headroom-maintainer-template-completion:end --> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 16ed73b commit 60d952e

2 files changed

Lines changed: 56 additions & 2 deletions

File tree

crates/headroom-core/src/transforms/magika_detector.rs

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
//! only. PR5 flips the ContentRouter to call us instead of the
3030
//! regex-based [`crate::transforms::content_detector`].
3131
32+
use std::sync::mpsc;
3233
use std::sync::{Mutex, OnceLock};
34+
use std::time::Duration;
3335

3436
use magika::Session;
3537
use thiserror::Error;
@@ -70,8 +72,57 @@ pub enum MagikaDetectorError {
7072
/// missing or ort can't init, retrying just wastes cycles).
7173
static MAGIKA_SESSION: OnceLock<Mutex<Result<Session, String>>> = OnceLock::new();
7274

75+
/// Default cap on magika ONNX session init.
76+
///
77+
/// On some platforms `Session::new()` can hang indefinitely instead of
78+
/// returning an error. Observed on Windows, where magika's transitive
79+
/// `ort` takes a DirectML / binary path on first init (fastembed is
80+
/// Windows-gated to `ort-load-dynamic` in `Cargo.toml` for the same
81+
/// reason, but magika carries its own `ort`). A hang — unlike an `Err` —
82+
/// is not caught by the tiered fallback in [`crate::transforms::detection`],
83+
/// so it stalls the entire compression pipeline until the proxy's own
84+
/// 30s+ timeout fires on every request. Bounding init converts that hang
85+
/// into the already-handled `Err` path. Override with
86+
/// `HEADROOM_MAGIKA_INIT_TIMEOUT_SECS`.
87+
const MAGIKA_INIT_TIMEOUT_SECS_DEFAULT: u64 = 5;
88+
89+
fn magika_init_timeout() -> Duration {
90+
let secs = std::env::var("HEADROOM_MAGIKA_INIT_TIMEOUT_SECS")
91+
.ok()
92+
.and_then(|v| v.trim().parse::<u64>().ok())
93+
.filter(|&s| s > 0)
94+
.unwrap_or(MAGIKA_INIT_TIMEOUT_SECS_DEFAULT);
95+
Duration::from_secs(secs)
96+
}
97+
7398
fn session() -> &'static Mutex<Result<Session, String>> {
74-
MAGIKA_SESSION.get_or_init(|| Mutex::new(Session::new().map_err(|e| e.to_string())))
99+
MAGIKA_SESSION.get_or_init(|| {
100+
let timeout = magika_init_timeout();
101+
let (tx, rx) = mpsc::channel();
102+
// Run the (potentially hanging) ONNX init on a side thread so we
103+
// can bound it. `Session: Send` (the static itself requires it),
104+
// so moving the result across the channel is sound. On timeout we
105+
// record an `Err` — `detection::detect` already falls through to
106+
// the unidiff/regex tiers on `Err` — and the orphaned init thread
107+
// is left to finish on its own; its eventual `send` lands on a
108+
// dropped receiver (harmless) and the `Session` is then dropped.
109+
let spawned = std::thread::Builder::new()
110+
.name("magika-init".into())
111+
.spawn(move || {
112+
let _ = tx.send(Session::new().map_err(|e| e.to_string()));
113+
});
114+
if let Err(e) = spawned {
115+
return Mutex::new(Err(format!("magika init thread spawn failed: {e}")));
116+
}
117+
match rx.recv_timeout(timeout) {
118+
Ok(res) => Mutex::new(res),
119+
Err(_) => Mutex::new(Err(format!(
120+
"magika session init exceeded {}s timeout; \
121+
using non-ML detection tiers",
122+
timeout.as_secs()
123+
))),
124+
}
125+
})
75126
}
76127

77128
/// Classify `content` and return the mapped Headroom [`ContentType`].

headroom/proxy/handlers/openai.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1834,7 +1834,10 @@ async def handle_openai_chat(
18341834
if result.waste_signals:
18351835
waste_signals_dict = result.waste_signals.to_dict()
18361836
except Exception as e:
1837-
logger.warning(f"Optimization failed: {e}")
1837+
logger.warning(
1838+
f"Optimization failed: {type(e).__name__}: {e}",
1839+
exc_info=True,
1840+
)
18381841
# Flag compression failure for observability
18391842
_compression_failed = True
18401843

0 commit comments

Comments
 (0)