fix(profile): bound pdftotext during boot to prevent startup deadlock#657
fix(profile): bound pdftotext during boot to prevent startup deadlock#657RealDiligent wants to merge 4 commits into
Conversation
matedev01
left a comment
There was a problem hiding this comment.
LGTM — this is exactly the surgical split I asked for when closing #618 (closes #617). extract_pdf_text is now async so pdftotext runs outside the Memory mutex, and the genie_common::subprocess::run_with_timeout helper reaps the child via kill_on_drop on timeout. Scoped to the deadlock fix only (5 files). fmt clean; CI validates the subprocess tests + full suite.
matedev01
left a comment
There was a problem hiding this comment.
Thanks — the design is right (async extract_pdf_text so pdftotext runs outside the Memory mutex, with a hard-deadline helper). But CI fails to compile: output_with_timeout(cmd: Command, …) takes Command by value, and both call sites pass &mut Command.
crates/genie-core/src/profile/ingest.rs:27—Command::new("pdftotext").args([…])returns&mut Command.crates/genie-common/src/subprocess.rs:72(test) —Command::new("sleep").arg("60").
Bind first, then pass by value, e.g.:
let mut cmd = Command::new("pdftotext");
cmd.args(["-layout", &path.to_string_lossy(), "-"]);
let output = output_with_timeout(cmd, PDF_EXTRACT_TIMEOUT).await;Same in the test. cargo clippy --workspace --all-targets -- -D warnings should pass before this can merge.
6c9bd69 to
213be9f
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Closes #617
Summary
The highest-severity subprocess hang: synchronous unbounded
pdftotextduringload_profileran insidewith_shared_memorywhile holding the process-wideMemorymutex during boot — a pathological PDF could prevent genie-core from ever starting.Root cause
profile/ingest.rsused blockingstd::process::Command::output()with no deadline.main.rsinvokedload_profileinsidewith_shared_memory, so the subprocess held the global memory lock for its entire (unbounded) duration before HTTP/voice started.Fix
genie_common::subprocess::output_with_timeout(kill_on_drop+tokio::time::timeout).extract_pdf_text(30s cap) runs without the memory lock; syncingest_pdf_textwrites facts under a short lock.load_profileasync and call it directly frommain.rs.Impact
Boot no longer wedges on a hung
pdftotext. Other subprocess sites in #617 remain for follow-up PRs.Risk / tradeoffs
Test plan
genie-common::subprocessunit tests (fast command succeeds; slow command times out).extract_pdf_textreturnsNonefor missing files without panicking.cargo test— CI validation required (fork workflow approval).Real Behavior Proof
Local:
cargo test -p genie-common --lib subprocess::— 2/2 passed. No Jetson hardware available.