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

gh-100762: Fix optimization in gen_close by iritkatriel · Pull Request #111069 · python/cpython · GitHub

/ cpython Public

gh-100762: Fix optimization in gen_close - #111069

Merged
iritkatriel merged 6 commits into
python:mainfrom
iritkatriel:refleak
Oct 25, 2023
ConversationCommits (6)ChecksFiles changed
Merged

gh-100762: Fix optimization in gen_close #111069
iritkatriel merged 6 commits into
python:mainfrom
iritkatriel:refleak

Conversation

iritkatriel commented Oct 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

In f02fa64 lines 383-389 were added, with returns that do not decref yf. I think this is ok because yf is always NULL in these cases. Adding the assertions.

Also, the optimization for the case of exception_handler_depth== 1 is not working because op.code is checked instead of op.arg.

bedevere-app Bot mentioned this pull request Oct 19, 2023
Closed
iritkatriel added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.12 only security fixes needs backport to 3.12 only security fixes and removed 3.12 only security fixes type-bug An unexpected behavior, bug, or error needs backport to 3.12 only security fixes labels Oct 19, 2023
iritkatriel changed the title gh-100762: fix refleak in gen_close gh-100762: assert that yf is NULL in gen_close in the cases where it is not DECREFed Oct 19, 2023
iritkatriel changed the title gh-100762: assert that yf is NULL in gen_close in the cases where it is not DECREFed gh-100762: Fix optimization in gen_close and assert that yf is NULL in the cases where it is not DECREFed Oct 19, 2023
iritkatriel commented Oct 19, 2023
View reviewed changes
Comment thread Objects/genobject.c Outdated
iritkatriel mentioned this pull request Oct 19, 2023
Merged
iritkatriel reopened this Oct 20, 2023
iritkatriel mentioned this pull request Oct 20, 2023
Closed
iritkatriel marked this pull request as draft October 20, 2023 12:48

markshannon commented Oct 24, 2023

Copy link
Copy Markdown
Member

This looks good.
The failure is in a new test, that didn't exist when the unintentional change from oparg to opcode was made. https://github.com/python/cpython/pull/101912/files#diff-2a0c449b68605ebd0872fd232e60ce7e838a77782a6d2e364764f99514fb508aR376

Just change the test.

markshannon commented Oct 24, 2023

Copy link
Copy Markdown
Member

One minor efficiency improvement that could be made is to not call _PyGen_yf(gen) until after the checks for FRAME_CREATED and FRAME_COMPLETED.

iritkatriel marked this pull request as ready for review October 24, 2023 11:11
iritkatriel added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 24, 2023
iritkatriel changed the title gh-100762: Fix optimization in gen_close and assert that yf is NULL in the cases where it is not DECREFed gh-100762: Fix optimization in gen_close Oct 24, 2023
iritkatriel added needs backport to 3.12 only security fixes and removed needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 24, 2023
iritkatriel merged commit 0db2517 into python:main Oct 25, 2023
markshannon mentioned this pull request Jan 5, 2024
Closed
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
pythongh-100762: Fix optimization in gen_close (python#111069)
3baff26

cdce8p commented Feb 22, 2024

Copy link
Copy Markdown
Contributor

Would it be possible / make sense to cherry-pick this change onto 3.12?

For pylint / astroid we're seeing ValueError: generator already executing errors since 3.12.0b1. We haven't been able to create a standalone reproducer - which is also the reason I haven't opened a bug report yet. However testing either 3.13.0a2 or 3.12 with this change cherry-picked on top does seem to resolve it as best as I can tell.

pylint-dev/pylint#9138

cdce8p mentioned this pull request Feb 22, 2024
Closed

iritkatriel commented Feb 22, 2024

Copy link
Copy Markdown
Member Author

Would it be possible / make sense to cherry-pick this change onto 3.12?

I think so. I'll make a PR.

iritkatriel added the needs backport to 3.12 only security fixes label Feb 22, 2024

miss-islington-app Bot commented Feb 22, 2024

Copy link
Copy Markdown

Thanks @iritkatriel for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 22, 2024
pythongh-100762: Fix optimization in gen_close (pythonGH-111069)
fdbb567 
(cherry picked from commit 0db2517)

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>

bedevere-app Bot commented Feb 22, 2024

Copy link
Copy Markdown

GH-115818 is a backport of this pull request to the 3.12 branch.

bedevere-app Bot removed the needs backport to 3.12 only security fixes label Feb 22, 2024
iritkatriel added a commit that referenced this pull request Feb 22, 2024
[3.12] gh-100762: Fix optimization in gen_close (GH-111069) (#115818)
eb4774d 
gh-100762: Fix optimization in gen_close  (GH-111069)
(cherry picked from commit 0db2517)

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
iritkatriel added a commit to iritkatriel/cpython that referenced this pull request Jun 13, 2024
Revert "[3.12] pythongh-100762: Fix optimization in gen_close (python…
47c7a30 
…GH-111069) (python#115818)"

This reverts commit eb4774d.
iritkatriel mentioned this pull request Jun 13, 2024
Merged
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
pythongh-100762: Fix optimization in gen_close (python#111069)
bba1850
gaogaotiantian mentioned this pull request Jun 12, 2025
Closed
markshannon mentioned this pull request Oct 21, 2025
Closed
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

markshannon markshannon approved these changes

Assignees

No one assigned

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs) skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL