Skip to content

PyThreadState_Clear() should only be called from the same interpreter #112723

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.

Sign up for GitHub

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

Jump to bottom
Closed
colesbury opened this issue Dec 4, 2023 · 0 comments
Closed

PyThreadState_Clear() should only be called from the same interpreter #112723

colesbury opened this issue Dec 4, 2023 · 0 comments
Labels
3.13 bugs and security fixes topic-free-threading topic-subinterpreters

Comments

@colesbury
Copy link
Contributor

colesbury commented Dec 4, 2023 •
edited by bedevere-app bot
Loading

PyThreadState_Clear() includes the following comment:

cpython/Python/pystate.c

Lines 1553 to 1558 in 1e4680c

/* XXX Conditions we need to enforce:
* the GIL must be held by the current thread
* current_fast_get()->interp must match tstate->interp
* for the main interpreter, current_fast_get() must be the main thread
*/

We should enforce this, particularly the comment about the matching interpreters. Calling PyThreadState_Clear() from the "wrong" interpreter is unsafe because if any of the PyObjects on the tstate are not NULL, calling their destructors from the wrong thread can lead to memory corruption.

This is also important for the "free threaded" builds because they have free lists associated with the PyThreadState and these will be cleared in PyThreadState_Clear() -- doing this in the wrong interpreter leads to memory corruption.

There are currently two places which call PyThreadState_Clear() from the "wrong" interpreter:

  1. interp_create() in _xxsubinterpretersmodule.c. This is pretty easy to fix by setting the thread state before calling clear. `

    cpython/Modules/_xxsubinterpretersmodule.c

    Line 266 in 1e4680c

    PyThreadState_Clear(tstate);
  2. new_interpreter() in pylifecycle.c in the error code path. This is trickier because the thread state is not fully initialized.

    cpython/Python/pylifecycle.c

    Line 2164 in 1e4680c

    PyThreadState_Clear(tstate);

Related: #101436 (comment)

cc @ericsnowcurrently

Linked PRs
@colesbury colesbury added the 3.13 bugs and security fixes label Dec 4, 2023
colesbury added a commit to colesbury/cpython that referenced this issue Dec 5, 2023
@colesbury
pythongh-112723: Call PyThreadState_Clear() from the correct interp…
ded968d 
…reter

The `PyThreadState_Clear()` function must only be called with the GIL
held and must be called from the same interpreter as the passed in
thread state. Otherwise, any Python objects on the thread state may be
destroyed using the wrong interpreter, leading to memory corruption.

This is also important for `Py_GIL_DISABLED` builds because free lists
will be associated with PyThreadStates and cleared in
`PyThreadState_Clear()`.

This fixes two places that called `PyThreadState_Clear()` from the wrong
interpreter and adds an assertion to `PyThreadState_Clear()`.
@bedevere-app bedevere-app bot mentioned this issue Dec 5, 2023
Merged
carljm pushed a commit that referenced this issue Dec 13, 2023
@colesbury
gh-112723: Call PyThreadState_Clear() from the correct interpreter (#…
a3c0318 
…112776)

The `PyThreadState_Clear()` function must only be called with the GIL
held and must be called from the same interpreter as the passed in
thread state. Otherwise, any Python objects on the thread state may be
destroyed using the wrong interpreter, leading to memory corruption.

This is also important for `Py_GIL_DISABLED` builds because free lists
will be associated with PyThreadStates and cleared in
`PyThreadState_Clear()`.

This fixes two places that called `PyThreadState_Clear()` from the wrong
interpreter and adds an assertion to `PyThreadState_Clear()`.
@github-project-automation github-project-automation bot moved this from Todo to Done in Subinterpreters Jan 5, 2024
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@colesbury @aisk
pythongh-112723: Call PyThreadState_Clear() from the correct interp…
07a5e7f 
…reter (python#112776)

The `PyThreadState_Clear()` function must only be called with the GIL
held and must be called from the same interpreter as the passed in
thread state. Otherwise, any Python objects on the thread state may be
destroyed using the wrong interpreter, leading to memory corruption.

This is also important for `Py_GIL_DISABLED` builds because free lists
will be associated with PyThreadStates and cleared in
`PyThreadState_Clear()`.

This fixes two places that called `PyThreadState_Clear()` from the wrong
interpreter and adds an assertion to `PyThreadState_Clear()`.
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
@colesbury @Glyphack
pythongh-112723: Call PyThreadState_Clear() from the correct interp…
9efc048 
…reter (python#112776)

The `PyThreadState_Clear()` function must only be called with the GIL
held and must be called from the same interpreter as the passed in
thread state. Otherwise, any Python objects on the thread state may be
destroyed using the wrong interpreter, leading to memory corruption.

This is also important for `Py_GIL_DISABLED` builds because free lists
will be associated with PyThreadStates and cleared in
`PyThreadState_Clear()`.

This fixes two places that called `PyThreadState_Clear()` from the wrong
interpreter and adds an assertion to `PyThreadState_Clear()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.13 bugs and security fixes topic-free-threading topic-subinterpreters
Projects
Subinterpreters
Status: Done
Milestone
No milestone
Development

No branches or pull requests
1 participant
@colesbury