Repository navigation
Studio: free PyAV's per-thread scalers before a fork so preexec_fn spawns still exec - #12863
Conversation
…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.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
…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.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Backend CI's
(Python 3.13, rest)shard failstest_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:VideoReformatterin athreading.localfor every thread that callsVideoFrame.reformat().Popenwith apreexec_fn(everychild_popen_kwargs()spawn on Linux) runsPyOS_AfterFork_Childin 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.setsid()/exec, the lifetime spawner blocks in_execute_childreading 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:
The same applies to a running Studio on PyAV 19 or later: once a long-lived thread has reformatted a video frame, later
preexec_fnspawns can hang.extras.txtpinsav==15.1.0on most platforms, which has no per-thread cache, butstudio.txtonly asks forav>=12.3.0, so environments that resolve 19 are exposed.Change
utils/process_lifetime.pyregisters fork hooks once, at import (run_serverimports the module before anything spawns), so every fork is covered:child_popen_kwargs()spawns, launches that bring their ownpreexec_fn(prepared sandbox launches, the sandbox probe, the Python and Terminal tools) and bareos.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.av.video.frameis not imported or has no such cache (PyAV 18 and earlier).Tests
test_a_fork_frees_other_threads_cached_scalers_in_the_parenttest_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 insws_freeContext.test_overlapping_forks_keep_caching_off_until_the_last_returnstest_the_hooks_are_registered_at_importtest_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.reformat(), thenPopen(preexec_fn = ...)(Python 3.13, rest)on a staging copytest_wsl_hostcancellation test fails (4 of 4 runs)Not covered
PyOS_AfterFork_Childclears 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 replacingpreexec_fnacross the spawn sites, which is a larger change.