gh-104341: Adjust tstate_must_exit() to Respect Interpreter Finalizat…

…ion (gh-104437) With the move to a per-interpreter GIL, this check slipped through the cracks.
python · May 15, 2023 · 26baa74 · 26baa74
1 parent cb88ae6
commit 26baa74
Show file tree

Hide file tree

Showing 11 changed files with 56 additions and 10 deletions.
diff --git a/Include/cpython/pylifecycle.h b/Include/cpython/pylifecycle.h
@@ -52,6 +52,7 @@ PyAPI_FUNC(const char *) _Py_gitidentifier(void);
 PyAPI_FUNC(const char *) _Py_gitversion(void);
 
 PyAPI_FUNC(int) _Py_IsFinalizing(void);
+PyAPI_FUNC(int) _Py_IsInterpreterFinalizing(PyInterpreterState *interp);
 
 /* Random */
 PyAPI_FUNC(int) _PyOS_URandom(void *buffer, Py_ssize_t size);

diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h
@@ -83,6 +83,13 @@ struct _is {
     int _initialized;
     int finalizing;
 
+    /* Set by Py_EndInterpreter().
+
+       Use _PyInterpreterState_GetFinalizing()
+       and _PyInterpreterState_SetFinalizing()
+       to access it, don't access it directly. */
+    _Py_atomic_address _finalizing;
+
     struct _obmalloc_state obmalloc;
 
     struct _ceval_state ceval;
@@ -191,6 +198,17 @@ struct _is {
 extern void _PyInterpreterState_Clear(PyThreadState *tstate);
 
 
+static inline PyThreadState*
+_PyInterpreterState_GetFinalizing(PyInterpreterState *interp) {
+    return (PyThreadState*)_Py_atomic_load_relaxed(&interp->_finalizing);
+}
+
+static inline void
+_PyInterpreterState_SetFinalizing(PyInterpreterState *interp, PyThreadState *tstate) {
+    _Py_atomic_store_relaxed(&interp->_finalizing, (uintptr_t)tstate);
+}
+
+
 /* cross-interpreter data registry */
 
 /* For now we use a global registry of shareable classes.  An

diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c
@@ -526,7 +526,7 @@ future_init(FutureObj *fut, PyObject *loop)
     if (is_true < 0) {
         return -1;
     }
-    if (is_true && !_Py_IsFinalizing()) {
+    if (is_true && !_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) {
         /* Only try to capture the traceback if the interpreter is not being
            finalized.  The original motivation to add a `_Py_IsFinalizing()`
            call was to prevent SIGSEGV when a Future is created in a __del__

diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c
@@ -293,7 +293,8 @@ _enter_buffered_busy(buffered *self)
                      "reentrant call inside %R", self);
         return 0;
     }
-    relax_locking = _Py_IsFinalizing();
+    PyInterpreterState *interp = PyInterpreterState_Get();
+    relax_locking = _Py_IsInterpreterFinalizing(interp);
     Py_BEGIN_ALLOW_THREADS
     if (!relax_locking)
         st = PyThread_acquire_lock(self->lock, 1);

diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c
@@ -419,7 +419,7 @@ connection_close(pysqlite_Connection *self)
         {
             /* If close is implicitly called as a result of interpreter
              * tear-down, we must not call back into Python. */
-            if (_Py_IsFinalizing()) {
+            if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get())) {
                 remove_callbacks(self->db);
             }
             (void)connection_exec_stmt(self, "ROLLBACK");

diff --git a/Modules/_winapi.c b/Modules/_winapi.c
@@ -133,7 +133,7 @@ overlapped_dealloc(OverlappedObject *self)
         {
             /* The operation is no longer pending -- nothing to do. */
         }
-        else if (_Py_IsFinalizing())
+        else if (_Py_IsInterpreterFinalizing(PyInterpreterState_Get()))
         {
             /* The operation is still pending -- give a warning.  This
                will probably only happen on Windows XP. */

diff --git a/Python/_warnings.c b/Python/_warnings.c
@@ -198,7 +198,7 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import)
     PyObject *warnings_module, *obj;
 
     /* don't try to import after the start of the Python finallization */
-    if (try_import && !_Py_IsFinalizing()) {
+    if (try_import && !_Py_IsInterpreterFinalizing(interp)) {
         warnings_module = PyImport_Import(&_Py_ID(warnings));
         if (warnings_module == NULL) {
             /* Fallback to the C implementation if we cannot get

diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c
@@ -332,6 +332,9 @@ tstate_must_exit(PyThreadState *tstate)
        After Py_Finalize() has been called, tstate can be a dangling pointer:
        point to PyThreadState freed memory. */
     PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime);
+    if (finalizing == NULL) {
+        finalizing = _PyInterpreterState_GetFinalizing(tstate->interp);
+    }
     return (finalizing != NULL && finalizing != tstate);
 }
 

diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
@@ -1788,6 +1788,7 @@ Py_FinalizeEx(void)
 
     /* Remaining daemon threads will automatically exit
        when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
+    _PyInterpreterState_SetFinalizing(tstate->interp, tstate);
     _PyRuntimeState_SetFinalizing(runtime, tstate);
     runtime->initialized = 0;
     runtime->core_initialized = 0;
@@ -2142,6 +2143,10 @@ Py_EndInterpreter(PyThreadState *tstate)
         Py_FatalError("not the last thread");
     }
 
+    /* Remaining daemon threads will automatically exit
+       when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
+    _PyInterpreterState_SetFinalizing(interp, tstate);
+
     // XXX Call something like _PyImport_Disable() here?
 
     _PyImport_FiniExternal(tstate->interp);
@@ -2152,6 +2157,18 @@ Py_EndInterpreter(PyThreadState *tstate)
     finalize_interp_delete(tstate->interp);
 }
 
+int
+_Py_IsInterpreterFinalizing(PyInterpreterState *interp)
+{
+    /* We check the runtime first since, in a daemon thread,
+       interp might be dangling pointer. */
+    PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime);
+    if (finalizing == NULL) {
+        finalizing = _PyInterpreterState_GetFinalizing(interp);
+    }
+    return finalizing != NULL;
+}
+
 /* Add the __main__ module */
 
 static PyStatus

