Skip to content

Commit 2359ab4

Browse files
committed
Fix propagation of errors in multiprocessing.util.spawnv_passfds
The error return pipe, `errpipe_write`, was not included in the "FDs to keep" that were passed to `_posixsubprocess.fork_exec`. This resulted in the pipe being closed in `_posixsubprocess.child_exec`, which in turn meant errors from `os.execv`/`os.execve` could not be written to it, preventing propagation. We saw this manifest when our env vars became too large. `strace` showed: ``` [pid 114] execve("/default-pegasus-venv/bin/python", ["/default-pegasus-venv/bin/python", "-B", "-c", "from multiprocessing.resource_tr"...], 0x62a996e37620 /* 311 vars */) = -1 E2BIG (Argument list too long) [pid 114] write(31, "OSError:", 8) = -1 EBADF (Bad file descriptor) [pid 114] write(31, "7", 1) = -1 EBADF (Bad file descriptor) [pid 114] write(31, ":", 1) = -1 EBADF (Bad file descriptor) [pid 114] write(31, "", 0) = -1 EBADF (Bad file descriptor) ``` This change fixes this, matching the behaviour in `subprocess.Popen._execute_child`: https://github.com/python/cpython/blob/138ed6db9f89171983dc32af4e7ad2e73d46a940/Lib/subprocess.py#L1897-L1901
1 parent 83387e0 commit 2359ab4

2 files changed

Lines changed: 25 additions & 1 deletion

File tree

Lib/multiprocessing/util.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,8 +513,9 @@ def _flush_std_streams():
513513

514514
def spawnv_passfds(path, args, passfds):
515515
import _posixsubprocess
516-
passfds = tuple(sorted(map(int, passfds)))
517516
errpipe_read, errpipe_write = os.pipe()
517+
passfds = tuple(
518+
sorted(itertools.chain(map(int, passfds), (errpipe_write,))))
518519
try:
519520
return _posixsubprocess.fork_exec(
520521
args, [path], True, passfds, None, None,

Lib/test/_test_multiprocessing.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import multiprocessing.managers
5454
import multiprocessing.pool
5555
import multiprocessing.queues
56+
import multiprocessing.spawn
5657
from multiprocessing.connection import wait
5758

5859
from multiprocessing import util
@@ -982,6 +983,28 @@ def test_forkserver_without_auth_fails(self):
982983
proc.start()
983984
proc.join()
984985

986+
@only_run_in_spawn_testsuite("spawn-specific test")
987+
def test_spawn_errors_propagated(self):
988+
"""
989+
Test that errors starting the subprocess using spawning are propagated
990+
991+
This covers a bug seen where `multiprocessing.util.spawnv_passfds`
992+
did not correctly include the error return pipe in the FDs to keep
993+
when calling `_posixsubprocess.fork_exec`.
994+
"""
995+
if self.TYPE == "threads":
996+
self.skipTest(f"test not appropriate for {self.TYPE}")
997+
# Temporarily set the spawn executable to a directory to force failure
998+
original_exe = multiprocessing.spawn.get_executable()
999+
try:
1000+
with tempfile.TemporaryDirectory() as temp_dir:
1001+
multiprocessing.spawn.set_executable(temp_dir)
1002+
p = self.Process()
1003+
with self.assertRaises(OSError):
1004+
p.start()
1005+
finally:
1006+
multiprocessing.spawn.set_executable(original_exe)
1007+
9851008
#
9861009
#
9871010
#

0 commit comments

Comments
 (0)