[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)

diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h
index ccfc2586f0f23514855661314f94725fe5e4b5e3..218c3978e64dce241c2774b0a084ed8a724a62a1 100644 (file)
--- 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 04f4400a9315ba7d596c7113f168a3bf8fa05935..4514dfd3fc85eb6d6cf3c11b5a9e4445402738aa 100644 (file)
--- 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 aacf2b5c2e2c4ff6813cd07ddc02c8cb28e0fed0..b44cb0b9b6e09dc2b774369154f87a03cb4eaa0d 100644 (file)
--- 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 2ee16e3de25da35ab783e499d0c50c6cf1d49d9f..1fe88fdf5a8218403399790ac0701e72074a269b 100644 (file)
--- 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