From f63d37877ad166041489a968233b57540f8456e8 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 8 Sep 2023 11:50:46 +0200 Subject: 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. --- 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 a30036a..9c0e42e 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -67,6 +67,10 @@ _Py_ThreadCanHandleSignals(PyInterpreterState *interp) extern _Py_thread_local PyThreadState *_Py_tss_tstate; #endif +#ifndef NDEBUG +extern int _PyThreadState_CheckConsistency(PyThreadState *tstate); +#endif + // Export for most shared extensions, used via _PyThreadState_GET() static // inline function. PyAPI_FUNC(PyThreadState *) _PyThreadState_GetCurrent(void); diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 49f34fc..05bb497 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -1074,9 +1074,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 e53ffa7..cef5317 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -163,16 +163,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). @@ -325,7 +315,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 @@ -386,7 +376,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; @@ -427,7 +417,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; @@ -466,7 +456,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); @@ -679,7 +669,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) { @@ -877,7 +867,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; } @@ -983,7 +973,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); @@ -1024,7 +1014,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 89275fd..09c3538 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2890,6 +2890,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