From b55cf2c2d82d7567720df113117752841ce74606 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 8 Sep 2023 13:10:33 +0200 Subject: [3.11] gh-104690: thread_run() checks for tstate dangling pointer (#109056) (#109134) 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 f63d37877ad166041489a968233b57540f8456e8) --- Include/internal/pycore_pystate.h | 4 ++++ Modules/_threadmodule.c | 7 +++++-- Python/ceval.c | 26 ++++++-------------------- Python/ceval_gil.h | 8 ++++---- Python/pystate.c | 18 ++++++++++++++++++ 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index cb8f115..8696aaf 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -61,6 +61,10 @@ _Py_ThreadCanHandlePendingCalls(void) } +#ifndef NDEBUG +extern int _PyThreadState_CheckConsistency(PyThreadState *tstate); +#endif + /* Variable and macro for in-line access to current thread and interpreter state */ diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 199e3b8..b70325a 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1076,9 +1076,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; tstate->thread_id = PyThread_get_thread_ident(); #ifdef PY_HAVE_THREAD_NATIVE_ID tstate->native_thread_id = PyThread_get_thread_native_id(); diff --git a/Python/ceval.c b/Python/ceval.c index 47df353..df11de0 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -216,20 +216,6 @@ _PyEvalFrameClearAndPop(PyThreadState *tstate, _PyInterpreterFrame *frame); "cannot access free variable '%s' where it is not associated with a" \ " value in enclosing scope" -#ifndef NDEBUG -/* Ensure that tstate is valid: sanity check for PyEval_AcquireThread() and - PyEval_RestoreThread(). Detect if tstate memory was freed. It can happen - when a thread continues to run after Python finalization, especially - daemon threads. */ -static int -is_tstate_valid(PyThreadState *tstate) -{ - assert(!_PyMem_IsPtrFreed(tstate)); - assert(!_PyMem_IsPtrFreed(tstate->interp)); - return 1; -} -#endif - /* This can set eval_breaker to 0 even though gil_drop_request became 1. We believe this is all right because the eval loop will release @@ -464,7 +450,7 @@ PyEval_AcquireThread(PyThreadState *tstate) void PyEval_ReleaseThread(PyThreadState *tstate) { - assert(is_tstate_valid(tstate)); + assert(_PyThreadState_CheckConsistency(tstate)); _PyRuntimeState *runtime = tstate->interp->runtime; PyThreadState *new_tstate = _PyThreadState_Swap(&runtime->gilstate, NULL); @@ -671,7 +657,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; } @@ -739,7 +725,7 @@ void _Py_FinishPendingCalls(PyThreadState *tstate) { assert(PyGILState_Check()); - assert(is_tstate_valid(tstate)); + assert(_PyThreadState_CheckConsistency(tstate)); struct _pending_calls *pending = &tstate->interp->ceval.pending; @@ -764,7 +750,7 @@ Py_MakePendingCalls(void) assert(PyGILState_Check()); PyThreadState *tstate = _PyThreadState_GET(); - assert(is_tstate_valid(tstate)); + assert(_PyThreadState_CheckConsistency(tstate)); /* Python signal handler doesn't really queue a callback: it only signals that a signal was received, see _PyEval_SignalReceived(). */ @@ -6947,7 +6933,7 @@ maybe_call_line_trace(Py_tracefunc func, PyObject *obj, int _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) { - assert(is_tstate_valid(tstate)); + assert(_PyThreadState_CheckConsistency(tstate)); /* The caller must hold the GIL */ assert(PyGILState_Check()); @@ -6999,7 +6985,7 @@ PyEval_SetProfile(Py_tracefunc func, PyObject *arg) int _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) { - assert(is_tstate_valid(tstate)); + assert(_PyThreadState_CheckConsistency(tstate)); /* The caller must hold the GIL */ assert(PyGILState_Check()); diff --git a/Python/ceval_gil.h b/Python/ceval_gil.h index 476ed7f..d20af26 100644 --- a/Python/ceval_gil.h +++ b/Python/ceval_gil.h @@ -171,7 +171,7 @@ drop_gil(struct _ceval_runtime_state *ceval, struct _ceval_state *ceval2, /* 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 @@ -226,7 +226,7 @@ take_gil(PyThreadState *tstate) PyThread_exit_thread(); } - assert(is_tstate_valid(tstate)); + assert(_PyThreadState_CheckConsistency(tstate)); PyInterpreterState *interp = tstate->interp; struct _ceval_runtime_state *ceval = &interp->runtime->ceval; struct _ceval_state *ceval2 = &interp->ceval; @@ -268,7 +268,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; @@ -307,7 +307,7 @@ _ready: drop_gil(ceval, ceval2, tstate); PyThread_exit_thread(); } - assert(is_tstate_valid(tstate)); + assert(_PyThreadState_CheckConsistency(tstate)); if (_Py_atomic_load_relaxed(&ceval2->gil_drop_request)) { RESET_GIL_DROP_REQUEST(interp); diff --git a/Python/pystate.c b/Python/pystate.c index dfca3f5..ec278ee 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2237,6 +2237,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 -- cgit v0.12