Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-88050: Fix asyncio subprocess kill process cleanly when process is blocked #32073

Merged
merged 25 commits into from
Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
485b26b
bpo-43884: Fix asyncio subprocess kill process cleanly when process i…
kumaraditya303 Mar 23, 2022
7c10817
Merge branch 'main' of https://github.com/python/cpython into fix-asy…
kumaraditya303 Mar 24, 2022
0b87f8d
rework
kumaraditya303 Mar 24, 2022
2bd08cf
start from scratch
kumaraditya303 Mar 28, 2022
6afa6df
Merge branch 'main' of https://github.com/python/cpython into fix-asy…
kumaraditya303 Mar 28, 2022
a2a3a1d
rework
kumaraditya303 Mar 28, 2022
e325b6c
remove script
kumaraditya303 Mar 28, 2022
6a37501
Merge branch 'main' into fix-asyncio-subprocess
kumaraditya303 Apr 17, 2022
5fa81d6
Merge branch 'main' into fix-asyncio-subprocess
kumaraditya303 May 4, 2022
74e42a7
Merge branch 'main' into fix-asyncio-subprocess
kumaraditya303 May 27, 2022
7946267
another try
kumaraditya303 May 27, 2022
4f18b7d
Merge branch 'main' into fix-asyncio-subprocess
kumaraditya303 Jul 7, 2022
d896e0b
skip unless sleep
kumaraditya303 Jul 8, 2022
b5de374
📜🤖 Added by blurb_it.
blurb-it[bot] Jul 8, 2022
727bb9e
Merge branch 'main' into fix-asyncio-subprocess
kumaraditya303 Jul 21, 2022
e7eca7a
Merge branch 'main' into fix-asyncio-subprocess
kumaraditya303 Sep 12, 2022
8b2cee6
fix test on windows
kumaraditya303 Oct 1, 2022
53fbc9a
fix comment
kumaraditya303 Oct 1, 2022
77c76dd
Merge branch 'main' of https://github.com/python/cpython into fix-asy…
kumaraditya303 Oct 1, 2022
e223141
Update Misc/NEWS.d/next/Library/2022-07-08-08-39-35.gh-issue-88050.0a…
kumaraditya303 Oct 5, 2022
4ac1184
Merge branch 'main' into fix-asyncio-subprocess
kumaraditya303 Oct 5, 2022
69a9fda
fixup test as Guido suggested
kumaraditya303 Oct 5, 2022
678cc76
Merge branch 'main' into fix-asyncio-subprocess
kumaraditya303 Oct 5, 2022
ba0382d
fix killing on windows
kumaraditya303 Oct 5, 2022
90e7ac5
Merge branch 'fix-asyncio-subprocess' of https://github.com/kumaradit…
kumaraditya303 Oct 5, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions Lib/asyncio/base_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,10 @@ 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():
p.pipe.close()
self._try_finish()

# wake up futures waiting for wait()
for waiter in self._exit_waiters:
if not waiter.cancelled():
waiter.set_result(returncode)
self._exit_waiters = None

async def _wait(self):
"""Wait until the process exit and return the process return code.
Expand All @@ -247,6 +243,11 @@ def _call_connection_lost(self, exc):
try:
self._protocol.connection_lost(exc)
finally:
# wake up futures waiting for wait()
for waiter in self._exit_waiters:
if not waiter.cancelled():
waiter.set_result(self._returncode)
self._exit_waiters = None
self._loop = None
self._proc = None
self._protocol = None
Expand Down
16 changes: 16 additions & 0 deletions Lib/test/test_asyncio/test_subprocess.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os
import shutil
import signal
import sys
import unittest
Expand Down Expand Up @@ -184,6 +185,21 @@ def test_kill(self):
else:
self.assertEqual(-signal.SIGKILL, returncode)

@unittest.skipUnless(shutil.which('sleep'), 'sleep is not available')
def test_kill_issue43884(self):
blocking_shell_command = 'sleep 10000'
kumaraditya303 marked this conversation as resolved.
Show resolved Hide resolved
proc = self.loop.run_until_complete(
asyncio.create_subprocess_shell(blocking_shell_command, stdout=asyncio.subprocess.PIPE)
)
self.loop.run_until_complete(asyncio.sleep(1))
proc.kill()
kumaraditya303 marked this conversation as resolved.
Show resolved Hide resolved
returncode = self.loop.run_until_complete(proc.wait())
if sys.platform == 'win32':
self.assertIsInstance(returncode, int)
# expect 1 but sometimes get 0
kumaraditya303 marked this conversation as resolved.
Show resolved Hide resolved
else:
self.assertEqual(-signal.SIGKILL, returncode)

def test_terminate(self):
args = PROGRAM_BLOCKED
proc = self.loop.run_until_complete(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix :mod:`asyncio` subprocess to kill process cleanly when process is blocked and avoid ``RuntimeError`` when loop is closed. Patch by Kumar Aditya.
kumaraditya303 marked this conversation as resolved.
Show resolved Hide resolved