You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
asyncio proc.kill() and proc.wait() are counter intuitive #119710
On Linux proc.wait() does not return after proc.kill() when there as sub/sub processes.
This only happens when the standard streams are piped, it looks like wait() blocks until the pipes are closed.
importasyncioimportosimporttimeasyncdefmain():
proc=awaitasyncio.create_subprocess_exec(
"/usr/bin/python3", "-c", "import os;os.system('sleep 20')",
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE)
time.sleep(1)
os.system("ps -f --forest --format pid,ppid,pgid,sid,comm")
print(f">> This python proces pid={os.getpid()} has a `python3` child process, which has a `sleep` child process.")
print(f">> Kill the `python3` child process pid={proc.pid}")
proc.kill()
os.system("ps -f --forest --format pid,ppid,pgid,sid,comm")
print(f">> The `python3` child process was killed, the `sleep` process gets orfaned.")
print(f">> Calling `proc.wait()` or `proc.communicate()` does not return until `sleep` process exits.")
awaitproc.wait()
asyncio.run(main())
I don't know if it's a documentation issue or a bug.
It would be more intuitive that proc.wait() after a proc.kill() returned when the process exited, and does not depend on other processes completion.
It would be more intuitive to use have the same behavior when standard streams are piped and when they are not.
It's a surprising feature. communicate() waits until stdout (and stderr) is closed. If a child process spawns "sleep 20", the "sleep 20" inherits stdout and keeps it open for 20 seconds.
Maybe you should create a new process group and kill the whole process group, rather than only killing the "parent" process (parent of "sleep 20"). See the start_new_session parameter of subprocess.Popen and then use os.killpg().
That's what I did in test.libregrtest to make sure that CTRL+C kills immediately all processes, and not only direct child processes.
Indeed the process groups can help, if the child process does not create
another process group. But that is another story.
Here my point is that we should clarify The documentation ton avoid any
surprise. The same behavior applies to proc.wait().
For the record, the crurent documentation says "Wait for child process to
terminate" which looks missleading to me.
I really consider this a pretty bad bug. It is absolutely impossible to wait for the executed process. I specifically wanted to wait for the launched process and not the pipe to be closed. Unfortunately, with the current API that is pretty much impossible.
There is not even a timeout= parameter for wait(), so I am now resorting to having a second task that specifically closes the pipe using proc.stdout._transport._protocol.pipe.close() (after killing the process just in case). I consider this a really nasty workaround, as I just need to wait for the process to quit. However, adding a second child watch is also not really possible …
Considering we have the mess, maybe one could:
Properly document the wait() behaviour
Fix Process.wait() to not immediately return if the process already quit (BaseSubprocessTransport._wait needs to test self._finished not self._returncode to be consistent)
Add a Process.waitpid() function with the expected behaviour that does not wait for the pipes to be closed
Maybe add a close() function that can be used to explicitly close all pipes. Though I suppose that will happen anyway when the object is deleted.
Reacted by hazyfossa, Alisa Sireneva and Anatoly Parshintsev
I've dug around and the bug seems to be a race condition depending on if proc.wait() was called before or after the process exited and could set its returncode.
$python test_asyncio_sub.pymain1 Timed outmain2 finishedException ignored in: <function BaseSubprocessTransport.__del__ at 0x7e6b57e227a0>Traceback (most recent call last): ... # <snip> raise RuntimeError('Event loop is closed')RuntimeError: Event loop is closedException ignored in: <function BaseSubprocessTransport.__del__ at 0x7e6b57e227a0>Traceback (most recent call last): ... # <snip> raise RuntimeError('Event loop is closed')RuntimeError: Event loop is closed
I've checked relevant issues and changes and it seems to boil down to #32073 replacing the notifying of blocked waiters from BaseSubprocessTransport._process_exited with instead closing pipes and delaying notifying readers when the closed pipes eventually leads to the connection_lost callback being triggered.
I've tried simply re-instating the wake-ups in _process_exited (with minor tweaks and confirmed it solves the above as well as allows the rest of the test-suite to still pass (on linux at least)
The issue it tried to solve was that pipes can outlive the process and if they do and you need to wait for both to complete before closing the loop to not have resource errors. The question is really if that is the responsibility of Process.wait) (I'd argue not since we don't guarantee it anyway as above examples show and the synchronous subprocess.Popen works the same way including grandchildren causing problems if you use kill)
@benzea (Or anyone else needing to wait for just the process exit as is currently)
It's possible to use the builtin notification of process exit by subclassing the protocol class asyncio.subprocess.SubprocessStreamProtocol and overriding process_exited to for example set an asyncio.Event you can wait on.
You'll have to use the lower-level AbstractEventLoop.subprocess_*() methods rather than asyncio.create_subprocess_exec though to supply your own protocol factory.
class_ProcessStreamProtocol(asyncio.subprocess.SubprocessStreamProtocol):
""" A subprocess protocol that allows us to be notified of ``process_exited`` """def__init__(self) ->None:
"""Match standard factory for asyncio.create_process"""super().__init__(limit=2**16, loop=asyncio.get_running_loop())
self.exited=asyncio.Event()
defprocess_exited(self) ->None:
super().process_exited()
self.exited.set()
asyncdefmain():
# Use loop.subprocess_shell()/subprocess_exec() rather than their# asyncio.create_subprocess_*() counterparts to get access to# transport/protocol.loop=asyncio.get_running_loop()
transport, protocol=awaitloop.subprocess_exec(
_ProcessStreamProtocol,
[sys.executable, "-c", "print('hello!')"]
)
process=asyncio.subprocess.Process(transport, protocol, loop)
# Should return when process is finished regardless of pipe status.awaitprotocol.exited.wait()
I've dug through the various iterations of this and it seems like simply reinstating the code to wake waiters when the process exits makes it consistent in both cases (neither wait before or after exit are held up by open streams) which also matches uvloop.
All tests passes as well with those changes since the issues with the pipes closing after the loop is closed seems to have been mitigated in other ways.
If that sounds reasonable I'll publish my PR for the fix and would mainly want some help with ensuring reasonable regression tests.
@gvanrossum (you've been involved in a lot of prior review of affected code)
Activity
vstinner commentedon Jun 3, 2024
It's a surprising feature. communicate() waits until stdout (and stderr) is closed. If a child process spawns "sleep 20", the "sleep 20" inherits stdout and keeps it open for 20 seconds.
Maybe you should create a new process group and kill the whole process group, rather than only killing the "parent" process (parent of "sleep 20"). See the start_new_session parameter of subprocess.Popen and then use
os.killpg().That's what I did in test.libregrtest to make sure that CTRL+C kills immediately all processes, and not only direct child processes.
piwicode commentedon Jun 3, 2024
vstinner commentedon Jun 3, 2024
wait() and communicate() are very different.
piwicode commentedon Jun 4, 2024
Indeed. This issue is about proc.wait() documentation. I updated the code snippet to male it clearer.
benzea commentedon Oct 20, 2024
I really consider this a pretty bad bug. It is absolutely impossible to wait for the executed process. I specifically wanted to wait for the launched process and not the pipe to be closed. Unfortunately, with the current API that is pretty much impossible.
There is not even a
timeout=parameter forwait(), so I am now resorting to having a second task that specifically closes the pipe usingproc.stdout._transport._protocol.pipe.close()(after killing the process just in case). I consider this a really nasty workaround, as I just need to wait for the process to quit. However, adding a second child watch is also not really possible …Considering we have the mess, maybe one could:
wait()behaviourProcess.wait()to not immediately return if the process already quit (BaseSubprocessTransport._waitneeds to testself._finishednotself._returncodeto be consistent)Process.waitpid()function with the expected behaviour that does not wait for the pipes to be closedclose()function that can be used to explicitly close all pipes. Though I suppose that will happen anyway when the object is deleted.Revert back to sync for now. Reason: python/cpython#119710
tapetersen commentedon Jun 10, 2026
I've dug around and the bug seems to be a race condition depending on if
proc.wait()was called before or after the process exited and could set its returncode.When run gives:
I've checked relevant issues and changes and it seems to boil down to #32073 replacing the notifying of blocked waiters from
BaseSubprocessTransport._process_exitedwith instead closing pipes and delaying notifying readers when the closed pipes eventually leads to theconnection_lostcallback being triggered.The pipe-closing was then removed by https://github.com/python/cpython/pull/100154/changes since it led to lost output but didn't re-instate the wakeups.
I've tried simply re-instating the wake-ups in
_process_exited(with minor tweaks and confirmed it solves the above as well as allows the rest of the test-suite to still pass (on linux at least)tapetersen commentedon Jun 10, 2026
Checking the rationale for the original change of https://github.com/python/cpython/pull/32073/changes it's explained here
The issue it tried to solve was that pipes can outlive the process and if they do and you need to wait for both to complete before closing the loop to not have resource errors. The question is really if that is the responsibility of
Process.wait)(I'd argue not since we don't guarantee it anyway as above examples show and the synchronoussubprocess.Popenworks the same way including grandchildren causing problems if you use kill)Any thoughts @kumaraditya303
tapetersen commentedon Jun 16, 2026
@benzea (Or anyone else needing to wait for just the process exit as is currently)
It's possible to use the builtin notification of process exit by subclassing the protocol class
asyncio.subprocess.SubprocessStreamProtocoland overridingprocess_exitedto for example set anasyncio.Eventyou can wait on.You'll have to use the lower-level
AbstractEventLoop.subprocess_*()methods rather thanasyncio.create_subprocess_execthough to supply your own protocol factory.pythongh-119710: Fix asyncio subprocess wait() hang on inherited open…
tapetersen commentedon Jun 23, 2026
I've dug through the various iterations of this and it seems like simply reinstating the code to wake waiters when the process exits makes it consistent in both cases (neither wait before or after exit are held up by open streams) which also matches uvloop.
All tests passes as well with those changes since the issues with the pipes closing after the loop is closed seems to have been mitigated in other ways.
If that sounds reasonable I'll publish my PR for the fix and would mainly want some help with ensuring reasonable regression tests.
@gvanrossum (you've been involved in a lot of prior review of affected code)
kumaraditya303 commentedon Jun 23, 2026
I'll try to look at this soon.
pythongh-119710: Let asyncio Process.wait() finish on only process exit.
pythongh-119710: Let asyncio Process.wait() finish on only process exit.
pythongh-119710: Let asyncio Process.wait() finish on only process exit.
tapetersen commentedon Jun 23, 2026
I did some work in #151983 as a suggestion or at least material for discussion.
gvanrossum commentedon Jun 23, 2026
pythongh-119710: Let asyncio Process.wait() finish on only process exit.
gh-119710: fix asyncio Process.wait() to finish on process exit and n…
pythongh-119710: fix asyncio Process.wait() to finish on process exit…
[3.13] gh-119710: fix asyncio Process.wait() to finish on process exi…
[3.14] gh-119710: fix asyncio Process.wait() to finish on process exi…
[3.15] gh-119710: fix asyncio Process.wait() to finish on process exi…