From cc999ffbaaaa802872cf68457e51df2f8ec0676e Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 7 Jan 2019 00:48:42 +0100 Subject: [PATCH 1/4] bpo-35537: subprocess uses os.posix_spawn in some cases subprocess.Popen now uses os.posix_spawn() in some cases, if: * os.posix_spawn() is available and properly reports errors to the parent process: macOS or glibc 2.26 and newer (or glibc 2.24 and newer on Linux). * executable path contains a directory * close_fds=False * preexec_fn, pass_fds, cwd, stdin, stdout, stderr and start_new_session parameters are not set --- Lib/subprocess.py | 80 +++++++++++++++++++ Lib/test/pythoninfo.py | 6 ++ .../2018-12-20-16-24-51.bpo-35537.z4E7aA.rst | 2 + 3 files changed, 88 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2018-12-20-16-24-51.bpo-35537.z4E7aA.rst diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 696617697047d2..0d0d0f8dcb5134 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -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 + 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. @@ -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, @@ -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. diff --git a/Lib/test/pythoninfo.py b/Lib/test/pythoninfo.py index 26bcf5f12d6636..4d5da9511d4e20 100644 --- a/Lib/test/pythoninfo.py +++ b/Lib/test/pythoninfo.py @@ -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 @@ -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, diff --git a/Misc/NEWS.d/next/Library/2018-12-20-16-24-51.bpo-35537.z4E7aA.rst b/Misc/NEWS.d/next/Library/2018-12-20-16-24-51.bpo-35537.z4E7aA.rst new file mode 100644 index 00000000000000..34bf8e0fb4c398 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-12-20-16-24-51.bpo-35537.z4E7aA.rst @@ -0,0 +1,2 @@ +The :mod:`subprocess` module now uses the :func:`os.posix_spawn` function if +possible for better performance. From 634a5239b921cd9ca6d450575a010d425e807ddd Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 15 Jan 2019 11:07:44 +0100 Subject: [PATCH 2/4] Don't use glibc POSIX implementation --- Lib/subprocess.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 0d0d0f8dcb5134..bf65425d236df7 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -615,6 +615,9 @@ def _use_posix_spawn(): * process attribute actions failed * file actions failed * exec() failed + + Prefer an implementation which can use vfork in some cases for best + performances. """ if _mswindows or not hasattr(os, 'posix_spawn'): # os.posix_spawn() is not available @@ -635,14 +638,13 @@ def _use_posix_spawn(): 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 if sys.platform == 'linux' and libc == 'glibc' and version >= (2, 24): - # glibc 2.24 has a new Linux posix_spawn implementation + # glibc 2.24 has a new Linux posix_spawn implementation using vfork # which properly reports errors to the parent process. return True + # Note: Don't use the POSIX implementation of glibc because it doesn't + # use vfork (even if glibc 2.26 added a pipe to properly report errors + # to the parent process). except (AttributeError, ValueError, OSError): # os.confstr() or CS_GNU_LIBC_VERSION value not available pass From 26302763e07fdb66f10eb7626455880ccd6bc222 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 15 Jan 2019 11:13:22 +0100 Subject: [PATCH 3/4] Rephrase NEWS entry --- Lib/subprocess.py | 2 +- .../next/Library/2018-12-20-16-24-51.bpo-35537.z4E7aA.rst | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index bf65425d236df7..b94575b8401ece 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1448,7 +1448,7 @@ def _posix_spawn(self, args, executable, env, restore_signals): kwargs = {} if restore_signals: - # See _Py_RestoreSignals() of Python/pylifecycle.c + # See _Py_RestoreSignals() in Python/pylifecycle.c sigset = [] for signame in ('SIGPIPE', 'SIGXFZ', 'SIGXFSZ'): signum = getattr(signal, signame, None) diff --git a/Misc/NEWS.d/next/Library/2018-12-20-16-24-51.bpo-35537.z4E7aA.rst b/Misc/NEWS.d/next/Library/2018-12-20-16-24-51.bpo-35537.z4E7aA.rst index 34bf8e0fb4c398..b14d7493bc6009 100644 --- a/Misc/NEWS.d/next/Library/2018-12-20-16-24-51.bpo-35537.z4E7aA.rst +++ b/Misc/NEWS.d/next/Library/2018-12-20-16-24-51.bpo-35537.z4E7aA.rst @@ -1,2 +1,2 @@ -The :mod:`subprocess` module now uses the :func:`os.posix_spawn` function if -possible for better performance. +The :mod:`subprocess` module can now use the :func:`os.posix_spawn` function in +some cases for better performance. From f000375c22b42e0b800f740d4685163821319c16 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 15 Jan 2019 18:05:02 +0100 Subject: [PATCH 4/4] Document optimization in What's New in Python 3.8 --- Doc/whatsnew/3.8.rst | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 370ef4604834f4..053fe902c4810d 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -275,6 +275,15 @@ xml Optimizations ============= +* The :mod:`subprocess` module can now use the :func:`os.posix_spawn` function + in some cases for better performance. Currently, it is only used on macOS + and Linux (using glibc 2.24 or newer) if all these conditions are met: + + * *close_fds* is false; + * *preexec_fn*, *pass_fds*, *cwd*, *stdin*, *stdout*, *stderr* and + *start_new_session* parameters are not set; + * the *executable* path contains a directory. + * :func:`shutil.copyfile`, :func:`shutil.copy`, :func:`shutil.copy2`, :func:`shutil.copytree` and :func:`shutil.move` use platform-specific "fast-copy" syscalls on Linux, macOS and Solaris in order to copy the file