FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-105288: wake up exit waiters after sub-process exits by keuin · Pull Request #105289 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-105288: wake up exit waiters after sub-process exits - #105289

Closed
keuin wants to merge 3 commits into
python:mainfrom
keuin:bugfix-asyncio-issue-gh-105288
ConversationCommits (3)ChecksFiles changed
Closed

gh-105288: wake up exit waiters after sub-process exits#105289
keuin wants to merge 3 commits into
python:mainfrom
keuin:bugfix-asyncio-issue-gh-105288

Conversation

keuin commented Jun 4, 2023 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown

bedevere-bot mentioned this pull request Jun 4, 2023
Closed

ghost commented Jun 4, 2023 •
edited by ghost
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

keuin force-pushed the bugfix-asyncio-issue-gh-105288 branch from 210bfce to 54bbc06 Compare June 4, 2023 19:12
keuin force-pushed the bugfix-asyncio-issue-gh-105288 branch from 54bbc06 to a54ad6f Compare June 5, 2023 14:05

gvanrossum commented Jun 5, 2023

Copy link
Copy Markdown
Member

@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.

kumaraditya303 commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

As evident by the failing CI this is wrong.

gvanrossum commented Jun 8, 2023

Copy link
Copy Markdown
Member

@kumaraditya303 Whoa. Why close the PR instead of requesting changes?

keuin commented Jun 8, 2023

Copy link
Copy Markdown
Author

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.

kumaraditya303 reopened this Jun 8, 2023
kumaraditya303 requested changes Jun 8, 2023
View reviewed changes

kumaraditya303 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

See my explanation on the issue

bedevere-bot commented Jun 8, 2023

Copy link
Copy Markdown

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

Hels15 commented Aug 18, 2023

Copy link
Copy Markdown
Contributor

What's going on here?

graingert reviewed Oct 22, 2024
View reviewed changes
Comment thread Lib/test/test_asyncio/test_subprocess.py
# 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())

graingert Oct 22, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

the problem is in asyncio.run - so this doesn't actually test the issue

keuin commented Oct 23, 2024

Copy link
Copy Markdown
Author

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.
I'm glad to see others want to fix the similar issue. I want to wait to see if it can fix this too.

keuin closed this Oct 23, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

graingert graingert left review comments
kumaraditya303 kumaraditya303 requested changes
1st1 Awaiting requested review from 1st1 1st1 is a code owner
asvetlov Awaiting requested review from asvetlov asvetlov is a code owner
gvanrossum Awaiting requested review from gvanrossum
willingc Awaiting requested review from willingc willingc is a code owner

Assignees

No one assigned

Labels

awaiting changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL