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

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.