Repository navigation
Conversation
uvloop called PyOS_BeforeFork(), PyOS_AfterFork_Parent() and (in the child) PyOS_AfterFork_Child() for every spawned process, so the callbacks registered with os.register_at_fork() ran even though the child never re-enters the interpreter before exec(). CPython's subprocess (and so the default asyncio loop) only does this when a preexec_fn is given, as documented for os.register_at_fork(). Do the same: install the child fork handler and call the PyOS_*Fork functions only when preexec_fn is set, and only make the error pipe inheritable in that case, since the child handler is what closes it. Fixes MagicStack#749.
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
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 #749.
uvloop called
PyOS_BeforeFork()/PyOS_AfterFork_Parent()for every spawned process, andPyOS_AfterFork_Child()in the child whenever libuv forked. As a result, callbacks registered withos.register_at_fork()ran on everycreate_subprocess_exec()/subprocess_exec(), even though the child never re-enters the interpreter beforeexec(). CPython's_posixsubprocess(and so the default asyncio loop) only does this when apreexec_fnis passed, which is also what theos.register_at_fork()docs describe.This change does the same:
PyOS_BeforeFork()/PyOS_AfterFork_Parent()are called, only whenpreexec_fnis set;Without a
preexec_fnthe child no longer runs any Python code betweenfork()andexec(). The steps the child handler did in that case were already covered by libuv: it resets all signal dispositions toSIG_DFLin the child (uv__process_child_init, orposix_spawnattr_setsigdefaulton macOS), anduv_loop_fork()is not needed beforeexec(). This is also what already happens on macOS when libuv usesposix_spawn(), sincepthread_atforkhandlers don't run there.On macOS the
before/after_in_parenthooks fired for every spawn; withpass_fds(which forces a real fork)after_in_childfired too.test_process_at_fork_hooksruns a small script in a fresh interpreter (the hooks can't be unregistered). The script registers all three hooks and spawns a child with no extra arguments, withpass_fds, and withpreexec_fn. The test checks that the hooks run only forpreexec_fn, under both the uvloop and the asyncio test cases.Testing (macOS arm64, CPython 3.12, built with
--use-system-libuvagainst Homebrew libuv 1.53.0):uvloop/handles/process.pyxreverted tomaster,Test_UV_Process.test_process_at_fork_hooksfails:AssertionError: Lists differ: ['before', 'after_in_parent'] != [].Test_AIO_Processpasses.python -m unittest discover tests: 529 tests OK (40 skipped). WithPYTHONASYNCIODEBUG=1, the only failure istest_fs_event_rename, which fails the same way onmaster.tests/test_sourcecode.py) pass.Not run: Linux, the free-threaded build, and the bundled-libuv build.