Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions Lib/subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,55 @@ def getoutput(cmd):
return getstatusoutput(cmd)[1]


def _use_posix_spawn():
"""Check is posix_spawn() can be used for subprocess.

subprocess requires a posix_spawn() implementation that reports properly
errors to the parent process, set errno on the following failures:

* process attribute actions failed
* file actions failed
* exec() failed
"""
if _mswindows or not hasattr(os, 'posix_spawn'):
# os.posix_spawn() is not available
return False

if sys.platform == 'darwin':
# posix_spawn() is a syscall on macOS and properly reports errors
return True

# Check libc name and runtime libc version
try:
ver = os.confstr('CS_GNU_LIBC_VERSION')
# parse 'glibc 2.28' as ('glibc', (2, 28))
parts = ver.split(maxsplit=1)
if len(parts) != 2:
# reject unknown format
raise ValueError
libc = parts[0]
version = tuple(map(int, parts[1].split('.')))

if libc == 'glibc' and version >= (2, 26):
# glibc 2.26 added a pipe to the POSIX implementation
# of posix_spawn() to properly report errors to the parent process.
return True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the patch that added a pipe to the POSIX implementation also switched to unconditional use of fork. Since the main motivation for using posix_spawn appears to be performance benefits of vfork, it seems that glibc's POSIX implementation shouldn't be used at all. I suggest to remove this branch.

Otherwise, LGTM.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. I didn't require to use vfork. But ok, I modified my PR to prefer posix_spawn() implementations which can use vfork in some cases for best performances. Let's start with a minimum platform support, and extend it later.

if sys.platform == 'linux' and libc == 'glibc' and version >= (2, 24):
# glibc 2.24 has a new Linux posix_spawn implementation
# which properly reports errors to the parent process.
return True
except (AttributeError, ValueError, OSError):
# os.confstr() or CS_GNU_LIBC_VERSION value not available
pass

# By default, consider that the implementation does not properly report
# errors.
return False


_USE_POSIX_SPAWN = _use_posix_spawn()


class Popen(object):
""" Execute a child program in a new process.

Expand Down Expand Up @@ -1390,6 +1439,23 @@ def _get_handles(self, stdin, stdout, stderr):
errread, errwrite)


def _posix_spawn(self, args, executable, env, restore_signals):
"""Execute program using os.posix_spawn()."""
if env is None:
env = os.environ

kwargs = {}
if restore_signals:
# See _Py_RestoreSignals() of Python/pylifecycle.c
sigset = []
for signame in ('SIGPIPE', 'SIGXFZ', 'SIGXFSZ'):
signum = getattr(signal, signame, None)
if signum is not None:
sigset.append(signum)
kwargs['setsigdef'] = sigset

self.pid = os.posix_spawn(executable, args, env, **kwargs)

def _execute_child(self, args, executable, preexec_fn, close_fds,
pass_fds, cwd, env,
startupinfo, creationflags, shell,
Expand All @@ -1414,6 +1480,20 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,

if executable is None:
executable = args[0]

if (_USE_POSIX_SPAWN
and os.path.dirname(executable)
and preexec_fn is None
and not close_fds
and not pass_fds
and cwd is None
and p2cread == p2cwrite == -1
and c2pread == c2pwrite == -1
and errread == errwrite == -1
and not start_new_session):
self._posix_spawn(args, executable, env, restore_signals)
return

orig_executable = executable

# For transferring possible exec failure from child to parent.
Expand Down
6 changes: 6 additions & 0 deletions Lib/test/pythoninfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,11 @@ def collect_get_config(info_add):
info_add('%s[%s]' % (prefix, key), repr(config[key]))


def collect_subprocess(info_add):
import subprocess
copy_attributes(info_add, subprocess, 'subprocess.%s', ('_USE_POSIX_SPAWN',))


def collect_info(info):
error = False
info_add = info.add
Expand Down Expand Up @@ -630,6 +635,7 @@ def collect_info(info):
collect_cc,
collect_gdbm,
collect_get_config,
collect_subprocess,

# Collecting from tests should be last as they have side effects.
collect_test_socket,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
The :mod:`subprocess` module now uses the :func:`os.posix_spawn` function if
possible for better performance.