diff options
| -rw-r--r-- | Lib/asyncio/unix_events.py | 18 | ||||
| -rw-r--r-- | Lib/test/test_asyncio/test_subprocess.py | 17 | ||||
| -rw-r--r-- | Misc/ACKS | 1 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/Library/2019-01-18-16-23-00.bpo-35721.d8djAJ.rst | 3 |
4 files changed, 29 insertions, 10 deletions
diff --git a/Lib/asyncio/unix_events.py b/Lib/asyncio/unix_events.py index 73d4bda..1aa3b39 100644 --- a/Lib/asyncio/unix_events.py +++ b/Lib/asyncio/unix_events.py @@ -757,12 +757,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 3908aab..551974a 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -468,9 +468,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: @@ -480,12 +478,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(): @@ -486,6 +486,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 0000000..5af4b1b --- /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. |