Skip to content
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.

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

gh-117142: Slightly hacky fix for memory leak of StgInfo #119424

Merged
merged 4 commits into from
May 23, 2024

Conversation

encukou
Copy link
Member

@encukou encukou commented May 22, 2024

Add a function that inlines PyObject_GetTypeData and skips type-checking, so it doesn't need access to the CType_Type object. This will break if the memory layout changes, but should be an acceptable solution to enable ctypes in subinterpreters in Python 3.13.

This is a less invasive version of #118139.

>python_d -X showrefcount -c "import ctypes"
[188 refs, 132 blocks]  # before
[0 refs, 0 blocks]      # after

Add a funciton that inlines PyObject_GetTypeData and skips
type-checking, so it doesn't need access to the CType_Type object.
This will break if the memory layout changes, but should
be an acceptable solution to enable ctypes in subinterpreters in
Python 3.13.
@encukou encukou changed the title gh-117142: Slightliy hacky fix for memory leak of StgInfo gh-117142: Slightly hacky fix for memory leak of StgInfo May 22, 2024
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

@neonene
Copy link
Contributor

neonene commented May 22, 2024

get_module_state_by_def_final function can be removed from ctypes.h.

@encukou encukou enabled auto-merge (squash) May 22, 2024 19:21
@encukou encukou disabled auto-merge May 22, 2024 19:21
@encukou encukou enabled auto-merge (squash) May 22, 2024 19:21
@encukou
Copy link
Member Author

encukou commented May 22, 2024

It can! Thank you!

@neonene
Copy link
Contributor

neonene commented May 23, 2024

Regarding the CI failure on Windows (free-threading x86 debug), PyType_Type.tp_basicsize is 476, whereas it is 464 with the GIL enabled (success).

@encukou encukou merged commit a192547 into python:main May 23, 2024
34 checks passed
@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 23, 2024
…nGH-119424)

Add a funciton that inlines PyObject_GetTypeData and skips
type-checking, so it doesn't need access to the CType_Type object.
This will break if the memory layout changes, but should
be an acceptable solution to enable ctypes in subinterpreters in
Python 3.13.

Mark _ctypes as safe for multiple interpreters

(cherry picked from commit a192547)

Co-authored-by: Petr Viktorin <encukou@gmail.com>
Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>
@bedevere-app
Copy link

bedevere-app bot commented May 23, 2024

GH-119468 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label May 23, 2024
@encukou encukou deleted the ctypes-stginfo-hack branch May 23, 2024 16:02
encukou added a commit that referenced this pull request May 30, 2024
…19424) (GH-119468)

gh-117142: Slightly hacky fix for memory leak of StgInfo (GH-119424)

Add a funciton that inlines PyObject_GetTypeData and skips
type-checking, so it doesn't need access to the CType_Type object.
This will break if the memory layout changes, but should
be an acceptable solution to enable ctypes in subinterpreters in
Python 3.13.

Mark _ctypes as safe for multiple interpreters

(cherry picked from commit a192547)

Co-authored-by: Petr Viktorin <encukou@gmail.com>
Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
…nGH-119424)

Add a funciton that inlines PyObject_GetTypeData and skips
type-checking, so it doesn't need access to the CType_Type object.
This will break if the memory layout changes, but should
be an acceptable solution to enable ctypes in subinterpreters in
Python 3.13.

Mark _ctypes as safe for multiple interpreters

Co-authored-by: neonene <53406459+neonene@users.noreply.github.com>
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