Backport r62724 from trunk. Fixes issue 2791. subprocess.Popen.communicate

now closes its stdout and stderr fds as soon as it is finished with them.

diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 094ca1ba88b847fda88b60176bd545a0036875a7..31c02df7af275338357d8d691f3a25d7b53ac284 100644 (file)
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -660,8 +660,10 @@ class Popen(object):
                 self.stdin.close()
             elif self.stdout:
                 stdout = self.stdout.read()
+                self.stdout.close()
             elif self.stderr:
                 stderr = self.stderr.read()
+                self.stderr.close()
             self.wait()
             return (stdout, stderr)
 

diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index c2db6fab20b7d4bb56b544ef83f147ff16820f1c..123d4a02182a89d922897ebabfd91e4bead72c11 100644 (file)
--- a/Lib/test/test_subprocess.py
+++ b/Lib/test/test_subprocess.py
@@ -304,6 +304,22 @@ class ProcessTestCase(unittest.TestCase):
         self.assertEqual(remove_stderr_debug_decorations(stderr),
                          "pineapple")
 
+    # This test is Linux specific for simplicity to at least have
+    # some coverage.  It is not a platform specific bug.
+    if os.path.isdir('/proc/%d/fd' % os.getpid()):
+        # 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)
+
     def test_communicate_returns(self):
         # communicate() should return None if no redirection is active
         p = subprocess.Popen([sys.executable, "-c",

diff --git a/Misc/NEWS b/Misc/NEWS
index 2a60b051cbea750151769f6bba5969ad42bb647e..4eb8a8070483f26da6c54d13b2e903053d563e72 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -82,6 +82,10 @@ Library
 - Bug #1433694: minidom's .normalize() failed to set .nextSibling for
   last child element.
 
+- Issue #2791: subprocess.Popen.communicate explicitly closes its
+  stdout and stderr fds rather than leaving them open until the
+  instance is destroyed.
+
 
 Extension Modules
 -----------------