Skip to content

bpo-46347: Fix PyEval_EvalCodeEx to correctly cleanup in error paths #30551

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 11, 2022
Merged

bpo-46347: Fix PyEval_EvalCodeEx to correctly cleanup in error paths #30551

merged 1 commit into from
Jan 11, 2022

Conversation

1st1
Copy link
Member

@1st1 1st1 commented Jan 11, 2022 •
edited by bedevere-bot
Loading

@larryhastings
Copy link
Contributor

Yup, that's how it's done!

@1st1
Copy link
Member Author

1st1 commented Jan 11, 2022

I don't think this needs to go in 3.10, so I'll just merge it into the main branch.

@1st1 1st1 added the skip news label Jan 11, 2022
@eirnym
Copy link

eirnym commented Jan 11, 2022

@1st1 Why do you think it doesn't need to go to 3.10 as well?

@1st1
Copy link
Member Author

1st1 commented Jan 11, 2022

@1st1 Why do you think it doesn't need to go to 3.10 as well?

Because any remaining leaks can only happen when the interpreter is about to shutdown anyways so leaking a bit of memory isn't critical. IMO it's not worth it to push more changes to 3.10, but that will ultimately be up to @pablogsal

@1st1 1st1 merged commit 20b5791 into python:main Jan 11, 2022
@bedevere-bot
Copy link

@1st1: Please replace # with GH- in the commit message next time. Thanks!

@1st1 1st1 deleted the fixmem2 branch January 11, 2022 23:37
@pablogsal pablogsal added the needs backport to 3.10 only security fixes label Jan 11, 2022
@miss-islington
Copy link
Contributor

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

@miss-islington
Copy link
Contributor

Sorry, @1st1, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 20b5791ce9b47195ce51cfd5acb223b1ca59cdf0 3.10

gpshead
gpshead reviewed Jan 12, 2022
View reviewed changes
Copy link
Member

@gpshead gpshead left a comment

Choose a reason for hiding this comment

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

post-merge LGTM

@gpshead
Copy link
Member

gpshead commented Jan 12, 2022

#30553 was the manual backport of this and #30554 to 3.10.

@ZeroIntensity ZeroIntensity removed the needs backport to 3.10 only security fixes label Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gpshead gpshead gpshead left review comments

@larryhastings larryhastings larryhastings approved these changes

@vstinner vstinner Awaiting requested review from vstinner

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

@pablogsal pablogsal Awaiting requested review from pablogsal

Assignees

@1st1 1st1

Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@1st1 @larryhastings @eirnym @bedevere-bot @miss-islington @gpshead @pablogsal @the-knights-who-say-ni @ZeroIntensity