diff options
author | Victor Stinner <vstinner@python.org> | 2023-09-08 09:50:46 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-09-08 09:50:46 (GMT) |
commit | f63d37877ad166041489a968233b57540f8456e8 (patch) | |
tree | d4c6bb8c8478a3d0e3760b11e7e24a143012ba6e | |
parent | b0edf3b98e4b3e68a13776e034b9dd86ad7e529d (diff) | |
download | cpython-f63d37877ad166041489a968233b57540f8456e8.zip cpython-f63d37877ad166041489a968233b57540f8456e8.tar.gz cpython-f63d37877ad166041489a968233b57540f8456e8.tar.bz2 |
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.
-rw-r--r-- | Include/internal/pycore_pystate.h | 4 | ||||
-rw-r--r-- | Modules/_threadmodule.c | 7 | ||||
-rw-r--r-- | Python/ceval_gil.c | 26 | ||||
-rw-r--r-- | 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 |