mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
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>
This commit is contained in:
parent
16ed73bca6
commit
60d952e857
2 changed files with 56 additions and 2 deletions
|
|
@ -29,7 +29,9 @@
|
|||
//! only. PR5 flips the ContentRouter to call us instead of the
|
||||
//! regex-based [`crate::transforms::content_detector`].
|
||||
|
||||
use std::sync::mpsc;
|
||||
use std::sync::{Mutex, OnceLock};
|
||||
use std::time::Duration;
|
||||
|
||||
use magika::Session;
|
||||
use thiserror::Error;
|
||||
|
|
@ -70,8 +72,57 @@ pub enum MagikaDetectorError {
|
|||
/// missing or ort can't init, retrying just wastes cycles).
|
||||
static MAGIKA_SESSION: OnceLock<Mutex<Result<Session, String>>> = OnceLock::new();
|
||||
|
||||
/// Default cap on magika ONNX session init.
|
||||
///
|
||||
/// On some platforms `Session::new()` can hang indefinitely instead of
|
||||
/// returning an error. Observed on Windows, where magika's transitive
|
||||
/// `ort` takes a DirectML / binary path on first init (fastembed is
|
||||
/// Windows-gated to `ort-load-dynamic` in `Cargo.toml` for the same
|
||||
/// reason, but magika carries its own `ort`). A hang — unlike an `Err` —
|
||||
/// is not caught by the tiered fallback in [`crate::transforms::detection`],
|
||||
/// so it stalls the entire compression pipeline until the proxy's own
|
||||
/// 30s+ timeout fires on every request. Bounding init converts that hang
|
||||
/// into the already-handled `Err` path. Override with
|
||||
/// `HEADROOM_MAGIKA_INIT_TIMEOUT_SECS`.
|
||||
const MAGIKA_INIT_TIMEOUT_SECS_DEFAULT: u64 = 5;
|
||||
|
||||
fn magika_init_timeout() -> Duration {
|
||||
let secs = std::env::var("HEADROOM_MAGIKA_INIT_TIMEOUT_SECS")
|
||||
.ok()
|
||||
.and_then(|v| v.trim().parse::<u64>().ok())
|
||||
.filter(|&s| s > 0)
|
||||
.unwrap_or(MAGIKA_INIT_TIMEOUT_SECS_DEFAULT);
|
||||
Duration::from_secs(secs)
|
||||
}
|
||||
|
||||
fn session() -> &'static Mutex<Result<Session, String>> {
|
||||
MAGIKA_SESSION.get_or_init(|| Mutex::new(Session::new().map_err(|e| e.to_string())))
|
||||
MAGIKA_SESSION.get_or_init(|| {
|
||||
let timeout = magika_init_timeout();
|
||||
let (tx, rx) = mpsc::channel();
|
||||
// Run the (potentially hanging) ONNX init on a side thread so we
|
||||
// can bound it. `Session: Send` (the static itself requires it),
|
||||
// so moving the result across the channel is sound. On timeout we
|
||||
// record an `Err` — `detection::detect` already falls through to
|
||||
// the unidiff/regex tiers on `Err` — and the orphaned init thread
|
||||
// is left to finish on its own; its eventual `send` lands on a
|
||||
// dropped receiver (harmless) and the `Session` is then dropped.
|
||||
let spawned = std::thread::Builder::new()
|
||||
.name("magika-init".into())
|
||||
.spawn(move || {
|
||||
let _ = tx.send(Session::new().map_err(|e| e.to_string()));
|
||||
});
|
||||
if let Err(e) = spawned {
|
||||
return Mutex::new(Err(format!("magika init thread spawn failed: {e}")));
|
||||
}
|
||||
match rx.recv_timeout(timeout) {
|
||||
Ok(res) => Mutex::new(res),
|
||||
Err(_) => Mutex::new(Err(format!(
|
||||
"magika session init exceeded {}s timeout; \
|
||||
using non-ML detection tiers",
|
||||
timeout.as_secs()
|
||||
))),
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// Classify `content` and return the mapped Headroom [`ContentType`].
|
||||
|
|
|
|||
|
|
@ -1834,7 +1834,10 @@ class OpenAIHandlerMixin:
|
|||
if result.waste_signals:
|
||||
waste_signals_dict = result.waste_signals.to_dict()
|
||||
except Exception as e:
|
||||
logger.warning(f"Optimization failed: {e}")
|
||||
logger.warning(
|
||||
f"Optimization failed: {type(e).__name__}: {e}",
|
||||
exc_info=True,
|
||||
)
|
||||
# Flag compression failure for observability
|
||||
_compression_failed = True
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue