[Python-checkins] bpo-35721: Close socket pair if Popen in _UnixSubprocessTransport fails (GH-11553)
Miss Islington (bot)
webhook-mailer at python.org
Mon May 20 08:36:02 EDT 2019
https://github.com/python/cpython/commit/3887932e1099931801876d53d05e25a43c3473b7
commit: 3887932e1099931801876d53d05e25a43c3473b7
branch: 3.7
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2019-05-20T05:35:56-07:00
summary:
bpo-35721: Close socket pair if Popen in _UnixSubprocessTransport fails (GH-11553)
This slightly expands an existing test case `test_popen_error` to trigger a `ResourceWarning` and fixes it.
https://bugs.python.org/issue35721
(cherry picked from commit 9932fd91e878b740704ff599522e945a4bbe2ae1)
Co-authored-by: Niklas Fiekas <niklas.fiekas at backscattering.de>
files:
A Misc/NEWS.d/next/Library/2019-01-18-16-23-00.bpo-35721.d8djAJ.rst
M Lib/asyncio/unix_events.py
M Lib/test/test_asyncio/test_subprocess.py
M Misc/ACKS
diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py
index 639300f976ab..a05ebfd51ba2 100644
--- a/Lib/asyncio/unix_events.py
+++ b/Lib/asyncio/unix_events.py
@@ -758,12 +758,18 @@ def _start(self, args, shell, stdin, stdout, stderr, bufsize, **kwargs):
# other end). Notably this is needed on AIX, and works
# just fine on other platforms.
stdin, stdin_w = socket.socketpair()
- self._proc = subprocess.Popen(
- args, shell=shell, stdin=stdin, stdout=stdout, stderr=stderr,
- universal_newlines=False, bufsize=bufsize, **kwargs)
- if stdin_w is not None:
- stdin.close()
- self._proc.stdin = open(stdin_w.detach(), 'wb', buffering=bufsize)
+ try:
+ self._proc = subprocess.Popen(
+ args, shell=shell, stdin=stdin, stdout=stdout, stderr=stderr,
+ universal_newlines=False, bufsize=bufsize, **kwargs)
+ if stdin_w is not None:
+ stdin.close()
+ self._proc.stdin = open(stdin_w.detach(), 'wb', buffering=bufsize)
+ stdin_w = None
+ finally:
+ if stdin_w is not None:
+ stdin.close()
+ stdin_w.close()
class AbstractChildWatcher:
diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py
index 235813aa977c..eeb6a73ffdad 100644
--- a/Lib/test/test_asyncio/test_subprocess.py
+++ b/Lib/test/test_asyncio/test_subprocess.py
@@ -463,9 +463,7 @@ def test_close_dont_kill_finished(self):
isinstance(self, SubprocessFastWatcherTests)):
asyncio.get_child_watcher()._callbacks.clear()
- def test_popen_error(self):
- # Issue #24763: check that the subprocess transport is closed
- # when BaseSubprocessTransport fails
+ def _test_popen_error(self, stdin):
if sys.platform == 'win32':
target = 'asyncio.windows_utils.Popen'
else:
@@ -475,12 +473,23 @@ def test_popen_error(self):
popen.side_effect = exc
create = asyncio.create_subprocess_exec(sys.executable, '-c',
- 'pass', loop=self.loop)
+ 'pass', stdin=stdin,
+ loop=self.loop)
with warnings.catch_warnings(record=True) as warns:
with self.assertRaises(exc):
self.loop.run_until_complete(create)
self.assertEqual(warns, [])
+ def test_popen_error(self):
+ # Issue #24763: check that the subprocess transport is closed
+ # when BaseSubprocessTransport fails
+ self._test_popen_error(stdin=None)
+
+ def test_popen_error_with_stdin_pipe(self):
+ # Issue #35721: check that newly created socket pair is closed when
+ # Popen fails
+ self._test_popen_error(stdin=subprocess.PIPE)
+
def test_read_stdout_after_process_exit(self):
async def execute():
diff --git a/Misc/ACKS b/Misc/ACKS
index 2d887bcb6a0f..5ca2ccf0b252 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -479,6 +479,7 @@ Florian Festi
John Feuerstein
Carl Feynman
Vincent Fiack
+Niklas Fiekas
Anastasia Filatova
Tomer Filiba
Segev Finer
diff --git a/Misc/NEWS.d/next/Library/2019-01-18-16-23-00.bpo-35721.d8djAJ.rst b/Misc/NEWS.d/next/Library/2019-01-18-16-23-00.bpo-35721.d8djAJ.rst
new file mode 100644
index 000000000000..5af4b1b89994
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-01-18-16-23-00.bpo-35721.d8djAJ.rst
@@ -0,0 +1,3 @@
+Fix :meth:`asyncio.SelectorEventLoop.subprocess_exec()` leaks file descriptors
+if ``Popen`` fails and called with ``stdin=subprocess.PIPE``.
+Patch by Niklas Fiekas.
More information about the Python-checkins
mailing list