Skip to content

fix(engine,producer): retry transient Windows spawn file locks on ffmpeg/ffprobe - #4059

Closed
CasbaL wants to merge 1 commit into
heygen-com:mainfrom
CasbaL:fix/spawn-ebusy-transient-retry
Closed

CasbaL wants to merge 1 commit into
heygen-com:mainfrom
CasbaL:fix/spawn-ebusy-transient-retry

Conversation

@CasbaL

@CasbaL CasbaL commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #4058

On Windows, spawn of the managed ffmpeg/ffprobe binaries can fail transiently with EBUSY/ETXTBSY — typically antivirus real-time scanning a just-executed exe, or a lingering handle from a concurrent sibling invocation (one render shares a single managed ffmpeg binary across many stages). Today a single transient failure fails the whole render with a bare spawn EBUSY and no retry.

This adds a shared engine helper, withTransientSpawnRetry, and adopts it at the three spawn sites:

  • runFfmpeg (engine): retries when the ManagedChildProcess outcome is a spawn_error carrying a transient errno.
  • audioExtractor.runFFmpeg (producer): retries a rejected transient spawn error.
  • audioPadTrim.runFfprobeJson (producer): retries a result-shaped spawn_error outcome.

Behavior

  • Transient errnos: EBUSY, ETXTBSY, EAGAIN, EMFILE, ENFILE — retried with exponential backoff (250 ms base, up to 3 attempts total); each retry is logged with the binary path and errno.
  • When retries are exhausted, the error names the binary, the errno, and the usual cause ("transient file lock — often antivirus real-time scanning").
  • Permanent failures (ENOENT, EACCES, …) pass through on the first attempt, unchanged.
  • The helper wraps the caller's existing spawn-and-wait flow rather than the raw spawn() call, so a healthy spawn keeps its exact synchronous timing — no changes to existing fake-timer test choreography.

Test plan

  • New unit tests (packages/engine/src/utils/ffSpawnRetry.test.ts, 13 cases): errno classification, backoff math, message enrichment, result-shaped vs rejected failure retry, exhaustion enrichment, permanent-failure pass-through.
  • Existing runFfmpeg (incl. the windowsHide regression) and chunkEncoder timeout suites pass unchanged.
  • audioPadTrim suite passes (37/37).
  • tsc --noEmit clean for @hyperframes/engine and @hyperframes/producer.

…peg/ffprobe

On Windows, spawning the managed ffmpeg/ffprobe binaries can fail
transiently with EBUSY/ETXTBSY when the executable is briefly locked —
overwhelmingly antivirus real-time scanning a just-written or just-executed
exe, occasionally a lingering handle from a concurrent sibling invocation
(one render shares a single managed ffmpeg binary across many stages).
All three spawn sites surfaced the raw child-process error: no retry, no
binary path, no stage context — the whole render failed with a bare
"spawn EBUSY".

Add a shared engine helper withTransientSpawnRetry that classifies
transient file-lock errnos (EBUSY, ETXTBSY, EAGAIN, EMFILE, ENFILE),
re-runs the caller's spawn-and-wait flow with exponential backoff (250ms
base, up to 3 attempts), logs each retry with the binary path, and
enriches exhausted failures with the binary path, errno, and a
transient-file-lock hint. runFfmpeg adopts it at the outcome level
(ManagedChildProcess spawn_error); audioExtractor and audioPadTrim's
ffprobe runner wrap their manual flows with it. A healthy spawn path
keeps its exact synchronous timing.

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at 80226960 (external contributor). Genesis's evidence gap is real and I'm grading it explicitly below, then answering the six technical questions with code citations.

Evidence grade

The mechanism (retry a spawn-time failure on a specific errno family) is plausible and internally consistent, but nothing in the PR is a witnessed Windows failure: no errno/stack from a real occurrence, no AV vendor/version, no frequency or concurrency data, no confirmation the managed binary is actually what's locked versus e.g. a stale file handle from the OS. The unit tests prove the helper's branching logic, not that this classification matches what Windows actually returns for an AV-locked exe. What would settle it: a support ticket or log excerpt from #4058 quoting the actual error.code and error.syscall, or — cheaper — a note from someone who has reproduced spawn EBUSY on this binary on real Windows hardware confirming the code is exactly EBUSY/ETXTBSY and not EPERM (Windows commonly surfaces AV/sharing-violation failures as EPERM via libuv's uv_translate_sys_error, not always EBUSY). Absent that, this ships a plausible fix for an unconfirmed root cause.

The six questions

1. Only three of the sites that spawn the managed binary — not five, and the ones left out matter more. Grepped the whole repo for spawn/execFile/spawnSync calls against getFfmpegBinary()/getFfprobeBinary() (the managed-binary resolver these three sites use), excluding tests:

  • packages/engine/src/utils/ffprobe.ts:68 (runFfprobe) — unfixed. This is the general-purpose ffprobe wrapper used across the engine (4 non-test import sites), not just audioPadTrim's own probe call. Almost certainly called more often than the three fixed sites combined.

  • packages/engine/src/services/streamingEncoder.ts:487 — unfixed. Live production path, consumed by producer/services/renderOrchestrator.ts and render/stages/captureStreamingStage.ts.

  • packages/engine/src/utils/gpuEncoder.ts:65,180 — unfixed. Two more getFfmpegBinary() spawns (encoder capability probing).

    packages/cli/* and packages/studio-server/helpers/proxyTranscoder.ts also spawn ffmpeg but via a different resolver (findFfBinary/findFFmpeg(), not getFfmpegBinary()) — plausibly out of scope for the managed embedded binary this bug is about, but worth the author confirming rather than assuming.

    If the root cause is real, it fires on the unfixed sites too. This is a blocker: the fix is incomplete on its own stated scope, not just under-verified.

2. Errno classification excludes EPERM/EACCES correctly (they're not in TRANSIENT_SPAWN_ERRNOS, ffSpawnErrno.ts:15-20), so a genuine permission error still fails fast — good. But see the evidence grade above: on Windows, AV/sharing-violation spawn failures often surface as EPERM, not EBUSY. If that's what #4058 actually hit, this PR retries nothing and the bug isn't fixed at all, just re-described. Needs the real errno from the issue to confirm.

3. Idempotency: safe, but by an implicit invariant in one of the three sites. runFfmpeg.ts and audioPadTrim.ts both retry through ManagedChildProcess, whose onError explicitly guards if (this.spawned) return (managedChildProcess.ts:107) — a spawn_error outcome is only produced when the child process image never loaded, so no output file could have been touched. audioExtractor.ts's runFFmpeg (producer) is a manual child_process.spawn + Promise wrapper with no such guard — it relies on the assumption that Node's raw 'error' event for these specific errno codes can only fire pre-spawn, which is true for EBUSY/ETXTBSY/ENOENT-class OS-level exec failures but is asserted nowhere in this file the way the engine's explicit spawned flag asserts it. Not a proven live bug, but a real asymmetry: the same invariant is enforced explicitly in one call site and by assumption in another. Worth a one-line comment or the same explicit guard.

  • No cross-attempt cleanup issue found otherwise — a spawn_error retry re-spawns fresh each time, no stale process or fd carried over.

4. Observable: yes. Every retry logs via console.warn with the binary path and errno (ffSpawnRetry.ts:80-83) before the delay. One gap: on retry exhaustion, the result-shaped path (runFfmpeg.ts's getTransientError callback) just returns the raw unenriched result (ffSpawnRetry.ts:118, if (attemptIndex + 2 > plan.attempts) return result;) — it never calls describeSpawnFailure, so runFfmpeg's caller sees the same bare errno message it always did on final failure, while the thrown-error path (audioExtractor, and audioPadTrim via its rejected-promise style) gets the enriched "after N attempts... antivirus" message via handleThrownSpawnError. Minor, but it means the retry's own context is invisible on the one call site (runFfmpeg) that's arguably the most-used.

5. Wiring verified by reading the call sites, not the tests, per the ask:

  • runFfmpeg.ts:116 — return withTransientSpawnRetry(getFfmpegBinary(), async () => {...}, (result) => ...). Confirmed.

  • audioExtractor.ts:90 — return withTransientSpawnRetry(getFfmpegBinary(), async () => {...}, () => undefined). Confirmed (the () => undefined third arg means this call site only retries on the thrown path, never the result-shaped path — correct, since this function only ever rejects or resolves void).

  • audioPadTrim.ts:660 — const outcome = await withTransientSpawnRetry(getFfprobeBinary(), () => spawnAndCollectFfprobe(args, signal), (result) => result.reason === "spawn_error" ? ... : undefined). Confirmed.

    All three genuinely call the helper. ffSpawnRetry.test.ts (13/13, ran myself) and the mutation test (neutralized the isTransientSpawnErrno guard in handleThrownSpawnError — the "never retries permanent failures" test caught it, 1 call became 3) confirm the helper's own branching is non-vacuous. No test exercises the retry through the real audioExtractor/audioPadTrim call sites specifically (only the generic helper and the pre-existing regression suites, both green — audioPadTrim.test.ts 37/37 via bun test, runFfmpeg.test.ts 18/18, ffSpawnRetry.test.ts 13/13), which is acceptable given the wiring is trivial and directly readable, but it means the only thing proving the site-level integration is this review, not CI.

6. Commit is unsigned (gh api .../commits/80226960 --jq .commit.verification → verified: false, reason: "unsigned"). Flagging per the org ruleset as asked — not acting on it.

Verdict: REQUEST CHANGES

Reasoning: The mechanism is well-built where it's applied — explicit spawn-vs-post-spawn distinction, correct errno exclusion, observable retries, non-vacuous unit tests — but it's applied to 3 of at least 6 confirmed managed-binary spawn sites, two of which (ffprobe.ts, streamingEncoder.ts) are likely hotter paths than the ones fixed. Combined with an unconfirmed root-cause errno (no real Windows repro, and Windows AV lockouts commonly surface as EPERM rather than the classified EBUSY/ETXTBSY), this needs either a real repro confirming the errno, or the same helper applied to the remaining sites (or an explicit, reviewable reason they're excluded), before it's ready to merge.

— Miga

@miguel-heygen

Copy link
Copy Markdown
Collaborator

Closing as stale after a few weeks without activity. Thanks for the contribution; feel free to reopen after rebasing on main.

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.

[Windows] Transient ffmpeg/ffprobe spawn failures (EBUSY/ETXTBSY) fail the whole render with a bare "spawn EBUSY" — no retry, no context

3 participants