https://github.com/python/cpython/commit/27bd08273ce822a4dbe0e73cca47441e99fd6f0d
commit: 27bd08273ce822a4dbe0e73cca47441e99fd6f0d
branch: main
author: Peter Bierma <zintensity...@gmail.com>
committer: ericsnowcurrently <ericsnowcurren...@gmail.com>
date: 2025-05-19T12:22:05-06:00
summary:
Revert "gh-128639: Don't assume one thread in subinterpreter finalization
(gh-128640)" (gh-134256)
This reverts commit 9859791f9e116c827468f307ac0770286c975c8b.
The original change broke the iOS and android buildbots, where the tests are
run single-process.
files:
D
Misc/NEWS.d/next/Core_and_Builtins/2025-01-08-12-52-47.gh-issue-128640.9nbh9z.rst
M Lib/test/test_interpreters/test_api.py
M Lib/test/test_interpreters/test_lifecycle.py
M Lib/test/test_threading.py
M Programs/_testembed.c
M Python/pylifecycle.c
diff --git a/Lib/test/test_interpreters/test_api.py
b/Lib/test/test_interpreters/test_api.py
index c7ee114fe0838c..1e2d572b1cbb81 100644
--- a/Lib/test/test_interpreters/test_api.py
+++ b/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()}
diff --git a/Lib/test/test_interpreters/test_lifecycle.py
b/Lib/test/test_interpreters/test_lifecycle.py
index 3f9ed1fb501522..ac24f6568acd95 100644
--- a/Lib/test/test_interpreters/test_lifecycle.py
+++ b/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)
diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
index 127ef658673612..b6e2d419019aa1 100644
--- a/Lib/test/test_threading.py
+++ b/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,
diff --git
a/Misc/NEWS.d/next/Core_and_Builtins/2025-01-08-12-52-47.gh-issue-128640.9nbh9z.rst
b/Misc/NEWS.d/next/Core_and_Builtins/2025-01-08-12-52-47.gh-issue-128640.9nbh9z.rst
deleted file mode 100644
index 040c6d56c47244..00000000000000
---
a/Misc/NEWS.d/next/Core_and_Builtins/2025-01-08-12-52-47.gh-issue-128640.9nbh9z.rst
+++ /dev/null
@@ -1 +0,0 @@
-Fix a crash when using threads inside of a subinterpreter.
diff --git a/Programs/_testembed.c b/Programs/_testembed.c
index 0a19d1126e246e..8e0e330f6605c9 100644
--- a/Programs/_testembed.c
+++ b/Programs/_testembed.c
@@ -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();
diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c
index b6bc2ea5211460..8394245d373030 100644
--- a/Python/pylifecycle.c
+++ b/Python/pylifecycle.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);
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com
