fix: bound legacy ffmpeg frame-extraction calls with a timeout - #65
Open
Praveen Kumar Mittal (mittalpk) wants to merge 1 commit into
Open
fix: bound legacy ffmpeg frame-extraction calls with a timeout#65Praveen Kumar Mittal (mittalpk) wants to merge 1 commit into
Praveen Kumar Mittal (mittalpk) wants to merge 1 commit into
Conversation
extractLegacyWindow/extractLegacySingle ran the system ffmpeg binary with no timeout, so a stalled ffmpeg (corrupt input, codec edge case, stuck pipe) hung frame extraction indefinitely. Adds a bounded timeout to both execFileAsync calls; on timeout the child is killed and the call rejects into the existing catch/log/return-empty handling. Fixes microsoft#14
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #14.
What's wrong
extractLegacyWindow/extractLegacySingleinelectron/frames/extractor.tsrun the systemffmpegbinary viaexecFileAsync(ffmpegPath, [...], { maxBuffer }), with notimeout. If ffmpeg stalls (corrupt input, codec edge case, stuck pipe), the promise never settles and frame extraction — and the describer/analysis pipeline behind it — hangs indefinitely for that session.Fix
Added a
LEGACY_FFMPEG_TIMEOUT_MS(60s) constant and passed it astimeouton bothexecFileAsynccalls. On timeout, Node kills the child and the call rejects, which both call sites already route into their existingcatch(log a warning, return an empty/null result) — the same handling as any other ffmpeg failure, so no new error-handling path was needed.How I tested it
extractLegacyWindow/extractLegacySingleare private and resolve their ffmpeg binary through a module-level, cachedwhich/wherelookup that isn't test-injectable without expanding this fix's scope. Instead, added a test that exercises the same mechanism directly: a genuinely hung child process (node -e "setInterval(...)"), assertingexecFileAsyncwith atimeoutkills it and rejects well within the bound rather than hanging.npm test: 155/158 pass; 2 failures + 1 cancelled, all confirmed identical on unpatchedmainwith none of this PR's changes — unrelated (a pre-existingdebug-bundle.test.ts/archivertyping issue and a flaky microphone-controller test).npm run typecheck: pre-existing, unrelated failures indebug-bundle.tsalso confirmed identical onmain; no new errors from this change.