[Python-checkins] bpo-35537: subprocess can use posix_spawn with pipes (GH-11575)

Wed Jan 23 13:00:45 EST 2019

https://github.com/python/cpython/commit/f6243ac1e4828299fe5a8e943d7bd41cab1f34cd
commit: f6243ac1e4828299fe5a8e943d7bd41cab1f34cd
branch: master
author: Victor Stinner <vstinner at redhat.com>
committer: GitHub <noreply at github.com>
date: 2019年01月23日T19:00:39+01:00
summary:
bpo-35537: subprocess can use posix_spawn with pipes (GH-11575)
* subprocess.Popen can now also use os.posix_spawn() with pipes,
 but only if pipe file descriptors are greater than 2.
* Fix Popen._posix_spawn(): set '_child_created' attribute to True.
* Add Popen._close_pipe_fds() helper function to factorize the code.
files:
M Doc/whatsnew/3.8.rst
M Lib/subprocess.py

diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst
index 8b38cce35368..ce46afd48158 100644
--- a/Doc/whatsnew/3.8.rst
+++ b/Doc/whatsnew/3.8.rst
@@ -280,8 +280,8 @@ Optimizations
 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;
+ * *preexec_fn*, *pass_fds*, *cwd* and *start_new_session* parameters
+ are not set;
 * the *executable* path contains a directory.
 
 * :func:`shutil.copyfile`, :func:`shutil.copy`, :func:`shutil.copy2`,
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index b94575b8401e..2300c7352e0d 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1066,6 +1066,34 @@ def wait(self, timeout=None):
 pass
 raise # resume the KeyboardInterrupt
 
+ def _close_pipe_fds(self,
+ p2cread, p2cwrite,
+ c2pread, c2pwrite,
+ errread, errwrite):
+ # self._devnull is not always defined.
+ devnull_fd = getattr(self, '_devnull', None)
+
+ if _mswindows:
+ if p2cread != -1:
+ p2cread.Close()
+ if c2pwrite != -1:
+ c2pwrite.Close()
+ if errwrite != -1:
+ errwrite.Close()
+ else:
+ if p2cread != -1 and p2cwrite != -1 and p2cread != devnull_fd:
+ os.close(p2cread)
+ if c2pwrite != -1 and c2pread != -1 and c2pwrite != devnull_fd:
+ os.close(c2pwrite)
+ if errwrite != -1 and errread != -1 and errwrite != devnull_fd:
+ os.close(errwrite)
+
+ if devnull_fd is not None:
+ os.close(devnull_fd)
+
+ # Prevent a double close of these handles/fds from __init__ on error.
+ self._closed_child_pipe_fds = True
+
 
 if _mswindows:
 #
@@ -1244,17 +1272,9 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
 # output pipe are maintained in this process or else the
 # pipe will not close when the child process exits and the
 # ReadFile will hang.
- if p2cread != -1:
- p2cread.Close()
- if c2pwrite != -1:
- c2pwrite.Close()
- if errwrite != -1:
- errwrite.Close()
- if hasattr(self, '_devnull'):
- os.close(self._devnull)
- # Prevent a double close of these handles/fds from __init__
- # on error.
- self._closed_child_pipe_fds = True
+ self._close_pipe_fds(p2cread, p2cwrite,
+ c2pread, c2pwrite,
+ errread, errwrite)
 
 # Retain the process handle, but close the thread handle
 self._child_created = True
@@ -1441,7 +1461,10 @@ def _get_handles(self, stdin, stdout, stderr):
 errread, errwrite)
 
 
- def _posix_spawn(self, args, executable, env, restore_signals):
+ def _posix_spawn(self, args, executable, env, restore_signals,
+ p2cread, p2cwrite,
+ c2pread, c2pwrite,
+ errread, errwrite):
 """Execute program using os.posix_spawn()."""
 if env is None:
 env = os.environ
@@ -1456,7 +1479,26 @@ def _posix_spawn(self, args, executable, env, restore_signals):
 sigset.append(signum)
 kwargs['setsigdef'] = sigset
 
+ file_actions = []
+ for fd in (p2cwrite, c2pread, errread):
+ if fd != -1:
+ file_actions.append((os.POSIX_SPAWN_CLOSE, fd))
+ for fd, fd2 in (
+ (p2cread, 0),
+ (c2pwrite, 1),
+ (errwrite, 2),
+ ):
+ if fd != -1:
+ file_actions.append((os.POSIX_SPAWN_DUP2, fd, fd2))
+ if file_actions:
+ kwargs['file_actions'] = file_actions
+
 self.pid = os.posix_spawn(executable, args, env, **kwargs)
+ self._child_created = True
+
+ self._close_pipe_fds(p2cread, p2cwrite,
+ c2pread, c2pwrite,
+ errread, errwrite)
 
 def _execute_child(self, args, executable, preexec_fn, close_fds,
 pass_fds, cwd, env,
@@ -1489,11 +1531,14 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
 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 (p2cread == -1 or p2cread > 2)
+ and (c2pwrite == -1 or c2pwrite > 2)
+ and (errwrite == -1 or errwrite > 2)
 and not start_new_session):
- self._posix_spawn(args, executable, env, restore_signals)
+ self._posix_spawn(args, executable, env, restore_signals,
+ p2cread, p2cwrite,
+ c2pread, c2pwrite,
+ errread, errwrite)
 return
 
 orig_executable = executable
@@ -1548,18 +1593,9 @@ def _execute_child(self, args, executable, preexec_fn, close_fds,
 # be sure the FD is closed no matter what
 os.close(errpipe_write)
 
- # self._devnull is not always defined.
- devnull_fd = getattr(self, '_devnull', None)
- if p2cread != -1 and p2cwrite != -1 and p2cread != devnull_fd:
- os.close(p2cread)
- if c2pwrite != -1 and c2pread != -1 and c2pwrite != devnull_fd:
- os.close(c2pwrite)
- if errwrite != -1 and errread != -1 and errwrite != devnull_fd:
- os.close(errwrite)
- if devnull_fd is not None:
- os.close(devnull_fd)
- # Prevent a double close of these fds from __init__ on error.
- self._closed_child_pipe_fds = True
+ self._close_pipe_fds(p2cread, p2cwrite,
+ c2pread, c2pwrite,
+ errread, errwrite)
 
 # Wait for exec to fail or succeed; possibly raising an
 # exception (limited in size)
