gh-104341: Ensure tstate Holding GIL is Always Current

Include/internal/pycore_ceval.h

@@ -101,6 +101,7 @@ extern void _PyEval_FiniGIL(PyInterpreterState *interp);
 
 extern void _PyEval_AcquireLock(PyThreadState *tstate);
 extern void _PyEval_ReleaseLock(PyThreadState *tstate);
+extern int _PyEval_HoldsLock(PyThreadState *tstate);
 extern PyThreadState * _PyThreadState_SwapNoGIL(PyThreadState *);
 
 extern void _PyEval_DeactivateOpCache(void);

Modules/_threadmodule.c

@@ -3,6 +3,7 @@
 /* Interface to Sjoerd's portable C thread library */
 
 #include "Python.h"
+#include "pycore_ceval.h"         // _PyEval_HoldsLock()
 #include "pycore_interp.h"        // _PyInterpreterState.threads.count
 #include "pycore_moduleobject.h"  // _PyModule_GetState()
 #include "pycore_pylifecycle.h"

Python/ceval_gil.c

@@ -282,7 +282,10 @@ drop_gil(struct _ceval_state *ceval, PyThreadState *tstate)
     if (!_Py_atomic_load_relaxed(&gil->locked)) {
         Py_FatalError("drop_gil: GIL is not locked");
     }
+    assert(_Py_tss_tstate == tstate);
+    assert(((PyThreadState *)_Py_atomic_load_relaxed(&gil->last_holder)) == tstate);
 
+    // XXX Does this still hold?
     /* tstate is allowed to be NULL (early interpreter init) */
     if (tstate != NULL) {
         /* Sub-interpreter support: threads might have been switched
@@ -348,6 +351,7 @@ take_gil(PyThreadState *tstate)
 
     assert(tstate != NULL);
 
+    assert(_Py_tss_tstate == tstate);
     if (tstate_must_exit(tstate)) {
         /* bpo-39877: If Py_Finalize() has been called and tstate is not the
            thread which called Py_Finalize(), exit immediately the thread.
@@ -629,29 +633,37 @@ _PyEval_ReleaseLock(PyThreadState *tstate)
     drop_gil(ceval, tstate);
 }
 
+int
+_PyEval_HoldsLock(PyThreadState *tstate)
+{
+    _Py_EnsureTstateNotNULL(tstate);
+    return current_thread_holds_gil(tstate->interp->ceval.gil, tstate);
+}
+
 void
 PyEval_AcquireThread(PyThreadState *tstate)
 {
     _Py_EnsureTstateNotNULL(tstate);
 
-    take_gil(tstate);
-
     if (_PyThreadState_SwapNoGIL(tstate) != NULL) {
         Py_FatalError("non-NULL old thread state");
     }
+
+    take_gil(tstate);
 }
 
 void
 PyEval_ReleaseThread(PyThreadState *tstate)
 {
     assert(is_tstate_valid(tstate));
 
+    struct _ceval_state *ceval = &tstate->interp->ceval;
+    drop_gil(ceval, tstate);
+
     PyThreadState *new_tstate = _PyThreadState_SwapNoGIL(NULL);
     if (new_tstate != tstate) {
         Py_FatalError("wrong thread state");
     }
-    struct _ceval_state *ceval = &tstate->interp->ceval;
-    drop_gil(ceval, tstate);
 }
 
 #ifdef HAVE_FORK
@@ -694,12 +706,15 @@ _PyEval_SignalAsyncExc(PyInterpreterState *interp)
 PyThreadState *
 PyEval_SaveThread(void)
 {
+    PyThreadState *current_tstate = _PyThreadState_GET();
+    _Py_EnsureTstateNotNULL(current_tstate);
+    struct _ceval_state *ceval = &current_tstate->interp->ceval;
+    assert(gil_created(ceval->gil));
+    drop_gil(ceval, current_tstate);
+
     PyThreadState *tstate = _PyThreadState_SwapNoGIL(NULL);
-    _Py_EnsureTstateNotNULL(tstate);
+    assert(tstate == current_tstate);
 
-    struct _ceval_state *ceval = &tstate->interp->ceval;
-    assert(gil_created(ceval->gil));
-    drop_gil(ceval, tstate);
     return tstate;
 }
 
@@ -708,9 +723,9 @@ PyEval_RestoreThread(PyThreadState *tstate)
 {
     _Py_EnsureTstateNotNULL(tstate);
 
-    take_gil(tstate);
-
     _PyThreadState_SwapNoGIL(tstate);
+
+    take_gil(tstate);
 }
 
 
@@ -1006,18 +1021,18 @@ _Py_HandlePending(PyThreadState *tstate)
     /* GIL drop request */
     if (_Py_atomic_load_relaxed_int32(&interp_ceval_state->gil_drop_request)) {
         /* Give another thread a chance */
+        assert(tstate == _PyThreadState_GET());
+        drop_gil(interp_ceval_state, tstate);
         if (_PyThreadState_SwapNoGIL(NULL) != tstate) {
             Py_FatalError("tstate mix-up");
         }
-        drop_gil(interp_ceval_state, tstate);
 
         /* Other threads may run now */
 
-        take_gil(tstate);
-
         if (_PyThreadState_SwapNoGIL(tstate) != NULL) {
             Py_FatalError("orphan tstate");
         }
+        take_gil(tstate);
     }
 
     /* Check for asynchronous exception. */

Python/pylifecycle.c

@@ -2027,8 +2027,16 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
     }
     _PyThreadState_Bind(tstate);
 
+    // We must release the GIL before swapping.
+    PyThreadState *current_tstate = _PyThreadState_GET();
+    if (current_tstate != NULL) {
+        // XXX Might new_interpreter() have been called without the GIL held?
+        _PyEval_ReleaseLock(current_tstate);
+    }
+
     // XXX For now we do this before the GIL is created.
     PyThreadState *save_tstate = _PyThreadState_SwapNoGIL(tstate);
+    assert(save_tstate == current_tstate);
     int has_gil = 0;
 
     /* From this point until the init_interp_create_gil() call,
@@ -2039,8 +2047,6 @@ new_interpreter(PyThreadState **tstate_p, const PyInterpreterConfig *config)
     /* Copy the current interpreter config into the new interpreter */
     const PyConfig *src_config;
     if (save_tstate != NULL) {
-        // XXX Might new_interpreter() have been called without the GIL held?
-        _PyEval_ReleaseLock(save_tstate);
         src_config = _PyInterpreterState_GetConfig(save_tstate->interp);
     }
     else

Python/pystate.c

@@ -1564,8 +1564,10 @@ _PyThreadState_DeleteCurrent(PyThreadState *tstate)
 {
     _Py_EnsureTstateNotNULL(tstate);
     tstate_delete_common(tstate);
-    current_fast_clear(tstate->interp->runtime);
+    assert(_Py_tss_tstate == tstate);
+    assert(_PyEval_HoldsLock(tstate));
     _PyEval_ReleaseLock(tstate);
+    current_fast_clear(tstate->interp->runtime);
     free_threadstate(tstate);
 }