-
-
Notifications
You must be signed in to change notification settings - Fork 30.8k
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
With asyncio subprocess, send_signal() and the child process watcher will both call waitpid() #87744
Comments
Under unix, when creating a asyncio subprocess, the child process watcher will call waitpid() to reap the child, but if you call send_signal() (or terminate() or kill() ) on the asyncio subprocess, this will also call waitpid(), causing exactly one of these to fail, as you cannot call waitpid() on a PID more than once. If the send_signal() fails, this doesn't seem much of an issue. I've seen this behaviour with the ThreadedChildWatcher, but possibly other Unix child watchers that use waitpid() suffer from the same problem. The behaviour is racey (it depends on which one completes the waitpid() first), but if you try it enough it will appear:
The output looks like:
This would be expected behaviour if I were explicitly calling waitpid() myself (ie I'm shooting my own foot, so I'd deserve the bad behaviour), but that's not the case here nor any other exotic code. |
Did you find a workaround for this? I tried the different watcher classes |
This one is new to me. I’m not sure how to approach it yet. |
@graingert Any idea here? |
Oops, I see the problem isn't an exception so much as a lost exit status. I'm guessing we need to improve the communication between asyncio.subprocess.Process.wait() and the child watchers. |
On my TODO list but low priority, I can review a PR fixing this though. |
Child watcher will be removed in 3.14 at which point this will have to be reworked again so I'll fix this after child watchers are removed, it will be much simpler then. |
Revisit after completion of #120804 |
) asyncio earlier relied on subprocess module to send signals to the process, this has some drawbacks one being that subprocess module unnecessarily calls waitpid on child processes and hence it races with asyncio implementation which internally uses child watchers. To mitigate this, now asyncio sends signals directly to the process without going through the subprocess on non windows systems. On Windows it fallbacks to subprocess module handling but on windows there are no child watchers so this issue doesn't exists altogether.
…pythonGH-121126) asyncio earlier relied on subprocess module to send signals to the process, this has some drawbacks one being that subprocess module unnecessarily calls waitpid on child processes and hence it races with asyncio implementation which internally uses child watchers. To mitigate this, now asyncio sends signals directly to the process without going through the subprocess on non windows systems. On Windows it fallbacks to subprocess module handling but on windows there are no child watchers so this issue doesn't exists altogether. (cherry picked from commit bd473aa) Co-authored-by: Kumar Aditya <kumaraditya@python.org>
GH-121126) (#121194) gh-87744: fix waitpid race while calling send_signal in asyncio (GH-121126) asyncio earlier relied on subprocess module to send signals to the process, this has some drawbacks one being that subprocess module unnecessarily calls waitpid on child processes and hence it races with asyncio implementation which internally uses child watchers. To mitigate this, now asyncio sends signals directly to the process without going through the subprocess on non windows systems. On Windows it fallbacks to subprocess module handling but on windows there are no child watchers so this issue doesn't exists altogether. (cherry picked from commit bd473aa) Co-authored-by: Kumar Aditya <kumaraditya@python.org>
…python#121126) asyncio earlier relied on subprocess module to send signals to the process, this has some drawbacks one being that subprocess module unnecessarily calls waitpid on child processes and hence it races with asyncio implementation which internally uses child watchers. To mitigate this, now asyncio sends signals directly to the process without going through the subprocess on non windows systems. On Windows it fallbacks to subprocess module handling but on windows there are no child watchers so this issue doesn't exists altogether.
…python#121126) asyncio earlier relied on subprocess module to send signals to the process, this has some drawbacks one being that subprocess module unnecessarily calls waitpid on child processes and hence it races with asyncio implementation which internally uses child watchers. To mitigate this, now asyncio sends signals directly to the process without going through the subprocess on non windows systems. On Windows it fallbacks to subprocess module handling but on windows there are no child watchers so this issue doesn't exists altogether.
…python#121126) asyncio earlier relied on subprocess module to send signals to the process, this has some drawbacks one being that subprocess module unnecessarily calls waitpid on child processes and hence it races with asyncio implementation which internally uses child watchers. To mitigate this, now asyncio sends signals directly to the process without going through the subprocess on non windows systems. On Windows it fallbacks to subprocess module handling but on windows there are no child watchers so this issue doesn't exists altogether.
…_signal in asyncio (python#121126)" This reverts the non-test changes of commit bd473aa.
This is a major issue for us, I am surprised that this issue is not getting more attention. It can be reproduced with the test case above on Python 3.9 and 3.10; please add corresponding labels. If I understand it correctly, the problem was introduced with https://bugs.python.org/issue38630 . Is it correct that the only workaround is to use os.kill instead of proc.terminate/proc.kill? |
Temper your rhetoric, please. Just upgrade to Python 3.13. |
Easier said than done. In our case the situation is the same as for most users, I believe: Python is provided by the platform. In our case, Yocto Kirkstone, which includes Python 3.10. In theory we could upgrade to the next Yocto LTS release which is Scarthgap, but it only includes Python 3.12; not 3.13. The workaround with os.kill seems to work, though, at least with SafeChildWatcher. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
Linked PRs
The text was updated successfully, but these errors were encountered: