[chiarorosa]

What

A transient lock on the ffmpeg/ffprobe executable no longer fails a render with a bare spawn EBUSY. retrySpawnWhileLocked (engine) reruns the call after 250 ms and 500 ms when spawn() fails with EBUSY/ETXTBSY. Once the attempts run out, the error names the binary and keeps code/errno/syscall:

[FFmpeg] Could not start "C:\...\ffmpeg.exe" (EBUSY) after 3 attempts: another process kept the executable open, often antivirus scanning it.

It is used at all 11 non-test call sites that start getFfmpegBinary()/getFfprobeBinary().

Why

On Windows, an executable can't be started while another process holds it open for writing or without sharing (antivirus scanning it, an installer still writing it). The measurements are in the issue comment. In short:

• The errno is EBUSY. EPERM is what an ACL that denies execute produces, which is permanent, so it is not retried.
• EBUSY is thrown synchronously from spawn(). Node only delivers EACCES/EAGAIN/EMFILE/ENFILE/ENOENT through the 'error' event (ChildProcess.prototype.spawn). So ManagedChildProcess's spawn_error outcome never sees it, and a retry keyed on that outcome would not fire for this bug.

Besides failing outright, two call sites cached the lock for the life of the process: the GPU probe's -encoders listing reported "no usable GPU encoder" (silently falling back to CPU), and the psnr filter listing reported "no filters". This PR protects the -encoders stage of the GPU probe; see the known limits below for what is left.

Three call sites cache a promise, so on main a synchronous EBUSY is kept as a rejection for the life of the process: getCachedGpuEncoder (every later GPU render fails), the setts duration check in aacEncode.ts (every later AAC encode fails) and ffprobeMajor for probe facts. Each now retries, and a lock that outlasts the retries is not cached, so the next call asks again.

Related work

Fixes #4058. #4059 attempted this and was closed as stale; this PR addresses its review: the real errno measured on Windows hardware, every managed-binary call site covered, and the enriched error on the runFfmpeg path as well.

How

• retrySpawnWhileLocked(binary, run) wraps the async function that already does the spawn, rather than spawn() itself. The first attempt runs synchronously, so a healthy spawn keeps its exact timing; existing fakes that emit close on the next tick are unaffected. Retrying is safe because the error is thrown before any process exists.
• Big functions are split into a thin wrapper plus …Once, keeping the diff small: runFfprobe, spawnStreamingEncoder, ffmpegStdout (the GPU -encoders and NVENC preset listings), canUseGpuEncoder, runFfprobeJson, ffprobeMajor. The execFile sites (psnrDb, psnr filter listing, readFfmpegVersion, setts duration check) wrap the call itself.
• runFfmpeg rechecks its abort signal at every attempt, so aborting during the wait starts no process. That keeps its existing "an aborted run starts no process" contract. Its timeout is an inactivity window that starts when the process spawns, so a retry does not shorten it; the only extra wait is the retry delays (at most ~750 ms).
• Two wrapped helpers nest (spawnStreamingEncoder awaits the GPU detection, which runs the wrapped ffmpegStdout). An error that has exhausted its attempts is tagged, and the classifier declines a tagged error, so an outer wrapper does not restart a sequence the inner one already gave up on: a lock that outlasts the window costs 3 spawns, not 9, and the message says "after 3 attempts".
• The retry window (3 attempts, ~750 ms total) is open for review, as asked in the issue. A full scan of a 100+ MB ffmpeg.exe may take longer; this is the single RETRY_DELAYS_MS constant.
• Known limits, both unchanged behavior that is much less likely now: if the lock outlasts every attempt during the psnr filter listing, its catch-all still caches "no filters"; and a lock that outlasts the attempts during the GPU probe's per-encoder stage is still swallowed by that stage's catch, which returns null and is cached for the life of the process (only the -encoders listing is protected).

Test plan

