Skip to content

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

Merged
1st1 merged 1 commit intopython:mainfrom
1st1:fixmem2
Jan 11, 2022
Merged

bpo-46347: Fix PyEval_EvalCodeEx to correctly cleanup in error paths#30551
1st1 merged 1 commit intopython:mainfrom
1st1:fixmem2

Conversation

@1st1
Copy link
Member

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

@larryhastings
Copy link
Contributor

larryhastings commented Jan 11, 2022

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

bedevere-bot commented Jan 11, 2022

@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

miss-islington commented Jan 11, 2022

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

@miss-islington
Copy link
Contributor

miss-islington commented Jan 11, 2022

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