Skip to content

Conversation

@JelleZijlstra
Copy link
Member

@JelleZijlstraJelleZijlstra commented Oct 17, 2024

Copy link
Member

@ZeroIntensityZeroIntensity left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@ZeroIntensity
Copy link
Member

Is there any backport needed here? I'm not sure if the original PR made it in 3.13

@JelleZijlstra
Copy link
MemberAuthor

No, this is a PEP 649 change and is only on main.

Copy link
Member

@AA-TurnerAA-Turner left a comment

Choose a reason for hiding this comment

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

Thanks Jelle!

A

@zware
Copy link
Member

Heads-up: this appears to have caused some refleak buildbot failures, see for example https://buildbot.python.org/#/builders/1613/builds/116

zware added a commit to zware/cpython that referenced this pull request Oct 17, 2024
zware added a commit that referenced this pull request Oct 17, 2024
@ZeroIntensity
Copy link
Member

Actually, on a second look, this part is technically unsafe:

else{Py_DECREF(dict); returnPyDict_SetItem(dict, name, value)}

If the dictionary's reference count is 1, then this will result in a use-after-free. Though, I guess that's never possible because the dictionary will be stored on the instance as well. In hindsight, there should have either been an assertion (or just not do it in the first place), but oh well.

ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@JelleZijlstra@ZeroIntensity@zware@AA-Turner