Issue #10763: subprocess.communicate() closes stdout and stderr if both are

pipes (bug specific to Windows).

Improve also the unit test: write a portable unit test.

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 452611c5a9a108cb990ca92bbd22a57d92c64eb0..c12898de168c00e99e6d74305a4edb73726014f0 100644 (file)
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -985,6 +985,7 @@ class Popen(object):
 
         def _readerthread(self, fh, buffer):
             buffer.append(fh.read())
+            fh.close()
 
 
         def _communicate(self, input):

diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 8ec3d719dc9aa1ecc54d2f1c79f413f6f443b153..f1c91435b5e7da6d61234a40bb7b7549e0c4d628 100644 (file)
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -366,22 +366,28 @@ class ProcessTestCase(BaseTestCase):
         self.assertEqual(stdout, b"banana")
         self.assertStderrEqual(stderr, b"pineapple")
 
-    # This test is Linux specific for simplicity to at least have
-    # some coverage.  It is not a platform specific bug.
-    @unittest.skipUnless(os.path.isdir('/proc/%d/fd' % os.getpid()),
-                         "Linux specific")
     # Test for the fd leak reported in http://bugs.python.org/issue2791.
     def test_communicate_pipe_fd_leak(self):
-        fd_directory = '/proc/%d/fd' % os.getpid()
-        num_fds_before_popen = len(os.listdir(fd_directory))
-        p = subprocess.Popen([sys.executable, "-c", "print()"],
-                             stdout=subprocess.PIPE)
-        p.communicate()
-        num_fds_after_communicate = len(os.listdir(fd_directory))
-        del p
-        num_fds_after_destruction = len(os.listdir(fd_directory))
-        self.assertEqual(num_fds_before_popen, num_fds_after_destruction)
-        self.assertEqual(num_fds_before_popen, num_fds_after_communicate)
+        for stdin_pipe in (False, True):
+            for stdout_pipe in (False, True):
+                for stderr_pipe in (False, True):
+                    options = {}
+                    if stdin_pipe:
+                        options['stdin'] = subprocess.PIPE
+                    if stdout_pipe:
+                        options['stdout'] = subprocess.PIPE
+                    if stderr_pipe:
+                        options['stderr'] = subprocess.PIPE
+                    if not options:
+                        continue
+                    p = subprocess.Popen((sys.executable, "-c", "pass"), **options)
+                    p.communicate()
+                    if p.stdin is not None:
+                        self.assertTrue(p.stdin.closed)
+                    if p.stdout is not None:
+                        self.assertTrue(p.stdout.closed)
+                    if p.stderr is not None:
+                        self.assertTrue(p.stderr.closed)
 
     def test_communicate_returns(self):
         # communicate() should return None if no redirection is active

diff --git a/Misc/NEWS b/Misc/NEWS
index 7b6d2d11c2f697f92bba5e2b7217cacc7d5c0aac..88dd83625ca1671fea7109f7cc54d8b2c9dd21f7 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -11,6 +11,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #10763: subprocess.communicate() closes stdout and stderr if both are
+  pipes (bug specific to Windows).
+
 - Issue #1693546: fix email.message RFC 2231 parameter encoding to be in better
   compliance (no "s around encoded values).