• Unit tests: spawnLockRetry.test.ts covers classification, backoff, the exhausted error, and no retry for EPERM/ENOENT/non-spawn EBUSY/non-errno errors.
• Per-call-site tests: engine/src/utils/spawnLockRetry.callSites.test.ts, producer/src/services/spawnLockRetry.callSites.test.ts, and a case in streamingEncoder.test.ts. Each fails on main with the bare spawn EBUSY (or, for the psnr listing, with the silent wrong result) and passes with the fix.
• Mutation check: removing the guard's negation, the syscall check, ETXTBSY, one retry delay, or the exhausted-error tag each fails at least one test. The nested-wrapper case asserts 3 spawns, not 9.
• Rebased onto 341fdd353: the engine suites for the touched modules pass on Windows (345 tests), and the producer call-site tests pass. The new tests for aacEncode, ffprobeMajor and getCachedGpuEncoder each fail without their fix. The Linux container runs above predate the rebase and were not repeated.
• Manual, real Windows 11 + ffmpeg 9.0.2: runFfmpeg against a copy of ffmpeg.exe held with FileShare.None. Lock released after 400 ms → succeeds on attempt 3. Lock held 3 s → throws the error above after 757 ms with code=EBUSY. Script below.
• Linux (oven/bun:1.4.3 container): the new tests, ffprobeArgvContract.test.ts and shared.ffmpegVersion.test.ts pass.
• Linux, engine suite: 8 failures in 4 files (ffprobe.test.ts HDR PNG fixtures, browserManager, audioFxRender, audioMixer.grouping), identical on main (4e6fcdb) and this branch in the same container (8 failed / 250 passed / 21 skipped both times), so they're environmental.
• Linux, producer unit lane: not run. test:classification calls node --test, and the oven/bun image has no Node, so the lane stops before any test on main too. On Windows the lane's failures (4 tests in 5 files) are identical on main.
• typecheck, oxlint, oxfmt, fallow, check-comment-citations.mjs and comment-ratchet.mjs pass.
• Documentation: none needed (no public option or default changed).
• Comments follow CONTRIBUTING.md "Comments".

On Windows, some unrelated tests fail identically on main (path separators, symlinks, ffmpeg 9 dropping -vsync); I left those alone. @hyperframes/inspector's build also fails on Windows on main (C:\C:\... path), and I can file it separately.

Windows repro (Node, no repo needed)

// node repro.mjs <path-to-ffmpeg.exe>
import { spawn } from "node:child_process";
import { copyFileSync } from "node:fs";
const exe = `${process.env.TEMP}\\ffmpeg-locked.exe`;
copyFileSync(process.argv[2], exe);
const holder = spawn("powershell.exe", ["-NoProfile", "-Command",
  `$f=[IO.File]::Open('${exe}','Open','Read','None'); 'READY'; [void][Console]::In.ReadLine()`]);
holder.stdout.once("data", () => {
  try { spawn(exe, ["-version"]); console.log("started"); }
  catch (e) { console.log("sync throw:", e.code, e.errno, e.syscall); } // EBUSY -4082 spawn
  holder.stdin.end("\n");
});

[chiarorosa]

This is my first PR here, so the workflows are waiting on maintainer approval (action_required). @miguel-heygen @miga-heygen, could one of you approve the CI run when you have a moment? The Windows render verification job is the most relevant one for this change. Thanks!

[chiarorosa]

Rebased on main (b7fa007bf). audioExtractor.ts was removed in #5772, so that call site and its test are gone. runFfmpeg now keeps the new stallTimeout path inside the retry. The description is updated to match.

[miguel-heygen]

Welcome, and thank you for this. The research behind it is the best part: measuring that the errno is EBUSY rather than EPERM, and establishing that Node throws it synchronously from spawn() instead of routing it through the 'error' event, is exactly why the earlier attempt could not have worked. I checked the execFile sites too, since those are the ones where it is easy to be wrong. Node's promisify(execFile) calls execFile inside a Promise executor, so the synchronous throw becomes a rejection and your retry does see it. Your test fakes match that real behaviour.

