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-96827: Don't touch closed loops from executor threads #96837

Merged
merged 4 commits into from
Sep 30, 2022

Conversation

gvanrossum
Copy link
Member

It's possible for an event loop's default executor to survive the loop. When the executor is finally shut down, a few things try to wake up the loop, and nasty RuntimeErrors are printed. But there's nothing the executor can do about that, so let's just not even try.

This is a draft PR because I haven't figured out a way to test this properly. (There's a repro in the issue, GH-96827.)

@gvanrossum gvanrossum marked this pull request as ready for review September 29, 2022 22:06
@gvanrossum
Copy link
Member Author

At this point (after investing over an hour of my time) I feel it's not worth it to try and come up with a test that reproduces the original issue. The changes appear straightforward enough: in certain edge cases don't attempt to set a future's state when its loop is already closed.

Copy link
Contributor

@kumaraditya303 kumaraditya303 left a comment

Choose a reason for hiding this comment

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

LGTM

@kumaraditya303 kumaraditya303 added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Sep 30, 2022
@gvanrossum gvanrossum merged commit e9d6376 into python:main Sep 30, 2022
@miss-islington
Copy link
Contributor

Thanks @gvanrossum for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@gvanrossum gvanrossum deleted the asyncio-thread-late branch September 30, 2022 19:55
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Sep 30, 2022
@bedevere-bot
Copy link

GH-97690 is a backport of this pull request to the 3.11 branch.

@bedevere-bot
Copy link

GH-97691 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Sep 30, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 30, 2022
…nGH-96837)

* When chaining futures, skip callback if loop closed.
* When shutting down an executor, don't wake a closed loop.
(cherry picked from commit e9d6376)

Co-authored-by: Guido van Rossum <guido@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 30, 2022
…nGH-96837)

* When chaining futures, skip callback if loop closed.
* When shutting down an executor, don't wake a closed loop.
(cherry picked from commit e9d6376)

Co-authored-by: Guido van Rossum <guido@python.org>
miss-islington added a commit that referenced this pull request Sep 30, 2022
* When chaining futures, skip callback if loop closed.
* When shutting down an executor, don't wake a closed loop.
(cherry picked from commit e9d6376)

Co-authored-by: Guido van Rossum <guido@python.org>
miss-islington added a commit that referenced this pull request Sep 30, 2022
* When chaining futures, skip callback if loop closed.
* When shutting down an executor, don't wake a closed loop.
(cherry picked from commit e9d6376)

Co-authored-by: Guido van Rossum <guido@python.org>
serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Oct 2, 2022
…n#96837)

* When chaining futures, skip callback if loop closed.
* When shutting down an executor, don't wake a closed loop.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants