You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
PyFunction_GetAnnotations is probably causing ref leaks #97943
In issue #30409PyFunction_GetAnnotations was fixed so it wouldn't return a tuple. That's good.
But this change changed the return value. PyFunction_GetAnnotations now returns a new reference to the annotations dict. But according to the documentation, it should return a borrowed reference.
If users only read the documentation, and assume that it's correct, they'll leak a reference to the annotation dict.
We could fix this by changing the documentation. However, all the other PyFunction_Get* functions also return borrowed references. So I think the correct fix is to remove the incref.
changed the title [-]PyFunction_GetAnnotations should be causing ref leaks[/-][+]PyFunction_GetAnnotations is probably causing ref leaks[/+]on Oct 5, 2022
I agree that changing back to a borrowed reference to the current version and backporting that behavior it is correct. Many more people would read the documentation and assume it's correct than would read the source code and know that it's a new reference that they need to manage.
I didn't do a full forensic analysis, but I think what happened here is:
func_get_annotations (the getter/setter) returns a fresh reference.
func_get_annotations was changed to call this code that says "oh! if it's a tuple, turn it into a dict." this function returns a new reference.
PyFunction_GetAnnotations wasn't changed, it was returning the dict.
the bug fix was, "oh! just have it call the same function to turn it from a tuple into a dict." but this function returns a new reference, and PyFunction_GetAnnotations shouldn't return a new reference.
Activity
larryhastings commentedon Oct 5, 2022
IDK how to tag people, so I'll just do it in a comment:
@pablogsal @methane
[-]PyFunction_GetAnnotations should be causing ref leaks[/-][+]PyFunction_GetAnnotations is probably causing ref leaks[/+]larryhastings commentedon Oct 5, 2022
Also, I consider this enough of a bug fix that it should be backported to versions still getting bug fixes.
pythongh-97943: PyFunction_GetAnnotations should return a borrowed re…
ericvsmith commentedon Oct 6, 2022
I agree that changing back to a borrowed reference to the current version and backporting that behavior it is correct. Many more people would read the documentation and assume it's correct than would read the source code and know that it's a new reference that they need to manage.
larryhastings commentedon Oct 6, 2022
I didn't do a full forensic analysis, but I think what happened here is:
gh-97943: PyFunction_GetAnnotations should return a borrowed referenc…
pythongh-97943: PyFunction_GetAnnotations should return a borrowed re…
pythongh-97943: PyFunction_GetAnnotations should return a borrowed re…
pythongh-97943: PyFunction_GetAnnotations should return a borrowed re…
gh-97943: PyFunction_GetAnnotations should return a borrowed referenc…
Merge branch 'main' into dictwatch
[3.10] gh-97943: PyFunction_GetAnnotations should return a borrowed r…
Merge branch 'main' into typewatch
pythongh-97943: PyFunction_GetAnnotations should return a borrowed re…
hauntsaninja commentedon Oct 28, 2022
Thanks for spotting, looks like it's been fixed and backported