mirror of
https://github.com/python/cpython.git
synced 2025-07-29 06:05:00 +00:00
Fixes issue2791: subprocess.Popen.communicate leaked a file descripton until
the last reference to the Popen instance was dropped. Adding explicit close() calls fixes it. Candidate for backport to release25-maint.
This commit is contained in:
parent
3aa84a7f28
commit
4036fd4b75
2 changed files with 21 additions and 5 deletions
|
@ -661,8 +661,10 @@ class Popen(object):
|
||||||
self.stdin.close()
|
self.stdin.close()
|
||||||
elif self.stdout:
|
elif self.stdout:
|
||||||
stdout = self.stdout.read()
|
stdout = self.stdout.read()
|
||||||
|
self.stdout.close()
|
||||||
elif self.stderr:
|
elif self.stderr:
|
||||||
stderr = self.stderr.read()
|
stderr = self.stderr.read()
|
||||||
|
self.stderr.close()
|
||||||
self.wait()
|
self.wait()
|
||||||
return (stdout, stderr)
|
return (stdout, stderr)
|
||||||
|
|
||||||
|
|
|
@ -287,14 +287,12 @@ class ProcessTestCase(unittest.TestCase):
|
||||||
stderr=subprocess.PIPE)
|
stderr=subprocess.PIPE)
|
||||||
(stdout, stderr) = p.communicate()
|
(stdout, stderr) = p.communicate()
|
||||||
self.assertEqual(stdout, None)
|
self.assertEqual(stdout, None)
|
||||||
# When running with a pydebug build, the # of references is outputted
|
self.assertEqual(remove_stderr_debug_decorations(stderr), "pineapple")
|
||||||
# to stderr, so just check if stderr at least started with "pinapple"
|
|
||||||
self.assert_(stderr.startswith("pineapple"))
|
|
||||||
|
|
||||||
def test_communicate(self):
|
def test_communicate(self):
|
||||||
p = subprocess.Popen([sys.executable, "-c",
|
p = subprocess.Popen([sys.executable, "-c",
|
||||||
'import sys,os;' \
|
'import sys,os;'
|
||||||
'sys.stderr.write("pineapple");' \
|
'sys.stderr.write("pineapple");'
|
||||||
'sys.stdout.write(sys.stdin.read())'],
|
'sys.stdout.write(sys.stdin.read())'],
|
||||||
stdin=subprocess.PIPE,
|
stdin=subprocess.PIPE,
|
||||||
stdout=subprocess.PIPE,
|
stdout=subprocess.PIPE,
|
||||||
|
@ -304,6 +302,22 @@ class ProcessTestCase(unittest.TestCase):
|
||||||
self.assertEqual(remove_stderr_debug_decorations(stderr),
|
self.assertEqual(remove_stderr_debug_decorations(stderr),
|
||||||
"pineapple")
|
"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):
|
def test_communicate_returns(self):
|
||||||
# communicate() should return None if no redirection is active
|
# communicate() should return None if no redirection is active
|
||||||
p = subprocess.Popen([sys.executable, "-c",
|
p = subprocess.Popen([sys.executable, "-c",
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue