Skip to content

fix(lib): use spawn-based pool in reverse_video_file to avoid fork deadlocks - #672

Closed
Agi-Asi wants to merge 2 commits into
jeertmans:mainfrom
Agi-Asi:fix/reverse-pool-deadlock
Closed

Agi-Asi wants to merge 2 commits into
jeertmans:mainfrom
Agi-Asi:fix/reverse-pool-deadlock

Conversation

@Agi-Asi

@Agi-Asi Agi-Asi commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Description

Segmented video reversal runs a multiprocessing.Pool while the parent process still holds an open PyAV input container (the with av.open(src) wrapping the whole body of reverse_video_file). On Linux the default fork start method duplicates the parent mid-flight: a forked child can inherit an FFmpeg/PyAV (or tqdm) lock in a locked state with no owner left to release it, and the pool then deadlocks on its own queue semaphore.

This matches the reported failures exactly:

Both are probabilistic — the fork has to race a held lock — which is why small projects never hit it and larger ones (more segments → more workers → more forks) hit it once every 10–20 renders, as reported.

Fix: create the pool from an explicit spawn context. Freshly spawned interpreters cannot inherit held locks, and this matches the default start method on macOS and Windows — Linux was the odd one out. reverse_video_file_in_one_chunk is a module-level function taking picklable Path arguments, so it is already spawn-compatible; a few interpreter startups are noise next to the video re-encoding the workers perform.

Testing

  • New regression test drives the segmented path (segment duration forced below the test asset's length, so the pool actually runs) and asserts a decodable, non-empty output.
  • pytest tests/test_utils.py — 3 passed.
  • ruff check / ruff format --check clean on the touched files.

Fixes #562
Fixes #569

…adlocks

Segmented video reversal runs a multiprocessing Pool while the parent
process still holds an OPEN PyAV input container (the 'with av.open(src)'
that wraps the whole function body). On Linux the default 'fork' start
method duplicates the parent mid-flight: a forked child can inherit an
FFmpeg/PyAV (or tqdm) lock in a locked state with no owner left to
release it, and the pool then deadlocks on its own queue semaphore.

This matches the reported failures exactly:
- renders hanging in 'Reversing large file by cutting it in segments'
  once every 10-20 runs (jeertmans#562)
- a worker stuck in multiprocessing/queues.py get() -> semlock
  __enter__, only surfaced as a KeyboardInterrupt stack trace (jeertmans#569)

Both are probabilistic (the fork must race a held lock), which is why
small projects never hit it and larger ones hit it intermittently.

Create the pool from an explicit 'spawn' context instead: freshly
spawned interpreters cannot inherit held locks, and this matches the
default start method on macOS and Windows — Linux was the odd one out.
The worker function (reverse_video_file_in_one_chunk) is a module-level
function taking picklable Path arguments, so it is already
spawn-compatible; the cost of a few interpreter startups is noise next
to the video re-encoding they perform.

Adds a regression test driving the segmented path (segment duration
forced below the test asset's length) and asserting a decodable,
non-empty output.

Fixes jeertmans#562
Fixes jeertmans#569
@github-actions github-actions Bot added waiting review Waiting review (or response) by a maintainer waiting triage This issue or PR needs to be labeled accordingly. labels Aug 27, 2026
@codacy-production

Copy link
Copy Markdown

Not up to standards ⛔

🔴 Issues 2 high

Alerts:
⚠ 2 issues (≤ 0 issues of at least minor severity)

Results:
2 new issues

Category Results
Security 2 high

View in Codacy

🟢 Metrics 0 duplication

Metric Results
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@codecov

codecov Bot commented Aug 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.10%. Comparing base (1549ef4) to head (e2b5ea8).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #672   +/-   ##
=======================================
  Coverage   79.10%   79.10%           
=======================================
  Files          23       23           
  Lines        2178     2178           
=======================================
  Hits         1723     1723           
  Misses        455      455           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jeertmans jeertmans added duplicate This issue or pull request already exists bug Something isn't working and removed waiting triage This issue or PR needs to be labeled accordingly. waiting review Waiting review (or response) by a maintainer labels Aug 27, 2026
@jeertmans

Copy link
Copy Markdown
Owner

Hi @Agi-Asi, thanks for your contribution! However, I believe this is a duplicated of #668, isn't it?

@Agi-Asi

Agi-Asi commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

You're right — #668 covers the same spawn-context fix (I missed it when scanning open PRs; my search matched issue references but #668's body used 'Closes #562/#569' formatting my scan tokenized differently — my mistake). Closing this in favor of #668, which is more complete (also sorts segment paths). Sorry for the noise, and thanks for the quick triage!

@Agi-Asi Agi-Asi closed this Aug 27, 2026
@jeertmans

Copy link
Copy Markdown
Owner

No worries!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working duplicate This issue or pull request already exists

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Concurrency bug when reversing videos [BUG] Rendering stalled in 'Reversing large file by cutting it in segment'

2 participants