Skip to content

gh-145376: Fix reference leak in _lprof.c#145539

Merged
vstinner merged 1 commit intopython:mainfrom
eendebakpt:refleak_lsprof
Mar 9, 2026
Merged

gh-145376: Fix reference leak in _lprof.c#145539
vstinner merged 1 commit intopython:mainfrom
eendebakpt:refleak_lsprof

Conversation

@eendebakpt
Copy link
Contributor

@eendebakpt eendebakpt commented Mar 5, 2026

Skipping news. This only affects cases where people manually replace the sys.monitoring.MISSING sentinel.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

The get_cfunc_from_callable() fix is surprising: PyCFunction_Check() is always true and so nobody noticed the reference leak before? Is it possible to trigger the leak with a reproducer?

@eendebakpt
Copy link
Contributor Author

LGTM.

The get_cfunc_from_callable() fix is surprising: PyCFunction_Check() is always true and so nobody noticed the reference leak before? Is it possible to trigger the leak with a reproducer?

I cannot create a reproducer from the Python side. The check Py_TYPE(callable) == &PyMethodDescr_Type ensures callable->tp_descr_get is always method_get from descrobject.c which (currently) returns a PyCFunction_Type instance or NULL.

A C-extension could override the tp_descr_get field (override as in setting a new tp_descr_get, not subclassing PyMethodDescr_Type) and return a non-PyCFunction_Type object, but under the consenting adult principle I do not think we have to guard for that.

I can leave this part as is, change the PyCFunction_Check into an assert or revert to the current main version.

Pinging @gaogaotiantian as the one who introduced the code in #103534

@vstinner vstinner merged commit 201e183 into python:main Mar 9, 2026
56 checks passed
@vstinner
Copy link
Member

vstinner commented Mar 9, 2026

Merged. Thanks for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants