From 91ce0d933c1a993db23994c1ea53d7a666dce96c Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Mon, 3 Jan 2011 18:45:09 +0000 Subject: [PATCH] Merged revisions 87695 via svnmerge from svn+ssh://pythondev@svn.python.org/python/branches/py3k ........ r87695 | antoine.pitrou | 2011-01-03 19:23:55 +0100 (lun., 03 janv. 2011) | 5 lines Issue #10806, issue #9905: Fix subprocess pipes when some of the standard file descriptors (0, 1, 2) are closed in the parent process. Initial patch by Ross Lagerwall. ........ --- Lib/subprocess.py | 39 ++++++++++++++++------------ Lib/test/test_subprocess.py | 52 +++++++++++++++++++++++++++++++++++++ Lib/test/test_support.py | 10 +++++++ Misc/NEWS | 4 +++ 4 files changed, 89 insertions(+), 16 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 4c235d9a614..2ecde0bb39d 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1051,14 +1051,17 @@ class Popen(object): errread, errwrite) - def _set_cloexec_flag(self, fd): + def _set_cloexec_flag(self, fd, cloexec=True): try: cloexec_flag = fcntl.FD_CLOEXEC except AttributeError: cloexec_flag = 1 old = fcntl.fcntl(fd, fcntl.F_GETFD) - fcntl.fcntl(fd, fcntl.F_SETFD, old | cloexec_flag) + if cloexec: + fcntl.fcntl(fd, fcntl.F_SETFD, old | cloexec_flag) + else: + fcntl.fcntl(fd, fcntl.F_SETFD, old & ~cloexec_flag) def _close_fds(self, but): @@ -1128,21 +1131,25 @@ class Popen(object): os.close(errpipe_read) # Dup fds for child - if p2cread is not None: - os.dup2(p2cread, 0) - if c2pwrite is not None: - os.dup2(c2pwrite, 1) - if errwrite is not None: - os.dup2(errwrite, 2) + def _dup2(a, b): + # dup2() removes the CLOEXEC flag but + # we must do it ourselves if dup2() + # would be a no-op (issue #10806). + if a == b: + self._set_cloexec_flag(a, False) + elif a is not None: + os.dup2(a, b) + _dup2(p2cread, 0) + _dup2(c2pwrite, 1) + _dup2(errwrite, 2) - # Close pipe fds. Make sure we don't close the same - # fd more than once, or standard fds. - if p2cread is not None and p2cread not in (0,): - os.close(p2cread) - if c2pwrite is not None and c2pwrite not in (p2cread, 1): - os.close(c2pwrite) - if errwrite is not None and errwrite not in (p2cread, c2pwrite, 2): - os.close(errwrite) + # Close pipe fds. Make sure we don't close the + # same fd more than once, or standard fds. + closed = { None } + for fd in [p2cread, c2pwrite, errwrite]: + if fd not in closed and fd > 2: + os.close(fd) + closed.add(fd) # Close all other fds, if asked for if close_fds: diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 26adf225d56..37dc245ebea 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -778,6 +778,58 @@ class POSIXProcessTestCase(BaseTestCase): self.assertStderrEqual(stderr, '') self.assertEqual(p.wait(), -signal.SIGTERM) + def check_close_std_fds(self, fds): + # Issue #9905: test that subprocess pipes still work properly with + # some standard fds closed + stdin = 0 + newfds = [] + for a in fds: + b = os.dup(a) + newfds.append(b) + if a == 0: + stdin = b + try: + for fd in fds: + os.close(fd) + out, err = subprocess.Popen([sys.executable, "-c", + 'import sys;' + 'sys.stdout.write("apple");' + 'sys.stdout.flush();' + 'sys.stderr.write("orange")'], + stdin=stdin, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE).communicate() + err = test_support.strip_python_stderr(err) + self.assertEqual((out, err), (b'apple', b'orange')) + finally: + for b, a in zip(newfds, fds): + os.dup2(b, a) + for b in newfds: + os.close(b) + + def test_close_fd_0(self): + self.check_close_std_fds([0]) + + def test_close_fd_1(self): + self.check_close_std_fds([1]) + + def test_close_fd_2(self): + self.check_close_std_fds([2]) + + def test_close_fds_0_1(self): + self.check_close_std_fds([0, 1]) + + def test_close_fds_0_2(self): + self.check_close_std_fds([0, 2]) + + def test_close_fds_1_2(self): + self.check_close_std_fds([1, 2]) + + def test_close_fds_0_1_2(self): + # Issue #10806: test that subprocess pipes still work properly with + # all standard fds closed. + self.check_close_std_fds([0, 1, 2]) + def test_wait_when_sigchild_ignored(self): # NOTE: sigchild_ignore.py may not be an effective test on all OSes. sigchild_ignore = test_support.findfile("sigchild_ignore.py", diff --git a/Lib/test/test_support.py b/Lib/test/test_support.py index 6c2b2b23974..cf912e10efc 100644 --- a/Lib/test/test_support.py +++ b/Lib/test/test_support.py @@ -1219,3 +1219,13 @@ def args_from_interpreter_flags(): if v > 0: args.append('-' + opt * v) return args + +def strip_python_stderr(stderr): + """Strip the stderr of a Python process from potential debug output + emitted by the interpreter. + + This will typically be run on the result of the communicate() method + of a subprocess.Popen object. + """ + stderr = re.sub(br"\[\d+ refs\]\r?\n?$", b"", stderr).strip() + return stderr diff --git a/Misc/NEWS b/Misc/NEWS index 9c900ffc570..c6faa12528a 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -25,6 +25,10 @@ Core and Builtins Library ------- +- Issue #10806, issue #9905: Fix subprocess pipes when some of the standard + file descriptors (0, 1, 2) are closed in the parent process. Initial + patch by Ross Lagerwall. + - Issue #4662: os.tempnam(), os.tmpfile() and os.tmpnam() now raise a py3k DeprecationWarning.