Skip to content

gh-86802: Fix asyncio memory leak; shielded tasks where cancelled log… #134331

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

Merged
merged 4 commits into from
May 20, 2025

Conversation

ChristianHrs
Copy link
Contributor

@ChristianHrs ChristianHrs commented May 20, 2025

@python-cla-bot
Copy link

python-cla-bot bot commented May 20, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app
Copy link

bedevere-app bot commented May 20, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@@ -972,10 +983,16 @@ def _inner_done_callback(inner, cur_task=cur_task):
else:
outer.set_result(inner.result())


def _outer_done_callback(outer):
if not inner.done():
inner.remove_done_callback(_inner_done_callback)
Copy link
Contributor

Choose a reason for hiding this comment

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

Before this PR, this line made the future_discard_from_awaited_by never execute for cases when the outer shield was cancelled. That was a memory leak, which is now fixed.

Comment on lines +993 to +994
if cur_task is not None:
inner.add_done_callback(_clear_awaited_by_callback)
Copy link
Contributor

Choose a reason for hiding this comment

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

Now we're only adding the callback to future_discard_from_awaited_by when there is a current task. Nice.

@ambv ambv merged commit f695eca into python:main May 20, 2025
39 checks passed
@ambv ambv added the needs backport to 3.14 bugs and security fixes label May 20, 2025
@miss-islington-app
Copy link

Thanks @ChristianHrs for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 20, 2025
… once through the exception handler (pythongh-134331)

(cherry picked from commit f695eca)

Co-authored-by: Christian Harries <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented May 20, 2025

GH-134343 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 20, 2025
@pablogsal
Copy link
Member

@ambv you are indeed a provider

ambv added a commit that referenced this pull request May 20, 2025
…g once through the exception handler (gh-134331) (gh-134343)

(cherry picked from commit f695eca)

Co-authored-by: Christian Harries <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
ChristianHrs added a commit to ChristianHrs/cpython that referenced this pull request May 20, 2025
…ions log once through the exception handler (pythongh-134331)"

This reverts commit f695eca.
ChristianHrs added a commit to ChristianHrs/cpython that referenced this pull request May 21, 2025
…ions log once through the exception handler (pythongh-134331)"

This reverts commit f695eca.
@ChristianHrs ChristianHrs deleted the gh-86802 branch May 21, 2025 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants