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-87744: fix waitpid race while calling send_signal in asyncio #121126

Merged
merged 6 commits into from
Jul 1, 2024

Conversation

kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Jun 28, 2024

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.

@kumaraditya303 kumaraditya303 added type-bug An unexpected behavior, bug, or error topic-asyncio labels Jun 28, 2024
@kumaraditya303 kumaraditya303 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 28, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit e0d6de5 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 28, 2024
@kumaraditya303 kumaraditya303 marked this pull request as ready for review June 29, 2024 05:55
@kumaraditya303 kumaraditya303 added the needs backport to 3.13 bugs and security fixes label Jun 29, 2024
@gvanrossum gvanrossum removed their request for review June 29, 2024 07:00
@kumaraditya303 kumaraditya303 added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 30, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit 9136f3f 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 30, 2024
@kumaraditya303
Copy link
Contributor Author

Buildbots have passed I'm merging this, will keep an eye on buildbots.

@kumaraditya303 kumaraditya303 merged commit bd473aa into python:main Jul 1, 2024
109 of 117 checks passed
@miss-islington-app
Copy link

Thanks @kumaraditya303 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 1, 2024
…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 <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jul 1, 2024

GH-121194 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jul 1, 2024
kumaraditya303 added a commit that referenced this pull request Jul 1, 2024
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 <[email protected]>
Akasurde pushed a commit to Akasurde/cpython that referenced this pull request Jul 3, 2024
…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.
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
…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.
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants