Skip to content

Studio: free PyAV's per-thread scalers before a fork so preexec_fn spawns still exec - #12863

Merged
danielhanchen merged 3 commits into
mainfrom
studio-fork-pyav-reformatter
Oct 6, 2026
Merged

danielhanchen merged 3 commits into
mainfrom
studio-fork-pyav-reformatter

Conversation

@danielhanchen

@danielhanchen danielhanchen commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Backend CI's (Python 3.13, rest) shard fails test_wsl_host.py::test_build_tool_cancellation_reaps_the_guest_process_group (both variants) because a spawn never execs. It is a real hang in the Linux spawn path, not a slow runner:

  • PyAV 19 caches a VideoReformatter in a threading.local for every thread that calls VideoFrame.reformat().
  • A Popen with a preexec_fn (every child_popen_kwargs() spawn on Linux) runs PyOS_AfterFork_Child in the child, which frees the other threads' state. The scaler freed there (sws_freeContext -> avpriv_slicethread_free) waits on FFmpeg slice threads that do not exist in the child.
  • The child sleeps on a futex before setsid()/exec, the lifetime spawner blocks in _execute_child reading the error pipe, and every later launch queued behind it blocks too.

On the CI worker, an earlier video test left an idle pool thread holding a scaler, so the WSL test's first guest spawn never started. A stack dump of the stuck child on the runner:

Thread (idle): "unsloth-child-spawner"
    pthread_cond_destroy (libc.so.6)
    avpriv_slicethread_free (libavutil)
    sws_freeContext (libswscale)
    VideoReformatter___dealloc__ (reformatter.c)
    do_fork_exec (_posixsubprocess.c:950)
    _execute_child (subprocess.py:1930)
    <lambda> (studio/backend/core/inference/engine_install.py:905)
    _run (studio/backend/utils/process_lifetime.py:446)

The same applies to a running Studio on PyAV 19 or later: once a long-lived thread has reformatted a video frame, later preexec_fn spawns can hang. extras.txt pins av==15.1.0 on most platforms, which has no per-thread cache, but studio.txt only asks for av>=12.3.0, so environments that resolve 19 are exposed.

Change

utils/process_lifetime.py registers fork hooks once, at import (run_server imports the module before anything spawns), so every fork is covered: child_popen_kwargs() spawns, launches that bring their own preexec_fn (prepared sandbox launches, the sandbox probe, the Python and Terminal tools) and bare os.fork() calls (the landlock probe).

  • before: swap PyAV's per-thread cache for one that drops writes. The old scalers are freed in the parent, where their slice threads still run. Because freeing a scaler releases the GIL, a reformat on another thread cannot cache a new one into this fork.
  • after_in_parent / after_in_child: restore a normal cache. A counter keeps it shut while forks on several threads overlap, and an unpaired call can only leave caching off, never block a fork.
  • No-op when av.video.frame is not imported or has no such cache (PyAV 18 and earlier).

Tests

  • test_a_fork_frees_other_threads_cached_scalers_in_the_parent
  • test_nothing_is_cached_into_a_fork_while_the_old_scalers_are_freed: a scaler whose destructor caches a new one, as a reformat on another thread would while this one is in sws_freeContext.
  • test_overlapping_forks_keep_caching_off_until_the_last_returns
  • test_the_hooks_are_registered_at_import
  • test_a_fork_after_a_video_reformat_still_completes[spawner|own_preexec|os_fork]: real PyAV, each scenario in its own process with an outer timeout and bounded waits; a stuck child is killed.
main this branch
new tests 7 failed (all three real fork paths hang) 7 passed
bare repro: idle thread after one reformat(), then Popen(preexec_fn = ...) hangs every run returns
300 forks across the three paths while 4 threads reformat continuously n/a 0 hung, 581 reformats, 0 errors, cache restored
process lifetime, orphan reaping, WSL host, video, MCP sessions, sandbox probe and Linux sandbox suites 985 passed, 3 skipped
Backend CI (Python 3.13, rest) on a staging copy test_wsl_host cancellation test fails (4 of 4 runs) passed (first revision; rerun below)

Not covered

PyOS_AfterFork_Child clears every other thread's state: thread-locals, its contextvars context, its in-flight exception and its running asyncio loop. Objects on other threads' stacks are left alone. A different native object held in one of those places with a destructor that waits on threads would hang the same way. Running no Python in the child would close that for good, but it means replacing preexec_fn across the spawn sites, which is a larger change.

…awns still exec

PyAV 19 caches a VideoReformatter in a threading.local for every thread that
reformats a frame. A Popen with a preexec_fn (every child_popen_kwargs spawn on
Linux) runs PyOS_AfterFork_Child, which frees the other threads' state; the
scaler freed there waits on FFmpeg slice threads the child does not have, so
the child never execs and the lifetime spawner, with every launch queued
behind it, blocks forever. This is the Backend CI failure in
test_build_tool_cancellation_reaps_the_guest_process_group: a video test on the
same xdist worker left an idle pool thread holding a scaler.

A pre-fork hook swaps PyAV's cache, so those scalers are freed in the parent
where their threads still run. child_popen_kwargs registers it too, so spawns
outside the lifetime spawner are covered.
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-06T17:10:06.519471Z 87efc41 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for security reviews. Please try again later.

pre-commit-ci Bot and others added 2 commits October 6, 2026 15:06
…e whole fork

Review fixes:
- The hook was registered lazily from child_popen_kwargs and the spawner, so a
  launch that brings its own preexec_fn (prepared sandbox launches, the
  sandbox probe, the Python and Terminal tools) or a bare os.fork (the
  landlock probe) could still be the first fork after a reformat. Register at
  import instead: run_server imports this module before anything spawns.
- Freeing the old scalers releases the GIL, so another thread could cache a
  new one into the replacement before the fork. The replacement now drops
  writes until the fork returns, and a counter keeps it shut while forks on
  several threads overlap.
- Tests cover all three fork paths with real PyAV, the refill race, overlapping
  forks and import-time registration, with bounded waits and tolerant cleanup.
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: 87efc41faa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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.

1 participant