Signaling an asyncio subprocess might raise ProcessLookupError, even if you haven't called .wait() yet #88319

Activity

1. syntaxcoloring commented on May 16, 2021

   

   syntaxcoloringmannequin

   on May 16, 2021

   MannequinAuthor

   More actions

   # Summary

   Basic use of asyncio.subprocess.Process.terminate() can raise a ProcessLookupError, depending on the timing of the subprocess's exit.

   I assume (but haven't checked) that this problem extends to .kill() and .send_signal().

   This breaks the expected POSIX semantics of signaling and waiting on a process. See the "Expected behavior" section.

   # Test case

   I've tested this on macOS 11.2.3 with Python 3.7.9 and Python 3.10.0a7, both installed via pyenv.

   import asyncio
   import sys
   
   # Tested with:
   # asyncio.ThreadedChildWatcher (3.10.0a7  only)
   # asyncio.MultiLoopChildWatcher (3.10.0a7 only)
   # asyncio.SafeChildWatcher (3.7.9 and 3.10.0a7)
   # asyncio.FastChildWatcher (3.7.9 and 3.10.0a7)
   # Not tested with asyncio.PidfdChildWatcher because I'm not on Linux.
   WATCHER_CLASS = asyncio.FastChildWatcher
   
   async def main():
       # Dummy command that should be executable cross-platform.
       process = await asyncio.subprocess.create_subprocess_exec(
           sys.executable, "--version"
       )
       
       for i in range(20):
           # I think the problem is that the event loop opportunistically wait()s
           # all outstanding subprocesses on its own. Do a bunch of separate
           # sleep() calls to give it a bunch of chances to do this, for reliable
           # reproduction.
           #
           # I'm not sure if this is strictly necessary for the problem to happen.
           # On my machine, the problem also happens with a single sleep(2.0).
           await asyncio.sleep(0.1)
       
       process.terminate() # This unexpectedly errors with ProcessLookupError.
   
       print(await process.wait())
   
   asyncio.set_child_watcher(WATCHER_CLASS())
   asyncio.run(main())
   

   The process.terminate() call raises a ProcessLookupError:

   Traceback (most recent call last):
     File "kill_is_broken.py", line 29, in <module>
       asyncio.run(main())
     File "/Users/maxpm/.pyenv/versions/3.7.9/lib/python3.7/asyncio/runners.py", line 43, in run
       return loop.run_until_complete(main)
     File "/Users/maxpm/.pyenv/versions/3.7.9/lib/python3.7/asyncio/base_events.py", line 587, in run_until_complete
       return future.result()
     File "kill_is_broken.py", line 24, in main
       process.terminate() # This errors with ProcessLookupError.
     File "/Users/maxpm/.pyenv/versions/3.7.9/lib/python3.7/asyncio/subprocess.py", line 131, in terminate
       self._transport.terminate()
     File "/Users/maxpm/.pyenv/versions/3.7.9/lib/python3.7/asyncio/base_subprocess.py", line 150, in terminate
       self._check_proc()
     File "/Users/maxpm/.pyenv/versions/3.7.9/lib/python3.7/asyncio/base_subprocess.py", line 143, in _check_proc
       raise ProcessLookupError()
   ProcessLookupError
   

   # Expected behavior and discussion

   Normally, with POSIX semantics, the wait() syscall tells the operating system that we won't send any more signals to that process, and that it's safe for the operating system to recycle that process's PID. This comment from Jack O'Connor on another issue explains it well: https://bugs.python.org/issue40550#msg382427

   So, I expect that on any given asyncio.subprocess.Process, if I call .terminate(), .kill(), or .send_signal() before I call .wait(), then:

   • It should not raise a ProcessLookupError.
   • The asyncio internals shouldn't do anything with a stale PID. (A stale PID is one that used to belong to our subprocess, but that we've since consumed through a wait() syscall, allowing the operating system to recycle it).

   asyncio internals are mostly over my head. But I think the problem is that the event loop opportunistically calls the wait() syscall on our child processes. So, as implemented, there's a race condition. If the event loop's wait() syscall happens to come before my .terminate() call, my .terminate() call will raise a ProcessLookupError.

   So, as a corollary to the expectations listed above, I think the implementation details should be either:

   • Ideally, the asyncio internals should not call syscall wait() on a process until I call wait() on that process.
   • Failing that, .terminate(), .kill() and .send_signal() should should no-op if the asyncio internals have already called .wait() on that process.
2. syntaxcoloring

   added

   3.7 (EOL)end of lifeend of life

   3.10 (EOL)end of lifeend of life

   topic-asyncio

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on May 16, 2021
3. syntaxcoloring

   changed the title [-]Signaling an asyncio subprocess raises ProcessLookupError, depending on timing[/-] [+]Signaling an asyncio subprocess might raise ProcessLookupError, even if you haven't called .wait() yet[/+] on May 16, 2021
4. syntaxcoloring

   changed the title [-]Signaling an asyncio subprocess raises ProcessLookupError, depending on timing[/-] [+]Signaling an asyncio subprocess might raise ProcessLookupError, even if you haven't called .wait() yet[/+] on May 16, 2021
5. ezio-melotti

   transferred this issue fromon Apr 10, 2022
6. ezio-melotti

   added this to asyncioon Jun 20, 2022
7. ezio-melotti

   moved this to Todo in asyncioon Jul 17, 2022
8. kumaraditya303

   removed

   3.10 (EOL)end of lifeend of life

   3.7 (EOL)end of lifeend of life

   on Oct 18, 2022
9. kumaraditya303 commented on Oct 23, 2022

   

   kumaraditya303

   on Oct 23, 2022

   Contributor

   More actions

   Duplicate of #87744
10. kumaraditya303

    marked this as a duplicate of With asyncio subprocess, send_signal() and the child process watcher will both call waitpid() #87744 on Oct 23, 2022
11. kumaraditya303

    closed this as not plannedon Oct 23, 2022
12. Repository owner moved this from Todo to Done in asyncioon Oct 23, 2022