bpo-39984: _PyThreadState_DeleteCurrent() takes tstate (GH-19051)

* _PyThreadState_DeleteCurrent() now takes tstate rather than
  runtime.
* Add ensure_tstate_not_null() helper to pystate.c.
* Add _PyEval_ReleaseLock() function.
* _PyThreadState_DeleteCurrent() now calls
  _PyEval_ReleaseLock(tstate) and frees PyThreadState memory after
  this call, not before.
* PyGILState_Release(): rename "tcur" variable to "tstate".

Author: vstinner

Include/internal/pycore_ceval.h

@@ -57,6 +57,8 @@ extern PyObject *_PyEval_EvalCode(
 extern int _PyEval_ThreadsInitialized(_PyRuntimeState *runtime);
 extern PyStatus _PyEval_InitThreads(PyThreadState *tstate);
 
+extern void _PyEval_ReleaseLock(PyThreadState *tstate);
+
 
 /* --- _Py_EnterRecursiveCall() ----------------------------------------- */
 

Include/internal/pycore_pylifecycle.h

@@ -104,7 +104,7 @@ PyAPI_FUNC(void) _PyErr_Print(PyThreadState *tstate);
 PyAPI_FUNC(void) _PyErr_Display(PyObject *file, PyObject *exception,
                                 PyObject *value, PyObject *tb);
 
-PyAPI_FUNC(void) _PyThreadState_DeleteCurrent(struct pyruntimestate *runtime);
+PyAPI_FUNC(void) _PyThreadState_DeleteCurrent(PyThreadState *tstate);
 
 #ifdef __cplusplus
 }

Lib/test/test_capi.py

@@ -62,7 +62,8 @@ def test_no_FatalError_infinite_loop(self):
         # This used to cause an infinite loop.
         self.assertTrue(err.rstrip().startswith(
                          b'Fatal Python error: '
-                         b'PyThreadState_Get: no current thread'))
+                         b'PyThreadState_Get: '
+                         b'current thread state is NULL (released GIL?)'))
 
     def test_memoryview_from_NULL_pointer(self):
         self.assertRaises(ValueError, _testcapi.make_memoryview_from_NULL_pointer)

Modules/_threadmodule.c

@@ -1028,7 +1028,7 @@ t_bootstrap(void *boot_raw)
     PyMem_DEL(boot_raw);
     tstate->interp->num_threads--;
     PyThreadState_Clear(tstate);
-    _PyThreadState_DeleteCurrent(runtime);
+    _PyThreadState_DeleteCurrent(tstate);
     PyThread_exit_thread();
 }
 

Python/ceval.c

@@ -193,7 +193,8 @@ static void
 ensure_tstate_not_null(const char *func, PyThreadState *tstate)
 {
     if (tstate == NULL) {
-        _Py_FatalErrorFunc(func, "current thread state is NULL");
+        _Py_FatalErrorFunc(func,
+                           "current thread state is NULL (released GIL?)");
     }
 }
 
@@ -313,6 +314,13 @@ PyEval_ReleaseLock(void)
     drop_gil(&runtime->ceval, tstate);
 }
 
