From f50a6b6b6d5c7b36e7fbe49266edd3083a23cbb9 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Mon, 3 Jan 2011 18:36:36 +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 | 37 +++++++++++++----------- Lib/test/support.py | 11 ++++++++ Lib/test/test_subprocess.py | 56 +++++++++++++++++++++++++++++++++++++ Misc/NEWS | 4 +++ 4 files changed, 92 insertions(+), 16 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 5684d523952..b8e3c0e4c85 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -1012,14 +1012,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): @@ -1080,23 +1083,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) + 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/support.py b/Lib/test/support.py index 842cda6c349..3353053071f 100644 --- a/Lib/test/support.py +++ b/Lib/test/support.py @@ -10,6 +10,7 @@ import gc import socket import sys import os +import re import platform import shutil import warnings @@ -1056,3 +1057,13 @@ def reap_children(): break except: break + +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/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index a633af50580..ce3e0c232b3 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -804,6 +804,62 @@ class ProcessTestCase(BaseTestCase): " non-zero with this error:\n%s" % stderr.decode('utf8')) + 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 = 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_surrogates_error_message(self): + def prepare(): + raise ValueError("surrogate:\uDCff") + # # Windows tests diff --git a/Misc/NEWS b/Misc/NEWS index 512ffac6bda..f9dd5a10270 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -27,6 +27,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 10753 - Characters ';','=' and ',' in the PATH_INFO environment variable won't be quoted when the URI is constructed by the wsgiref.util 's request_uri method. According to RFC 3986, these characters can be a part of