[3.12] gh-109793: Allow Switching Interpreters During Finalization (gh-109794)

Include/internal/pycore_interp.h

@@ -194,6 +194,10 @@ struct _is {
     struct _Py_interp_cached_objects cached_objects;
     struct _Py_interp_static_objects static_objects;
 
+    /* The ID of the OS thread in which we are finalizing.
+       We use _Py_atomic_address instead of adding a new _Py_atomic_ulong. */
+    _Py_atomic_address _finalizing_id;
+
    /* the initial PyInterpreterState.threads.head */
     PyThreadState _initial_thread;
 };
@@ -209,9 +213,23 @@ _PyInterpreterState_GetFinalizing(PyInterpreterState *interp) {
     return (PyThreadState*)_Py_atomic_load_relaxed(&interp->_finalizing);
 }
 
+static inline unsigned long
+_PyInterpreterState_GetFinalizingID(PyInterpreterState *interp) {
+    return (unsigned long)_Py_atomic_load_relaxed(&interp->_finalizing_id);
+}
+
 static inline void
 _PyInterpreterState_SetFinalizing(PyInterpreterState *interp, PyThreadState *tstate) {
     _Py_atomic_store_relaxed(&interp->_finalizing, (uintptr_t)tstate);
+    if (tstate == NULL) {
+        _Py_atomic_store_relaxed(&interp->_finalizing_id, 0);
+    }
+    else {
+        // XXX Re-enable this assert once gh-109860 is fixed.
+        //assert(tstate->thread_id == PyThread_get_thread_ident());
+        _Py_atomic_store_relaxed(&interp->_finalizing_id,
+                                 (uintptr_t)tstate->thread_id);
+    }
 }
 
 

Include/internal/pycore_pystate.h

@@ -36,8 +36,12 @@ _Py_IsMainInterpreter(PyInterpreterState *interp)
 static inline int
 _Py_IsMainInterpreterFinalizing(PyInterpreterState *interp)
 {
-    return (_PyRuntimeState_GetFinalizing(interp->runtime) != NULL &&
-            interp == &interp->runtime->_main_interpreter);
+    /* bpo-39877: Access _PyRuntime directly rather than using
+       tstate->interp->runtime to support calls from Python daemon threads.
+       After Py_Finalize() has been called, tstate can be a dangling pointer:
+       point to PyThreadState freed memory. */
+    return (_PyRuntimeState_GetFinalizing(&_PyRuntime) != NULL &&
+            interp == &_PyRuntime._main_interpreter);
 }
 
 

Include/internal/pycore_runtime.h

@@ -163,6 +163,9 @@ typedef struct pyruntimestate {
     struct _Py_static_objects static_objects;
     struct _Py_cached_objects cached_objects;
 
+    /* The ID of the OS thread in which we are finalizing.
+       We use _Py_atomic_address instead of adding a new _Py_atomic_ulong. */
+    _Py_atomic_address _finalizing_id;
     /* The value to use for sys.path[0] in new subinterpreters.
        Normally this would be part of the PyConfig struct.  However,
        we cannot add it there in 3.12 since that's an ABI change. */
@@ -210,9 +213,23 @@ _PyRuntimeState_GetFinalizing(_PyRuntimeState *runtime) {
     return (PyThreadState*)_Py_atomic_load_relaxed(&runtime->_finalizing);
 }
 
+static inline unsigned long
+_PyRuntimeState_GetFinalizingID(_PyRuntimeState *runtime) {
+    return (unsigned long)_Py_atomic_load_relaxed(&runtime->_finalizing_id);
+}
+
 static inline void
 _PyRuntimeState_SetFinalizing(_PyRuntimeState *runtime, PyThreadState *tstate) {
     _Py_atomic_store_relaxed(&runtime->_finalizing, (uintptr_t)tstate);
+    if (tstate == NULL) {
+        _Py_atomic_store_relaxed(&runtime->_finalizing_id, 0);
+    }
+    else {
+        // XXX Re-enable this assert once gh-109860 is fixed.
+        //assert(tstate->thread_id == PyThread_get_thread_ident());
+        _Py_atomic_store_relaxed(&runtime->_finalizing_id,
+                                 (uintptr_t)tstate->thread_id);
+    }
 }
 
 #ifdef __cplusplus

Lib/test/test_interpreters.py

@@ -639,6 +639,26 @@ def test_sys_path_0(self):
                 self.assertEqual(sp0_sub, expected)
 
 
+class FinalizationTests(TestBase):
+
+    def test_gh_109793(self):
+        import subprocess
+        argv = [sys.executable, '-c', '''if True:
+            import _xxsubinterpreters as _interpreters
+            interpid = _interpreters.create()
+            raise Exception
+            ''']
+        proc = subprocess.run(argv, capture_output=True, text=True)
+        self.assertIn('Traceback', proc.stderr)
+        if proc.returncode == 0 and support.verbose:
+            print()
+            print("--- cmd unexpected succeeded ---")
+            print(f"stdout:\n{proc.stdout}")
+            print(f"stderr:\n{proc.stderr}")
+            print("------")
+        self.assertEqual(proc.returncode, 1)
+
+
 class TestIsShareable(TestBase):
 
     def test_default_shareables(self):

Misc/NEWS.d/next/Core and Builtins/2023-09-25-09-24-10.gh-issue-109793.zFQBkv.rst

@@ -0,0 +1,4 @@
+The main thread no longer exits prematurely when a subinterpreter
+is cleaned up during runtime finalization.  The bug was a problem
+particularly because, when triggered, the Python process would
+always return with a 0 exitcode, even if it failed.

Python/pystate.c

@@ -2916,11 +2916,26 @@ _PyThreadState_MustExit(PyThreadState *tstate)
        tstate->interp->runtime to support calls from Python daemon threads.
        After Py_Finalize() has been called, tstate can be a dangling pointer:
        point to PyThreadState freed memory. */
+    unsigned long finalizing_id = _PyRuntimeState_GetFinalizingID(&_PyRuntime);
     PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime);
     if (finalizing == NULL) {
+        // XXX This isn't completely safe from daemon thraeds,
+        // since tstate might be a dangling pointer.
         finalizing = _PyInterpreterState_GetFinalizing(tstate->interp);
+        finalizing_id = _PyInterpreterState_GetFinalizingID(tstate->interp);
     }
-    return (finalizing != NULL && finalizing != tstate);
+    // XXX else check &_PyRuntime._main_interpreter._initial_thread
+    if (finalizing == NULL) {
+        return 0;
+    }
+    else if (finalizing == tstate) {
+        return 0;
+    }
+    else if (finalizing_id == PyThread_get_thread_ident()) {
+        /* gh-109793: we must have switched interpreters. */
+        return 0;
+    }
+    return 1;
 }