From b980b7f1078232efba5ee55baf5c2e05f9784f60 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sat, 10 Dec 2022 07:33:43 +0000 Subject: [PATCH 1/8] wip --- Lib/asyncio/base_subprocess.py | 8 +++++--- Lib/test/test_asyncio/test_subprocess.py | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Lib/asyncio/base_subprocess.py b/Lib/asyncio/base_subprocess.py index e15bb4141fc02a..9d7a768529b611 100644 --- a/Lib/asyncio/base_subprocess.py +++ b/Lib/asyncio/base_subprocess.py @@ -215,9 +215,11 @@ def _process_exited(self, returncode): # object. On Python 3.6, it is required to avoid a ResourceWarning. self._proc.returncode = returncode self._call(self._protocol.process_exited) - for p in self._pipes.values(): - if p is not None: - p.pipe.close() + # The pipes should not be closed otherwise some data may be lost. + # https://github.com/python/cpython/issues/100133 + # for p in self._pipes.values(): + # if p is not None: + # p.pipe.close() self._try_finish() diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py index 7411f735da3be6..2f25c9ca6ca444 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -686,6 +686,21 @@ async def execute(): self.assertIsNone(self.loop.run_until_complete(execute())) + def test_subprocess_output_stdout(self): + async def get_command_stdout(cmd, *args): + proc = await asyncio.create_subprocess_exec( + cmd, *args, stdout=asyncio.subprocess.PIPE, + ) + stdout, _ = await proc.communicate() + return stdout.decode().strip() + + async def main(): + outputs = [f'foo{i}' for i in range(10)] + res = await asyncio.gather(*[get_command_stdout('echo', out) for out in outputs]) + self.assertEqual(res, outputs) + + self.loop.run_until_complete(main()) + if sys.platform != 'win32': # Unix From 6b9d8c17cb1a1a948a819e7bf1aa73296f4a86cf Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sat, 10 Dec 2022 08:09:03 +0000 Subject: [PATCH 2/8] add comment & test --- Lib/asyncio/base_subprocess.py | 5 ++++- Lib/test/test_asyncio/test_subprocess.py | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Lib/asyncio/base_subprocess.py b/Lib/asyncio/base_subprocess.py index 9d7a768529b611..e8cb7f3bf2f1de 100644 --- a/Lib/asyncio/base_subprocess.py +++ b/Lib/asyncio/base_subprocess.py @@ -215,8 +215,11 @@ def _process_exited(self, returncode): # object. On Python 3.6, it is required to avoid a ResourceWarning. self._proc.returncode = returncode self._call(self._protocol.process_exited) + # See https://github.com/python/cpython/issues/100133 # The pipes should not be closed otherwise some data may be lost. - # https://github.com/python/cpython/issues/100133 + # If the pipe is closed here then _UnixReadPipeTransport will remove the + # reader prematurely and the data will be lost, instead of doing that + # the pipe will be closed when the process is finished. # for p in self._pipes.values(): # if p is not None: # p.pipe.close() diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py index 2f25c9ca6ca444..e982ffd41e726f 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -687,6 +687,7 @@ async def execute(): self.assertIsNone(self.loop.run_until_complete(execute())) def test_subprocess_output_stdout(self): + # See https://github.com/python/cpython/issues/100133 async def get_command_stdout(cmd, *args): proc = await asyncio.create_subprocess_exec( cmd, *args, stdout=asyncio.subprocess.PIPE, From af1d4e696956f2131e56ea0d5360249484363fab Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sat, 10 Dec 2022 08:26:06 +0000 Subject: [PATCH 3/8] fix comment --- Lib/asyncio/base_subprocess.py | 3 ++- Lib/test/test_asyncio/test_subprocess.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/Lib/asyncio/base_subprocess.py b/Lib/asyncio/base_subprocess.py index e8cb7f3bf2f1de..775052fe981f9f 100644 --- a/Lib/asyncio/base_subprocess.py +++ b/Lib/asyncio/base_subprocess.py @@ -219,7 +219,8 @@ def _process_exited(self, returncode): # The pipes should not be closed otherwise some data may be lost. # If the pipe is closed here then _UnixReadPipeTransport will remove the # reader prematurely and the data will be lost, instead of doing that - # the pipe will be closed when the process is finished. + # the pipe will be closed when the process is finished via _pipe_connection_lost + # followed by _try_finish. # for p in self._pipes.values(): # if p is not None: # p.pipe.close() diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py index e982ffd41e726f..90a0965f732979 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -686,7 +686,7 @@ async def execute(): self.assertIsNone(self.loop.run_until_complete(execute())) - def test_subprocess_output_stdout(self): + def test_subprocess_communicate_stdout(self): # See https://github.com/python/cpython/issues/100133 async def get_command_stdout(cmd, *args): proc = await asyncio.create_subprocess_exec( From 6b87ae7cbad0d9b88c136e48d67ef56191ede47f Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Sat, 10 Dec 2022 08:36:09 +0000 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst diff --git a/Misc/NEWS.d/next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst b/Misc/NEWS.d/next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst new file mode 100644 index 00000000000000..29bba3e684bb6d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst @@ -0,0 +1 @@ +Fix bug in :mod:`asyncio` where a subprocess would sometimes lose data received from pipe. Patch by Kumar Aditya. From 3f55f146015170181e0e41b69ded0cb1ed69d5e0 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Sat, 10 Dec 2022 14:34:29 +0530 Subject: [PATCH 5/8] use sys.executable --- Lib/test/test_asyncio/test_subprocess.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py index 90a0965f732979..31cdaac24c6c5a 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -697,7 +697,8 @@ async def get_command_stdout(cmd, *args): async def main(): outputs = [f'foo{i}' for i in range(10)] - res = await asyncio.gather(*[get_command_stdout('echo', out) for out in outputs]) + res = await asyncio.gather(*[get_command_stdout(sys.executable, '-c', + f'import sys; print({out!r})') for out in outputs]) self.assertEqual(res, outputs) self.loop.run_until_complete(main()) From 4a55951ac9b744c0198046cb377b5664107cf759 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Mon, 12 Dec 2022 10:11:00 +0000 Subject: [PATCH 6/8] fix news --- .../next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst b/Misc/NEWS.d/next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst index 29bba3e684bb6d..881e6ed80fed5a 100644 --- a/Misc/NEWS.d/next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst +++ b/Misc/NEWS.d/next/Library/2022-12-10-08-36-07.gh-issue-100133.g-zQlp.rst @@ -1 +1 @@ -Fix bug in :mod:`asyncio` where a subprocess would sometimes lose data received from pipe. Patch by Kumar Aditya. +Fix regression in :mod:`asyncio` where a subprocess would sometimes lose data received from pipe. From 63714527cd7cfdf28afce015461716b0240f5841 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Mon, 12 Dec 2022 10:11:32 +0000 Subject: [PATCH 7/8] fix import --- Lib/test/test_asyncio/test_subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_asyncio/test_subprocess.py b/Lib/test/test_asyncio/test_subprocess.py index 31cdaac24c6c5a..3830dea7d9ba29 100644 --- a/Lib/test/test_asyncio/test_subprocess.py +++ b/Lib/test/test_asyncio/test_subprocess.py @@ -698,7 +698,7 @@ async def get_command_stdout(cmd, *args): async def main(): outputs = [f'foo{i}' for i in range(10)] res = await asyncio.gather(*[get_command_stdout(sys.executable, '-c', - f'import sys; print({out!r})') for out in outputs]) + f'print({out!r})') for out in outputs]) self.assertEqual(res, outputs) self.loop.run_until_complete(main()) From e9c8fced1b404a96ceb581875079f4b85b97814e Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Mon, 12 Dec 2022 10:12:48 +0000 Subject: [PATCH 8/8] remove redundant code --- Lib/asyncio/base_subprocess.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Lib/asyncio/base_subprocess.py b/Lib/asyncio/base_subprocess.py index 775052fe981f9f..4c9b0dd5653c0c 100644 --- a/Lib/asyncio/base_subprocess.py +++ b/Lib/asyncio/base_subprocess.py @@ -215,15 +215,6 @@ def _process_exited(self, returncode): # object. On Python 3.6, it is required to avoid a ResourceWarning. self._proc.returncode = returncode self._call(self._protocol.process_exited) - # See https://github.com/python/cpython/issues/100133 - # The pipes should not be closed otherwise some data may be lost. - # If the pipe is closed here then _UnixReadPipeTransport will remove the - # reader prematurely and the data will be lost, instead of doing that - # the pipe will be closed when the process is finished via _pipe_connection_lost - # followed by _try_finish. - # for p in self._pipes.values(): - # if p is not None: - # p.pipe.close() self._try_finish()