GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-125723: Fix crash with f_locals when generator frame outlive their generator by efimov-mikhail · Pull Request #126956 · python/cpython

Conversation

@efimov-mikhail

@efimov-mikhail efimov-mikhail commented Nov 18, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

It seems that this PR had a problem.
It provides incorrect clearing of generator frame locals without checking any another references on this frame.
In this PR we can see a more accurate way of clearing such a frame.

  • Issue: [3.13] Crash when generator frame proxies outlive their generator #125723

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Eclips4 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@efimov-mikhail
pythongh-125723: Fix crash w/ f_locals when generator frame outlive t…
113a9dc 
…heir generator
@efimov-mikhail
efimov-mikhail requested a review from markshannon as a code owner November 18, 2024 09:29
@bedevere-app bedevere-app Bot added the awaiting review label Nov 18, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Nov 18, 2024
[3.13] Crash when generator frame proxies outlive their generator #125723
Closed
@efimov-mikhail

efimov-mikhail commented Nov 18, 2024

Copy link
Copy Markdown
Member Author

cc @ncoghlan
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Eclips4
Eclips4 requested review from iritkatriel and ncoghlan November 18, 2024 09:32
Eclips4
Eclips4 reviewed Nov 18, 2024
View reviewed changes
Comment thread Lib/test/test_generators.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@efimov-mikhail @Eclips4
Update Lib/test/test_generators.py comment
d43ebe5 
Co-authored-by: Kirill Podoprigora <[email protected]>
markshannon
markshannon approved these changes Nov 22, 2024
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

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.

Looks good.
Nice clean fix and thorough tests.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
efimov-mikhail reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Nov 22, 2024
@efimov-mikhail

efimov-mikhail commented Dec 7, 2024

Copy link
Copy Markdown
Member Author

So, this PR can be merged? Or there's something I should improve?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn

sobolevn commented Jan 19, 2025

Copy link
Copy Markdown
Member

I am planning to merge this in a couple of days if no one has objections.
efimov-mikhail reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
sobolevn
sobolevn approved these changes Jan 19, 2025
View reviewed changes
ncoghlan
ncoghlan approved these changes Jan 20, 2025
View reviewed changes

@ncoghlan ncoghlan left a comment

Copy link
Copy Markdown
Contributor

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.

LGTM, too! Nice detective work, thanks. Minor typo fix suggestion in the NEWS entry.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
efimov-mikhail reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2024-11-18-12-17-45.gh-issue-125723.tW_hFG.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
efimov-mikhail and others added 2 commits January 22, 2025 09:10
@efimov-mikhail @ncoghlan
Update Misc/NEWS.d/next/Core_and_Builtins/2024-11-18-12-17-45.gh-issu…
38423a2 
…e-125723.tW_hFG.rst

Co-authored-by: Alyssa Coghlan <[email protected]>
@efimov-mikhail
Merge branch 'main' into issue_125723_gi_frame_outlive_generator
5943e26
@sobolevn
sobolevn merged commit 8e20e42 into python:main Jan 22, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Jan 22, 2025
@ncoghlan ncoghlan added the needs backport to 3.13 only security fixes label Jun 12, 2025
@miss-islington-app

miss-islington-app Bot commented Jun 12, 2025

Copy link
Copy Markdown

Thanks @efimov-mikhail for the PR, and @sobolevn for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app

miss-islington-app Bot commented Jun 12, 2025

Copy link
Copy Markdown

Sorry, @efimov-mikhail and @sobolevn, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 8e20e42cc63321dacc500d7670bfc225ca04e78b 3.13
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app miss-islington-app Bot assigned sobolevn Jun 12, 2025
@ncoghlan ncoghlan removed the needs backport to 3.13 only security fixes label Jun 12, 2025
@bedevere-app

bedevere-app Bot commented Jun 13, 2025

Copy link
Copy Markdown

GH-135453 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ncoghlan pushed a commit that referenced this pull request Jun 13, 2025
@efimov-mikhail
[3.13] gh-125723: Fix crash with f_locals when generator frame outliv…
e753a7c 
…e their generator (GH-135453)

Backport of 8e20e42 from GH-126956

Closes GH-125723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Eclips4 Eclips4 Eclips4 left review comments
@ncoghlan ncoghlan ncoghlan approved these changes
@sobolevn sobolevn sobolevn approved these changes
@markshannon markshannon markshannon approved these changes
@iritkatriel iritkatriel Awaiting requested review from iritkatriel

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@sobolevn sobolevn

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@efimov-mikhail @sobolevn @ncoghlan @markshannon @Eclips4
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.