Skip to content

gh-109216: Fix possible memory leak in BUILD_MAP #109257

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
Sep 12, 2023
Merged

gh-109216: Fix possible memory leak in BUILD_MAP #109257

merged 1 commit into from
Sep 12, 2023

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Sep 11, 2023 •
edited by bedevere-bot
Loading

_PyDict_FromItems can only fail with MemoryError and several other errors which can be considered as disasters, not a regular control flow.

But, it should be now in sync with BUILD_CONST_KEY_MAP

@sobolevn sobolevn requested a review from gvanrossum September 11, 2023 09:45
@bedevere-bot bedevere-bot mentioned this pull request Sep 11, 2023
Closed
gvanrossum
gvanrossum approved these changes Sep 12, 2023
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Yeah, that's a good catch. This bug looks quite old, it's in 3.11, and I think it existed even before this code was refactored in #31564.

@kumaraditya303 kumaraditya303 merged commit 247ee1b into python:main Sep 12, 2023
@kumaraditya303
Copy link
Contributor

kumaraditya303 commented Sep 12, 2023

@sobolevn Can you manually backport this to older branches? Thanks

@sobolevn
Copy link
Member Author

sobolevn commented Sep 12, 2023

Sure, will do later today.

@sobolevn sobolevn added needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Sep 12, 2023
@miss-islington
Copy link
Contributor

miss-islington commented Sep 12, 2023

Thanks @sobolevn for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

miss-islington commented Sep 12, 2023

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

@miss-islington
Copy link
Contributor

miss-islington commented Sep 12, 2023

Sorry, @sobolevn and @kumaraditya303, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 247ee1bf841524667f883ebba5e343101f609026 3.11

@miss-islington
Copy link
Contributor

miss-islington commented Sep 12, 2023

Sorry, @sobolevn and @kumaraditya303, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 247ee1bf841524667f883ebba5e343101f609026 3.12

This was referenced Sep 12, 2023
Merged
Merged
vstinner pushed a commit to vstinner/cpython that referenced this pull request Sep 13, 2023
@sobolevn @vstinner
pythongh-109216: Fix possible memory leak in BUILD_MAP (python#109257)
86f7be0
@ZeroIntensity ZeroIntensity removed needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

Assignees

@kumaraditya303 kumaraditya303

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sobolevn @kumaraditya303 @miss-islington @gvanrossum @bedevere-bot @ZeroIntensity