+void
+_PyEval_ReleaseLock(PyThreadState *tstate)
+{
+    struct _ceval_runtime_state *ceval = &tstate->interp->runtime->ceval;
+    drop_gil(ceval, tstate);
+}
+
 void
 PyEval_AcquireThread(PyThreadState *tstate)
 {

Python/pystate.c

@@ -37,6 +37,16 @@ extern "C" {
     _Py_atomic_store_relaxed(&(gilstate)->tstate_current, \
                              (uintptr_t)(value))
 
+static void
+ensure_tstate_not_null(const char *func, PyThreadState *tstate)
+{
+    if (tstate == NULL) {
+        _Py_FatalErrorFunc(func,
+                           "current thread state is NULL (released GIL?)");
+    }
+}
+
+
 /* Forward declarations */
 static PyThreadState *_PyGILState_GetThisThreadState(struct _gilstate_runtime_state *gilstate);
 static void _PyThreadState_Delete(PyThreadState *tstate, int check_current);
@@ -400,9 +410,7 @@ PyInterpreterState *
 PyInterpreterState_Get(void)
 {
     PyThreadState *tstate = _PyThreadState_GET();
-    if (tstate == NULL) {
-        Py_FatalError("no current thread state");
-    }
+    ensure_tstate_not_null(__func__, tstate);
     PyInterpreterState *interp = tstate->interp;
     if (interp == NULL) {
         Py_FatalError("no current interpreter");
@@ -819,9 +827,7 @@ tstate_delete_common(PyThreadState *tstate,
                      struct _gilstate_runtime_state *gilstate)
 {
     _PyRuntimeState *runtime = tstate->interp->runtime;
-    if (tstate == NULL) {
-        Py_FatalError("NULL tstate");
-    }
+    ensure_tstate_not_null(__func__, tstate);
     PyInterpreterState *interp = tstate->interp;
     if (interp == NULL) {
         Py_FatalError("NULL interp");
@@ -835,8 +841,6 @@ tstate_delete_common(PyThreadState *tstate,
         tstate->next->prev = tstate->prev;
     HEAD_UNLOCK(runtime);
 
-    PyMem_RawFree(tstate);
-
     if (gilstate->autoInterpreterState &&
         PyThread_tss_get(&gilstate->autoTSSkey) == tstate)
     {
@@ -855,6 +859,7 @@ _PyThreadState_Delete(PyThreadState *tstate, int check_current)
         }
     }
     tstate_delete_common(tstate, gilstate);
+    PyMem_RawFree(tstate);
 }
 
 
@@ -866,22 +871,22 @@ PyThreadState_Delete(PyThreadState *tstate)
 
 
 void
-_PyThreadState_DeleteCurrent(_PyRuntimeState *runtime)
+_PyThreadState_DeleteCurrent(PyThreadState *tstate)
 {
-    struct _gilstate_runtime_state *gilstate = &runtime->gilstate;
-    PyThreadState *tstate = _PyRuntimeGILState_GetThreadState(gilstate);
-    if (tstate == NULL) {
-        Py_FatalError("no current tstate");
-    }
+    ensure_tstate_not_null(__func__, tstate);
+    struct _gilstate_runtime_state *gilstate = &tstate->interp->runtime->gilstate;
     tstate_delete_common(tstate, gilstate);
     _PyRuntimeGILState_SetThreadState(gilstate, NULL);
-    PyEval_ReleaseLock();
+    _PyEval_ReleaseLock(tstate);
+    PyMem_RawFree(tstate);
 }
 
 void
 PyThreadState_DeleteCurrent(void)
 {
-    _PyThreadState_DeleteCurrent(&_PyRuntime);
+    struct _gilstate_runtime_state *gilstate = &_PyRuntime.gilstate;
+    PyThreadState *tstate = _PyRuntimeGILState_GetThreadState(gilstate);
+    _PyThreadState_DeleteCurrent(tstate);
 }
 
 
@@ -938,9 +943,7 @@ PyThreadState *
 PyThreadState_Get(void)
 {
     PyThreadState *tstate = _PyThreadState_GET();
-    if (tstate == NULL) {
-        Py_FatalError("no current thread");
-    }
+    ensure_tstate_not_null(__func__, tstate);
     return tstate;
 }
 
@@ -1342,8 +1345,8 @@ void
 PyGILState_Release(PyGILState_STATE oldstate)
 {
     _PyRuntimeState *runtime = &_PyRuntime;
-    PyThreadState *tcur = PyThread_tss_get(&runtime->gilstate.autoTSSkey);
-    if (tcur == NULL) {
+    PyThreadState *tstate = PyThread_tss_get(&runtime->gilstate.autoTSSkey);
+    if (tstate == NULL) {
         Py_FatalError("auto-releasing thread-state, "
                       "but no thread-state for this thread");
     }
@@ -1353,26 +1356,27 @@ PyGILState_Release(PyGILState_STATE oldstate)
        but while this is very new (April 2003), the extra check
        by release-only users can't hurt.
     */
-    if (!PyThreadState_IsCurrent(tcur)) {
+    if (!PyThreadState_IsCurrent(tstate)) {
         Py_FatalError("This thread state must be current when releasing");
     }
-    assert(PyThreadState_IsCurrent(tcur));
-    --tcur->gilstate_counter;
-    assert(tcur->gilstate_counter >= 0); /* illegal counter value */
+    assert(PyThreadState_IsCurrent(tstate));
+    --tstate->gilstate_counter;
+    assert(tstate->gilstate_counter >= 0); /* illegal counter value */
 
     /* If we're going to destroy this thread-state, we must
      * clear it while the GIL is held, as destructors may run.
      */
-    if (tcur->gilstate_counter == 0) {
+    if (tstate->gilstate_counter == 0) {
         /* can't have been locked when we created it */
         assert(oldstate == PyGILState_UNLOCKED);
-        PyThreadState_Clear(tcur);
+        PyThreadState_Clear(tstate);
         /* Delete the thread-state.  Note this releases the GIL too!
          * It's vital that the GIL be held here, to avoid shutdown
          * races; see bugs 225673 and 1061968 (that nasty bug has a
          * habit of coming back).
          */
-        _PyThreadState_DeleteCurrent(runtime);
+        assert(_PyRuntimeGILState_GetThreadState(&runtime->gilstate) == tstate);
+        _PyThreadState_DeleteCurrent(tstate);
     }
     /* Release the lock if necessary */
     else if (oldstate == PyGILState_UNLOCKED)