Skip to content

Commit

Permalink
[3.12] gh-104690: thread_run() checks for tstate dangling pointer (#1…
Browse files Browse the repository at this point in the history
…09056) (#109133)

gh-104690: thread_run() checks for tstate dangling pointer (#109056)

thread_run() of _threadmodule.c now calls
_PyThreadState_CheckConsistency() to check if tstate is a dangling
pointer when Python is built in debug mode.

Rename ceval_gil.c is_tstate_valid() to
_PyThreadState_CheckConsistency() to reuse it in _threadmodule.c.

(cherry picked from commit f63d378)
  • Loading branch information
vstinner committed Oct 2, 2023
1 parent 9207c87 commit 30748d3
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 20 deletions.
4 changes: 4 additions & 0 deletions Include/internal/pycore_pystate.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ extern _Py_thread_local PyThreadState *_Py_tss_tstate;
#endif
PyAPI_DATA(PyThreadState *) _PyThreadState_GetCurrent(void);

#ifndef NDEBUG
extern int _PyThreadState_CheckConsistency(PyThreadState *tstate);
#endif

/* Get the current Python thread state.
This function is unsafe: it does not check for error and it can return NULL.
Expand Down
7 changes: 5 additions & 2 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1072,9 +1072,12 @@ static void
thread_run(void *boot_raw)
{
struct bootstate *boot = (struct bootstate *) boot_raw;
PyThreadState *tstate;
PyThreadState *tstate = boot->tstate;

// gh-104690: If Python is being finalized and PyInterpreterState_Delete()
// was called, tstate becomes a dangling pointer.
assert(_PyThreadState_CheckConsistency(tstate));

tstate = boot->tstate;
_PyThreadState_Bind(tstate);
PyEval_AcquireThread(tstate);
tstate->interp->threads.count++;
Expand Down
26 changes: 8 additions & 18 deletions Python/ceval_gil.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,16 +162,6 @@ UNSIGNAL_ASYNC_EXC(PyInterpreterState *interp)
COMPUTE_EVAL_BREAKER(interp, ceval, ceval2);
}

#ifndef NDEBUG
/* Ensure that tstate is valid */
static int
is_tstate_valid(PyThreadState *tstate)
{
assert(!_PyMem_IsPtrFreed(tstate));
assert(!_PyMem_IsPtrFreed(tstate->interp));
return 1;
}
#endif

/*
* Implementation of the Global Interpreter Lock (GIL).
Expand Down Expand Up @@ -324,7 +314,7 @@ drop_gil(struct _ceval_state *ceval, PyThreadState *tstate)
/* Not switched yet => wait */
if (((PyThreadState*)_Py_atomic_load_relaxed(&gil->last_holder)) == tstate)
{
assert(is_tstate_valid(tstate));
assert(_PyThreadState_CheckConsistency(tstate));
RESET_GIL_DROP_REQUEST(tstate->interp);
/* NOTE: if COND_WAIT does not atomically start waiting when
releasing the mutex, another thread can run through, take
Expand Down Expand Up @@ -385,7 +375,7 @@ take_gil(PyThreadState *tstate)
PyThread_exit_thread();
}

assert(is_tstate_valid(tstate));
assert(_PyThreadState_CheckConsistency(tstate));
PyInterpreterState *interp = tstate->interp;
struct _ceval_state *ceval = &interp->ceval;
struct _gil_runtime_state *gil = ceval->gil;
Expand Down Expand Up @@ -426,7 +416,7 @@ take_gil(PyThreadState *tstate)
}
PyThread_exit_thread();
}
assert(is_tstate_valid(tstate));
assert(_PyThreadState_CheckConsistency(tstate));

SET_GIL_DROP_REQUEST(interp);
drop_requested = 1;
Expand Down Expand Up @@ -465,7 +455,7 @@ take_gil(PyThreadState *tstate)
drop_gil(ceval, tstate);
PyThread_exit_thread();
}
assert(is_tstate_valid(tstate));
assert(_PyThreadState_CheckConsistency(tstate));

if (_Py_atomic_load_relaxed(&ceval->gil_drop_request)) {
RESET_GIL_DROP_REQUEST(interp);
Expand Down Expand Up @@ -673,7 +663,7 @@ PyEval_AcquireThread(PyThreadState *tstate)
void
PyEval_ReleaseThread(PyThreadState *tstate)
{
assert(is_tstate_valid(tstate));
assert(_PyThreadState_CheckConsistency(tstate));

PyThreadState *new_tstate = _PyThreadState_SwapNoGIL(NULL);
if (new_tstate != tstate) {
Expand Down Expand Up @@ -871,7 +861,7 @@ Py_AddPendingCall(int (*func)(void *), void *arg)
static int
handle_signals(PyThreadState *tstate)
{
assert(is_tstate_valid(tstate));
assert(_PyThreadState_CheckConsistency(tstate));
if (!_Py_ThreadCanHandleSignals(tstate->interp)) {
return 0;
}
Expand Down Expand Up @@ -977,7 +967,7 @@ void
_Py_FinishPendingCalls(PyThreadState *tstate)
{
assert(PyGILState_Check());
assert(is_tstate_valid(tstate));
assert(_PyThreadState_CheckConsistency(tstate));

if (make_pending_calls(tstate->interp) < 0) {
PyObject *exc = _PyErr_GetRaisedException(tstate);
Expand Down Expand Up @@ -1018,7 +1008,7 @@ Py_MakePendingCalls(void)
assert(PyGILState_Check());

PyThreadState *tstate = _PyThreadState_GET();
assert(is_tstate_valid(tstate));
assert(_PyThreadState_CheckConsistency(tstate));

/* Only execute pending calls on the main thread. */
if (!_Py_IsMainThread() || !_Py_IsMainInterpreter(tstate->interp)) {
Expand Down
18 changes: 18 additions & 0 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -2848,6 +2848,24 @@ _PyThreadState_PopFrame(PyThreadState *tstate, _PyInterpreterFrame * frame)
}


#ifndef NDEBUG
// Check that a Python thread state valid. In practice, this function is used
// on a Python debug build to check if 'tstate' is a dangling pointer, if the
// PyThreadState memory has been freed.
//
// Usage:
//
// assert(_PyThreadState_CheckConsistency(tstate));
int
_PyThreadState_CheckConsistency(PyThreadState *tstate)
{
assert(!_PyMem_IsPtrFreed(tstate));
assert(!_PyMem_IsPtrFreed(tstate->interp));
return 1;
}
#endif


#ifdef __cplusplus
}
#endif

0 comments on commit 30748d3

Please sign in to comment.