From: Gregory P. Smith Date: Tue, 14 Dec 2010 13:43:30 +0000 (+0000) Subject: Issue #6559: fix the subprocess.Popen pass_fds implementation. Add a unittest. X-Git-Tag: v3.2b2~104 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8edd99d0852c45f70b6abc851e6b326d4250cd33;p=python Issue #6559: fix the subprocess.Popen pass_fds implementation. Add a unittest. Issue #7213: Change the close_fds default on Windows to better match the new default on POSIX. True when possible (False if stdin/stdout/stderr are supplied). Update the documentation to reflect all of the above. --- diff --git a/Doc/library/subprocess.rst b/Doc/library/subprocess.rst index df22c33555..e08dc8e95e 100644 --- a/Doc/library/subprocess.rst +++ b/Doc/library/subprocess.rst @@ -28,7 +28,7 @@ Using the subprocess Module This module defines one class called :class:`Popen`: -.. class:: Popen(args, bufsize=0, executable=None, stdin=None, stdout=None, stderr=None, preexec_fn=None, close_fds=_PLATFORM_DEFAULT, shell=False, cwd=None, env=None, universal_newlines=False, startupinfo=None, creationflags=0, restore_signals=True, start_new_session=False) +.. class:: Popen(args, bufsize=0, executable=None, stdin=None, stdout=None, stderr=None, preexec_fn=None, close_fds=True, shell=False, cwd=None, env=None, universal_newlines=False, startupinfo=None, creationflags=0, restore_signals=True, start_new_session=False, pass_fds=()) Arguments are: @@ -153,14 +153,22 @@ This module defines one class called :class:`Popen`: If *close_fds* is true, all file descriptors except :const:`0`, :const:`1` and :const:`2` will be closed before the child process is executed. (Unix only). - The default varies by platform: :const:`False` on Windows and :const:`True` - on POSIX and other platforms. + The default varies by platform: Always true on Unix. On Windows it is + true when *stdin*/*stdout*/*stderr* are :const:`None`, false otherwise. On Windows, if *close_fds* is true then no handles will be inherited by the child process. Note that on Windows, you cannot set *close_fds* to true and also redirect the standard handles by setting *stdin*, *stdout* or *stderr*. -.. versionchanged:: 3.2 - The default was changed to True on non Windows platforms. + .. versionchanged:: 3.2 + The default for *close_fds* was changed from :const:`False` to + what is described above. + + *pass_fds* is an optional sequence of file descriptors to keep open + between the parent and child. Providing any *pass_fds* forces + *close_fds* to be :const:`True`. (Unix only) + + .. versionadded:: 3.2 + The *pass_fds* parameter was added. If *cwd* is not ``None``, the child's current directory will be changed to *cwd* before it is executed. Note that this directory is not considered when diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 729a53b622..ac46ce85c0 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -27,10 +27,10 @@ This module defines one class called Popen: class Popen(args, bufsize=0, executable=None, stdin=None, stdout=None, stderr=None, - preexec_fn=None, close_fds=_PLATFORM_DEFAULT, shell=False, + preexec_fn=None, close_fds=True, shell=False, cwd=None, env=None, universal_newlines=False, startupinfo=None, creationflags=0, - restore_signals=True, start_new_session=False): + restore_signals=True, start_new_session=False, pass_fds=()): Arguments are: @@ -81,8 +81,11 @@ is executed. If close_fds is true, all file descriptors except 0, 1 and 2 will be closed before the child process is executed. The default for close_fds -varies by platform: False on Windows and True on all other platforms -such as POSIX. +varies by platform: Always true on POSIX. True when stdin/stdout/stderr +are None on Windows, false otherwise. + +pass_fds is an optional sequence of file descriptors to keep open between the +parent and child. Providing any pass_fds implicitly sets close_fds to true. if shell is true, the specified command will be executed through the shell. @@ -621,17 +624,14 @@ def getoutput(cmd): return getstatusoutput(cmd)[1] -if mswindows: - _PLATFORM_DEFAULT = False -else: - _PLATFORM_DEFAULT = True +_PLATFORM_DEFAULT_CLOSE_FDS = object() class Popen(object): def __init__(self, args, bufsize=0, executable=None, stdin=None, stdout=None, stderr=None, - preexec_fn=None, close_fds=_PLATFORM_DEFAULT, shell=False, - cwd=None, env=None, universal_newlines=False, + preexec_fn=None, close_fds=_PLATFORM_DEFAULT_CLOSE_FDS, + shell=False, cwd=None, env=None, universal_newlines=False, startupinfo=None, creationflags=0, restore_signals=True, start_new_session=False, pass_fds=()): @@ -648,12 +648,24 @@ class Popen(object): if preexec_fn is not None: raise ValueError("preexec_fn is not supported on Windows " "platforms") - if close_fds and (stdin is not None or stdout is not None or - stderr is not None): - raise ValueError("close_fds is not supported on Windows " - "platforms if you redirect stdin/stdout/stderr") + any_stdio_set = (stdin is not None or stdout is not None or + stderr is not None) + if close_fds is _PLATFORM_DEFAULT_CLOSE_FDS: + if any_stdio_set: + close_fds = False + else: + close_fds = True + elif close_fds and any_stdio_set: + raise ValueError( + "close_fds is not supported on Windows platforms" + " if you redirect stdin/stdout/stderr") else: # POSIX + if close_fds is _PLATFORM_DEFAULT_CLOSE_FDS: + close_fds = True + if pass_fds and not close_fds: + warnings.warn("pass_fds overriding close_fds.", RuntimeWarning) + close_fds = True if startupinfo is not None: raise ValueError("startupinfo is only supported on Windows " "platforms") @@ -661,9 +673,6 @@ class Popen(object): raise ValueError("creationflags is only supported on Windows " "platforms") - if pass_fds and not close_fds: - raise ValueError("pass_fds requires close_fds=True.") - self.stdin = None self.stdout = None self.stderr = None @@ -876,7 +885,7 @@ class Popen(object): unused_restore_signals, unused_start_new_session): """Execute program (MS Windows version)""" - assert not pass_fds, "pass_fds not yet supported on Windows" + assert not pass_fds, "pass_fds not supported on Windows." if not isinstance(args, str): args = list2cmdline(args) @@ -1091,7 +1100,7 @@ class Popen(object): # precondition: fds_to_keep must be sorted and unique start_fd = 3 for fd in fds_to_keep: - if fd > start_fd: + if fd >= start_fd: os.closerange(start_fd, fd) start_fd = fd + 1 if start_fd <= MAXFD: diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 74e7404389..0804a13225 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1042,6 +1042,37 @@ class POSIXProcessTestCase(BaseTestCase): "Some fds were left open") self.assertIn(1, remaining_fds, "Subprocess failed") + def test_pass_fds(self): + fd_status = support.findfile("fd_status.py", subdir="subprocessdata") + + open_fds = set() + + for x in range(5): + fds = os.pipe() + self.addCleanup(os.close, fds[0]) + self.addCleanup(os.close, fds[1]) + open_fds.update(fds) + + for fd in open_fds: + p = subprocess.Popen([sys.executable, fd_status], + stdout=subprocess.PIPE, close_fds=True, + pass_fds=(fd, )) + output, ignored = p.communicate() + + remaining_fds = set(map(int, output.split(b','))) + to_be_closed = open_fds - {fd} + + self.assertIn(fd, remaining_fds, "fd to be passed not passed") + self.assertFalse(remaining_fds & to_be_closed, + "fd to be closed passed") + + # pass_fds overrides close_fds with a warning. + with self.assertWarns(RuntimeWarning) as context: + self.assertFalse(subprocess.call( + [sys.executable, "-c", "import sys; sys.exit(0)"], + close_fds=False, pass_fds=(fd, ))) + self.assertIn('overriding close_fds', str(context.warning)) + @unittest.skipUnless(mswindows, "Windows specific tests") class Win32ProcessTestCase(BaseTestCase): diff --git a/Misc/NEWS b/Misc/NEWS index 6ca5fc8f1a..aa90c16a43 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -23,8 +23,12 @@ Library - Issue #10107: Warn about unsaved files in IDLE on OSX. - Issue #7213: subprocess.Popen's default for close_fds has been changed. - It is now platform specific, keeping its default of False on Windows and - changing the default to True on POSIX and other platforms. + It is now True in most cases other than on Windows when input, output or + error handles are provided. + +- Issue #6559: subprocess.Popen has a new pass_fds parameter (actually + added in 3.2beta1) to allow specifying a specific list of file descriptors + to keep open in the child process. What's New in Python 3.2 Beta 1? diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 0b5e54470f..5f226a851d 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -107,7 +107,7 @@ static void child_exec(char *const exec_array[], errno = 0; /* We don't want to report an OSError. */ goto error; } - if (keep_fd <= start_fd) + if (keep_fd < start_fd) continue; for (fd_num = start_fd; fd_num < keep_fd; ++fd_num) { close(fd_num);