From 30748d36b33fe08be98c8be4e629418584c796b0 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 2 Oct 2023 16:55:06 +0200 Subject: [3.12] gh-104690: thread_run() checks for tstate dangling pointer (#109056) (#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 f63d37877ad166041489a968233b57540f8456e8) --- Include/internal/pycore_pystate.h | 4 ++++ Modules/_threadmodule.c | 7 +++++-- Python/ceval_gil.c | 26 ++++++++------------------ Python/pystate.c | 18 ++++++++++++++++++ 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index ccfc258..218c397 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -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. diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 04f4400..4514dfd 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -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++; diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index aacf2b5..b44cb0b 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -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). @@ -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 @@ -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; @@ -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; @@ -465,7 +455,7 @@ _ready: 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); @@ -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) { @@ -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; } @@ -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); @@ -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)) { diff --git a/Python/pystate.c b/Python/pystate.c index 2ee16e3..1fe88fd 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -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 -- cgit v0.12