Revert "gh-128639: Don't assume one thread in subinterpreter finalization (gh-128640)"

This reverts commit 9859791.

Author: ZeroIntensity

Lib/test/test_interpreters/test_api.py

@@ -647,59 +647,6 @@ def test_created_with_capi(self):
                     self.interp_exists(interpid))
 
 
-    def test_remaining_threads(self):
-        r_interp, w_interp = self.pipe()
-
-        FINISHED = b'F'
-
-        # It's unlikely, but technically speaking, it's possible
-        # that the thread could've finished before interp.close() is
-        # reached, so this test might not properly exercise the case.
-        # However, it's quite unlikely and I'm too lazy to deal with it.
-        interp = interpreters.create()
-        interp.exec(f"""if True:
-            import os
-            import threading
-            import time
-
-            def task():
-                time.sleep(1)
-                os.write({w_interp}, {FINISHED!r})
-
-            threads = [threading.Thread(target=task) for _ in range(3)]
-            for t in threads:
-                t.start()
-            """)
-        interp.close()
-
-        self.assertEqual(os.read(r_interp, 1), FINISHED)
-
-    def test_remaining_daemon_threads(self):
-        interp = _interpreters.create(
-            types.SimpleNamespace(
-                use_main_obmalloc=False,
-                allow_fork=False,
-                allow_exec=False,
-                allow_threads=True,
-                allow_daemon_threads=True,
-                check_multi_interp_extensions=True,
-                gil='own',
-            )
-        )
-        _interpreters.exec(interp, f"""if True:
-            import threading
-            import time
-
-            def task():
-                time.sleep(100)
-
-            threads = [threading.Thread(target=task, daemon=True) for _ in range(3)]
-            for t in threads:
-                t.start()
-            """)
-        _interpreters.destroy(interp)
-
-
 class TestInterpreterPrepareMain(TestBase):
 
     def test_empty(self):
@@ -808,10 +755,7 @@ def script():
                 spam.eggs()
 
             interp = interpreters.create()
-            try:
-                interp.exec(script)
-            finally:
-                interp.close()
+            interp.exec(script)
             """)
 
         stdout, stderr = self.assert_python_failure(scriptfile)
@@ -820,7 +764,7 @@ def script():
         #      File "{interpreters.__file__}", line 179, in exec
         self.assertEqual(stderr, dedent(f"""\
             Traceback (most recent call last):
-              File "{scriptfile}", line 10, in <module>
+              File "{scriptfile}", line 9, in <module>
                 interp.exec(script)
                 ~~~~~~~~~~~^^^^^^^^
               {interpmod_line.strip()}

Lib/test/test_interpreters/test_lifecycle.py

@@ -132,7 +132,6 @@ def test_sys_path_0(self):
                     'sub': sys.path[0],
                 }}, indent=4), flush=True)
                 """)
-            interp.close()
             '''
         # <tmp>/
         #   pkg/
@@ -173,10 +172,7 @@ def test_gh_109793(self):
         argv = [sys.executable, '-c', '''if True:
             from test.support import interpreters
             interp = interpreters.create()
-            try:
-                raise Exception
-            finally:
-                interp.close()
+            raise Exception
             ''']
         proc = subprocess.run(argv, capture_output=True, text=True)
         self.assertIn('Traceback', proc.stderr)

Lib/test/test_threading.py

@@ -1689,7 +1689,10 @@ def f():
 
             _testcapi.run_in_subinterp(%r)
             """ % (subinterp_code,)
-        assert_python_ok("-c", script)
+        with test.support.SuppressCrashReport():
+            rc, out, err = assert_python_failure("-c", script)
+        self.assertIn("Fatal Python error: Py_EndInterpreter: "
+                      "not the last thread", err.decode())
 
     def _check_allowed(self, before_start='', *,
                        allowed=True,

Misc/NEWS.d/next/Core_and_Builtins/2025-01-08-12-52-47.gh-issue-128640.9nbh9z.rst

@@ -1395,12 +1395,9 @@ static int test_audit_subinterpreter(void)
     PySys_AddAuditHook(_audit_subinterpreter_hook, NULL);
     _testembed_initialize();
 
-    PyThreadState *tstate = PyThreadState_Get();
-    for (int i = 0; i < 3; ++i)
-    {
-        Py_EndInterpreter(Py_NewInterpreter());
-        PyThreadState_Swap(tstate);
-    }
+    Py_NewInterpreter();
+    Py_NewInterpreter();
+    Py_NewInterpreter();
 
     Py_Finalize();
 

Programs/_testembed.c

@@ -1992,7 +1992,6 @@ resolve_final_tstate(_PyRuntimeState *runtime)
             }
             else {
                 /* Fall back to the current tstate.  It's better than nothing. */
-                // XXX No it's not
                 main_tstate = tstate;
             }
         }
