From 3887932e1099931801876d53d05e25a43c3473b7 Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Mon, 20 May 2019 05:35:56 -0700 Subject: [PATCH] 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 --- Lib/asyncio/unix_events.py | 18 ++++++++++++------ Lib/test/test_asyncio/test_subprocess.py | 17 +++++++++++++---- Misc/ACKS | 1 + .../2019-01-18-16-23-00.bpo-35721.d8djAJ.rst | 3 +++ 4 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-01-18-16-23-00.bpo-35721.d8djAJ.rst 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 @@ class _UnixSubprocessTransport(base_subprocess.BaseSubprocessTransport): # 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 @@ class SubprocessMixin: 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 @@ class SubprocessMixin: 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. -- 2.47.3