diff --git a/Python/pystate.c b/Python/pystate.c
@@ -1436,11 +1436,13 @@ PyThreadState_Clear(PyThreadState *tstate)
 
     if (verbose && tstate->cframe->current_frame != NULL) {
         /* bpo-20526: After the main thread calls
-           _PyRuntimeState_SetFinalizing() in Py_FinalizeEx(), threads must
-           exit when trying to take the GIL. If a thread exit in the middle of
-           _PyEval_EvalFrameDefault(), tstate->frame is not reset to its
-           previous value. It is more likely with daemon threads, but it can
-           happen with regular threads if threading._shutdown() fails
+           _PyInterpreterState_SetFinalizing() in Py_FinalizeEx()
+           (or in Py_EndInterpreter() for subinterpreters),
+           threads must exit when trying to take the GIL.
+           If a thread exit in the middle of _PyEval_EvalFrameDefault(),
+           tstate->frame is not reset to its previous value.
+           It is more likely with daemon threads, but it can happen
+           with regular threads if threading._shutdown() fails
            (ex: interrupted by CTRL+C). */
         fprintf(stderr,
           "PyThreadState_Clear: warning: thread still has a frame\n");

diff --git a/Python/sysmodule.c b/Python/sysmodule.c
@@ -332,6 +332,7 @@ _PySys_ClearAuditHooks(PyThreadState *ts)
     }
 
     _PyRuntimeState *runtime = ts->interp->runtime;
+    /* The hooks are global so we have to check for runtime finalization. */
     PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(runtime);
     assert(finalizing == ts);
     if (finalizing != ts) {
@@ -2039,6 +2040,9 @@ sys__clear_type_cache_impl(PyObject *module)
     Py_RETURN_NONE;
 }
 
+/* Note that, for now, we do not have a per-interpreter equivalent
+  for sys.is_finalizing(). */
+
 /*[clinic input]
 sys.is_finalizing