@@ -2038,16 +2037,6 @@ _Py_Finalize(_PyRuntimeState *runtime)
 
     _PyAtExit_Call(tstate->interp);
 
-    /* Clean up any lingering subinterpreters.
-
-       Two preconditions need to be met here:
-
-        - This has to happen before _PyRuntimeState_SetFinalizing is
-          called, or else threads might get prematurely blocked.
-        - The world must not be stopped, as finalizers can run.
-    */
-    finalize_subinterpreters();
-
     assert(_PyThreadState_GET() == tstate);
 
     /* Copy the core config, PyInterpreterState_Delete() free
@@ -2135,6 +2124,9 @@ _Py_Finalize(_PyRuntimeState *runtime)
     _PyImport_FiniExternal(tstate->interp);
     finalize_modules(tstate);
 
+    /* Clean up any lingering subinterpreters. */
+    finalize_subinterpreters();
+
     /* Print debug stats if any */
     _PyEval_Fini();
 
@@ -2418,8 +2410,9 @@ Py_NewInterpreter(void)
     return tstate;
 }
 
-/* Delete an interpreter.  This requires that the given thread state
-   is current, and that the thread has no remaining frames.
+/* Delete an interpreter and its last thread.  This requires that the
+   given thread state is current, that the thread has no remaining
+   frames, and that it is its interpreter's only remaining thread.
    It is a fatal error to violate these constraints.
 
    (Py_FinalizeEx() doesn't have these constraints -- it zaps
@@ -2449,15 +2442,14 @@ Py_EndInterpreter(PyThreadState *tstate)
     _Py_FinishPendingCalls(tstate);
 
     _PyAtExit_Call(tstate->interp);
-    _PyRuntimeState *runtime = interp->runtime;
-    _PyEval_StopTheWorldAll(runtime);
-    PyThreadState *list = _PyThreadState_RemoveExcept(tstate);
+
+    if (tstate != interp->threads.head || tstate->next != NULL) {
+        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);
-    _PyEval_StartTheWorldAll(runtime);
-    _PyThreadState_DeleteList(list, /*is_after_fork=*/0);
 
     // XXX Call something like _PyImport_Disable() here?
 
@@ -2488,8 +2480,6 @@ finalize_subinterpreters(void)
     PyInterpreterState *main_interp = _PyInterpreterState_Main();
     assert(final_tstate->interp == main_interp);
     _PyRuntimeState *runtime = main_interp->runtime;
-    assert(!runtime->stoptheworld.world_stopped);
-    assert(_PyRuntimeState_GetFinalizing(runtime) == NULL);
     struct pyinterpreters *interpreters = &runtime->interpreters;
 
     /* Get the first interpreter in the list. */
@@ -2518,17 +2508,27 @@ finalize_subinterpreters(void)
 
     /* Clean up all remaining subinterpreters. */
     while (interp != NULL) {
-        /* Make a tstate for finalization. */
-        PyThreadState *tstate = _PyThreadState_NewBound(interp, _PyThreadState_WHENCE_FINI);
-        if (tstate == NULL) {
-            // XXX Some graceful way to always get a thread state?
-            Py_FatalError("thread state allocation failed");
+        assert(!_PyInterpreterState_IsRunningMain(interp));
+
+        /* Find the tstate to use for fini.  We assume the interpreter
+           will have at most one tstate at this point. */
+        PyThreadState *tstate = interp->threads.head;
+        if (tstate != NULL) {
+            /* Ideally we would be able to use tstate as-is, and rely
+               on it being in a ready state: no exception set, not
+               running anything (tstate->current_frame), matching the
+               current thread ID (tstate->thread_id).  To play it safe,
+               we always delete it and use a fresh tstate instead. */
+            assert(tstate != final_tstate);
+            _PyThreadState_Attach(tstate);
+            PyThreadState_Clear(tstate);
+            _PyThreadState_Detach(tstate);
+            PyThreadState_Delete(tstate);
         }
-
-        /* Enter the subinterpreter. */
-        _PyThreadState_Attach(tstate);
+        tstate = _PyThreadState_NewBound(interp, _PyThreadState_WHENCE_FINI);
 
         /* Destroy the subinterpreter. */
+        _PyThreadState_Attach(tstate);
         Py_EndInterpreter(tstate);
         assert(_PyThreadState_GET() == NULL);