Skip to content

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

Merged
encukou merged 4 commits into
python:mainfrom
encukou:ctypes-stginfo-hack
May 23, 2024
Merged

gh-117142: Slightly hacky fix for memory leak of StgInfo#119424
encukou merged 4 commits into
python:mainfrom
encukou:ctypes-stginfo-hack

Conversation

@encukou

@encukouencukou commented May 22, 2024

Copy link
Copy Markdown
Member

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.
@encukouencukou changed the title gh-117142: Slightliy hacky fix for memory leak of StgInfogh-117142: Slightly hacky fix for memory leak of StgInfoMay 22, 2024

@ericsnowcurrentlyericsnowcurrently left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@neonene

Copy link
Copy Markdown
Contributor

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
Copy Markdown
MemberAuthor

It can! Thank you!

@neonene

Copy link
Copy Markdown
Contributor

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:mainMay 23, 2024
@miss-islington-app

Copy link
Copy Markdown

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
Copy Markdown

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

@bedevere-appbedevere-appBot 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@encukou@neonene@ericsnowcurrently