One change needed before merge. In packages/engine/src/utils/spawnLockRetry.ts:38-41, the exhausted-attempts error copies code, errno and syscall from the original, which means it still satisfies isLockedExecutableSpawn. Two of the wrapped helpers nest, so an outer wrapper retries a sequence the inner one already gave up on. The concrete path is spawnStreamingEncoder at packages/engine/src/services/streamingEncoder.ts:472, which awaits getCachedGpuEncoder() at line 494, which calls the wrapped detectGpuEncoder at packages/engine/src/utils/gpuEncoder.ts:65. The -encoders spawn at gpuEncoder.ts:70 is not inside any catch; the .catch at line 85 covers only the probe stage. So if antivirus holds the executable for about 4 seconds while a GPU render starts, the inner wrapper tries 3 times over 750 ms, throws, and the outer wrapper treats that as a fresh locked spawn and restarts the whole encoder start twice more. Nine spawns, roughly 3.75 seconds, and the message the user finally reads still says "after 3 attempts". Not copying syscall onto the enriched error, or tagging it so the classifier declines a tagged error, fixes it in one place.

A smaller accuracy point on the same file. The description lists the GPU probe's cached "no usable GPU encoder" as one of the two cached-lock bugs this fixes, but only the -encoders stage is actually protected. A lock that outlasts the retries during the per-encoder probe is still swallowed by the catch at gpuEncoder.ts:51, returns null, and cachedGpuEncoder at gpuEncoder.ts:88 keeps that for the life of the process. The window is much smaller now, which is a real improvement, but it belongs in your "Known limit" note next to the psnr one, or the cache could decline to store a lock-caused null.

Two minor ones you can take or leave. In packages/engine/src/utils/runFfmpeg.ts, deadlineAtMs is recomputed per attempt, so a caller passing timeout: 30_000 can now wait up to 30.75 seconds; worth a line in the description since the timeout is a public option. And both spawnLockRetry.callSites.test.ts files use real timers, so each retrying case genuinely sleeps 250 ms and the eight of them add about two seconds to every suite run; spawnLockRetry.test.ts already shows the fake-timer pattern.

Separately, and not something you can act on: at this head only the Graphite mergeability check and the WIP check have run, so none of the Windows lanes has ever executed on this branch. For a change about a Windows spawn path that is the evidence that matters most, so a maintainer approving the workflow run is the other thing blocking merge. The branch is also 170 commits behind main and will need a rebase.

[jrusso1020]

Reviewed at e4b02843. I agree with Miguel's request for changes, and I have one small addition.

The nested retry he found is real. At this head the exhausted error in spawnLockRetry.ts copies syscall: "spawn" and the EBUSY code back onto itself, so isLockedExecutableSpawn still matches it. And because spawnStreamingEncoder awaits the wrapped detectGpuEncoder, the outer wrapper restarts a sequence the inner one already gave up on. If you tag the enriched error so the classifier skips it, every nesting is fixed in one place. A test that nests two wrappers and asserts three spawns, not nine, would pin it.

The rest looks right to me. The first attempt runs synchronously, so healthy spawns keep their timing. The abort recheck in runFfmpeg keeps the "an aborted run starts no process" contract. EPERM is correctly left alone. The security pass is clean: no workflow, dependency or network changes.

Need: yes, #4058 is still open and main has no retry · Size: ok · Reuse: nothing to reuse, since engine has no spawn-retry helper today · Simpler: one helper is the right size

— Rames

[chiarorosa]

Thanks @miguel-heygen and @jrusso1020, both reviews were useful. Pushed 86ae93d and rebased onto dc56b3d20, so the conflict is gone.

Nested retries (the required change). An error that has exhausted its attempts is now tagged in a module-level WeakSet, and isLockedExecutableSpawn declines a tagged error. So spawnStreamingEncoder → detectGpuEncoder no longer restarts a sequence the inner wrapper gave up on: a lock that outlasts the window costs 3 spawns instead of 9, and the message still says "after 3 attempts". I kept code/errno/syscall on the error for callers rather than dropping syscall, since the tag covers every nesting in one place. New test in spawnLockRetry.test.ts nests two wrappers and asserts 3 calls, not 9; it fails if I remove the tag check.

Description. I narrowed the claim: only the GPU probe's -encoders stage is protected. A lock that outlasts the retries during the per-encoder stage is still swallowed by the catch at gpuEncoder.ts:51 and cached, so I added it to the known limits next to the psnr one. I did not change the cache to decline a lock-caused null; that felt like a separate change. Happy to do it here if you prefer.

timeout. After the rebase, runFfmpeg's timeout is an inactivity window that starts at spawn, so there is no per-attempt deadline to recompute. The only extra wait is the retry delays, at most ~750 ms. I added a line to the description.

Not taken. The two spawnLockRetry.callSites.test.ts files still use real timers (~2 s extra per run). I can move them to the fake-timer pattern from spawnLockRetry.test.ts if you want it in this PR.

The rebase also touched psnrFilterAvailability.ts: main now passes a flag to listFfmpeg, so I kept that and wrapped the call in the retry. I re-ran the touched engine suites on Windows (185 tests) and the producer audioPadTrim suite under bun test; the Windows lanes in CI still need a maintainer to approve the workflow run.

[miguel-heygen]

Re-checked at 86ae93d. The earlier blocker is fixed: an error this module has already given up on is tagged (exhaustedErrors), so an outer retrySpawnWhileLocked passes it through instead of starting three more attempts, and the exhausted error keeps code, errno and syscall with the original as its cause.

One change before this merges: the comment at the top of packages/engine/src/utils/spawnLockRetry.ts is 8 lines, and the repository merges comments of at most 3 to 4 lines. Something like this keeps what the code cannot say:

// Windows throws EBUSY (POSIX: ETXTBSY) from spawn() itself while another process holds the
// executable open, e.g. antivirus; nothing has started, so running the caller again is safe.

The Node source link and the list of codes Node routes through 'error' read well in the PR description. Optional while you are there: the call-site tests wait the real 250 ms and 500 ms delays; fake timers would keep them fast. I have approved the waiting CI runs so the Windows lanes run on this head.

[chiarorosa]

@miguel-heygen Pushed 233e40a: the top comment of spawnLockRetry.ts is now the two lines you suggested (this clears the comment-ratchet failure), and both call-site test files use fake timers (this clears the flake-gate timeouts). The engine call-site suite now runs in about 2 s.

[chiarorosa]

@miguel-heygen Rebased onto main (341fdd353) and pushed 633cf65; the conflicts are gone.

• gpuEncoder.ts: main moved the -encoders spawn into ffmpegStdout, so the retry now wraps that helper and also covers the new NVENC preset listing.
• Two spawns that came in with the rebase were not wrapped: supportsSettsDuration in aacEncode.ts and ffprobeMajor in ffprobe.ts. Both cache a promise, so a sync EBUSY was kept as a rejection for the life of the process. They now retry, and a lock that outlasts the retries is not cached.
• Same for getCachedGpuEncoder, which now caches the detection promise on main: a rejection from an exhausted lock is no longer kept.

Each new call-site test fails without its fix. Touched engine suites, the producer call-site tests, typecheck, oxlint, oxfmt, fallow and both comment checks pass locally on Windows. The workflow runs will need approval again for this head.

[chiarorosa]

@miguel-heygen @jrusso1020 Both requested changes are in (the exhausted-error tag in 86ae93d and the two-line helper comment in 233e40a), and the branch is rebased onto main at 633cf65 (details in the comment above). Could you take another look when you have a moment? The workflow runs for this head are also waiting for approval.