When subprocess.Popen is used with stdio redirection, it can't use posix_spawn, which triggers unsafe os.fork() as shown by this warning: lib/portage/tests/emerge/test_binpkg_fetch.py::BinpkgFetchtestCase::testLocalFilePkgSyncUpdate /usr/lib/python3.12/multiprocessing/popen_fork.py:66: DeprecationWarning: This process (pid=2493839) is multi-threaded, use of fork() may lead to deadlocks in the child. self.pid = os.fork()
Maybe subprocess.Popen could be fixed to support stdio redirection with posix_spawn, since posix_spawn supports file descriptor inheritance similar to fork/exec, and file actions in the child process including posix_spawn_file_actions_adddup2 and posix_spawn_file_actions_addclose.
There's https://github.com/python/cpython/issues/114467 to add subprocess support for cwd with posix_spawn. We might use portage.process.spawn to implement a subset of the subprocess.Popen interface, since it can support all of the required features via multiprocessing.Process with the "spawn" multiprocessing start method.
I'm curious how subprocess.Popen ends up calling into multiprocessing.popen_fork in the first place. Reading through the code, I think it should make these calls: subprocess.Popen._execute_child _posixsubprocess.fork_exec(..., use_vfork=True) vfork()
(In reply to Mike Gilbert from comment #3) > I'm curious how subprocess.Popen ends up calling into > multiprocessing.popen_fork in the first place. > > Reading through the code, I think it should make these calls: > > subprocess.Popen._execute_child > _posixsubprocess.fork_exec(..., use_vfork=True) > vfork() Yeah, you're right. Now that you mention it I can't reproduce the warning message without the pytest -n auto --dist=worksteal arguments, so I guess those args only trigger it via some kind of multiprocessing in pytest. It seems like there's no problem here, and that's great! Thanks!
Actually it wasn't using the multiprocessing spawn start method under the pytest-xdist plugin when I thought it was, and I fixed that here: https://gitweb.gentoo.org/proj/portage.git/commit/?id=3110ec376cbcb1f5b7fb82ba30ec958798bb32cf commit 3110ec376cbcb1f5b7fb82ba30ec958798bb32cf Author: Zac Medico <zmedico@gentoo.org> Date: 2024-02-12 16:46:52 -0800 actions: Fix interaction between start-method and pytest-xdist Use portage.test.TestCase setUp method to setup the multiprocessing start method if needed. It needs to be done relatively late in order to work with the pytest-xdist plugin due to execnet usage. Bug: https://bugs.gentoo.org/914876 Signed-off-by: Zac Medico <zmedico@gentoo.org> However, this led to pytest-xdist workers crashing intermittently (bug 924416).
