-
-
Notifications
You must be signed in to change notification settings - Fork 31.3k
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-105288: wake up exit waiters after sub-process exits #105289
Conversation
210bfce
to
54bbc06
Compare
54bbc06
to
a54ad6f
Compare
@kumaraditya303 Once you are convinced of the validity of the issue (which I expect you will be), will you review ad merge this PR? I was on vacation for 10 days and am way behind. |
As evident by the failing CI this is wrong. |
@kumaraditya303 Whoa. Why close the PR instead of requesting changes? |
I will fix my test in these days. Sorry because I'm not familiar with the test framework and testing a frozen event loop is much harder than usual. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my explanation on the issue
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase And if you don't make the requested changes, you will be poked with soft cushions! |
What's going on here? |
# We have to run the event loop in a separate thread, | ||
# to make sure we can fail in the main thread | ||
# and successfully notify the test driver. | ||
self.loop.run_until_complete(main()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem is in asyncio.run - so this doesn't actually test the issue
I think this issue is a duplicate of others. The misuse of argument shows some deep resource leak, but is hard to find the root cause. |
asyncio.wait
hang if there are async subprocess tasks #105288