Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 9859791

Browse filesBrowse files
ZeroIntensityZeroIntensity
authored
pythongh-128639: Don't assume one thread in subinterpreter finalization (pythongh-128640)
Incidentally, this also fixed the warning not showing up if a subinterpreter wasn't cleaned up via _interpreters.destroy. I had to update some of the tests as a result.
1 parent c4ad92e commit 9859791
Copy full SHA for 9859791

File tree

Expand file treeCollapse file tree

6 files changed

+99
-38
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+99
-38
lines changed

‎Lib/test/test_interpreters/test_api.py

Copy file name to clipboardExpand all lines: Lib/test/test_interpreters/test_api.py
+58-2Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,59 @@ def test_created_with_capi(self):
647647
self.interp_exists(interpid))
648648

649649

650+
def test_remaining_threads(self):
651+
r_interp, w_interp = self.pipe()
652+
653+
FINISHED = b'F'
654+
655+
# It's unlikely, but technically speaking, it's possible
656+
# that the thread could've finished before interp.close() is
657+
# reached, so this test might not properly exercise the case.
658+
# However, it's quite unlikely and I'm too lazy to deal with it.
659+
interp = interpreters.create()
660+
interp.exec(f"""if True:
661+
import os
662+
import threading
663+
import time
664+
665+
def task():
666+
time.sleep(1)
667+
os.write({w_interp}, {FINISHED!r})
668+
669+
threads = [threading.Thread(target=task) for _ in range(3)]
670+
for t in threads:
671+
t.start()
672+
""")
673+
interp.close()
674+
675+
self.assertEqual(os.read(r_interp, 1), FINISHED)
676+
677+
def test_remaining_daemon_threads(self):
678+
interp = _interpreters.create(
679+
types.SimpleNamespace(
680+
use_main_obmalloc=False,
681+
allow_fork=False,
682+
allow_exec=False,
683+
allow_threads=True,
684+
allow_daemon_threads=True,
685+
check_multi_interp_extensions=True,
686+
gil='own',
687+
)
688+
)
689+
_interpreters.exec(interp, f"""if True:
690+
import threading
691+
import time
692+
693+
def task():
694+
time.sleep(100)
695+
696+
threads = [threading.Thread(target=task, daemon=True) for _ in range(3)]
697+
for t in threads:
698+
t.start()
699+
""")
700+
_interpreters.destroy(interp)
701+
702+
650703
class TestInterpreterPrepareMain(TestBase):
651704

652705
def test_empty(self):
@@ -755,7 +808,10 @@ def script():
755808
spam.eggs()
756809
757810
interp = interpreters.create()
758-
interp.exec(script)
811+
try:
812+
interp.exec(script)
813+
finally:
814+
interp.close()
759815
""")
760816

761817
stdout, stderr = self.assert_python_failure(scriptfile)
@@ -764,7 +820,7 @@ def script():
764820
# File "{interpreters.__file__}", line 179, in exec
765821
self.assertEqual(stderr, dedent(f"""\
766822
Traceback (most recent call last):
767-
File "{scriptfile}", line 9, in <module>
823+
File "{scriptfile}", line 10, in <module>
768824
interp.exec(script)
769825
~~~~~~~~~~~^^^^^^^^
770826
{interpmod_line.strip()}

‎Lib/test/test_interpreters/test_lifecycle.py

Copy file name to clipboardExpand all lines: Lib/test/test_interpreters/test_lifecycle.py
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ def test_sys_path_0(self):
132132
'sub': sys.path[0],
133133
}}, indent=4), flush=True)
134134
""")
135+
interp.close()
135136
'''
136137
# <tmp>/
137138
# pkg/
@@ -172,7 +173,10 @@ def test_gh_109793(self):
172173
argv = [sys.executable, '-c', '''if True:
173174
from test.support import interpreters
174175
interp = interpreters.create()
175-
raise Exception
176+
try:
177+
raise Exception
178+
finally:
179+
interp.close()
176180
''']
177181
proc = subprocess.run(argv, capture_output=True, text=True)
178182
self.assertIn('Traceback', proc.stderr)

‎Lib/test/test_threading.py

Copy file name to clipboardExpand all lines: Lib/test/test_threading.py
+1-4Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1689,10 +1689,7 @@ def f():
16891689
16901690
_testcapi.run_in_subinterp(%r)
16911691
""" % (subinterp_code,)
1692-
with test.support.SuppressCrashReport():
1693-
rc, out, err = assert_python_failure("-c", script)
1694-
self.assertIn("Fatal Python error: Py_EndInterpreter: "
1695-
"not the last thread", err.decode())
1692+
assert_python_ok("-c", script)
16961693

16971694
def _check_allowed(self, before_start='', *,
16981695
allowed=True,

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

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix a crash when using threads inside of a subinterpreter.

‎Programs/_testembed.c

Copy file name to clipboardExpand all lines: Programs/_testembed.c
+6-3Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1395,9 +1395,12 @@ static int test_audit_subinterpreter(void)
13951395
PySys_AddAuditHook(_audit_subinterpreter_hook, NULL);
13961396
_testembed_initialize();
13971397

1398-
Py_NewInterpreter();
1399-
Py_NewInterpreter();
1400-
Py_NewInterpreter();
1398+
PyThreadState *tstate = PyThreadState_Get();
1399+
for (int i = 0; i < 3; ++i)
1400+
{
1401+
Py_EndInterpreter(Py_NewInterpreter());
1402+
PyThreadState_Swap(tstate);
1403+
}
14011404

14021405
Py_Finalize();
14031406

‎Python/pylifecycle.c

Copy file name to clipboardExpand all lines: Python/pylifecycle.c
+28-28Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1992,6 +1992,7 @@ resolve_final_tstate(_PyRuntimeState *runtime)
19921992
}
19931993
else {
19941994
/* Fall back to the current tstate. It's better than nothing. */
1995+
// XXX No it's not
19951996
main_tstate = tstate;
19961997
}
19971998
}
@@ -2037,6 +2038,16 @@ _Py_Finalize(_PyRuntimeState *runtime)
20372038

20382039
_PyAtExit_Call(tstate->interp);
20392040

2041+
/* Clean up any lingering subinterpreters.
2042+
2043+
Two preconditions need to be met here:
2044+
2045+
- This has to happen before _PyRuntimeState_SetFinalizing is
2046+
called, or else threads might get prematurely blocked.
2047+
- The world must not be stopped, as finalizers can run.
2048+
*/
2049+
finalize_subinterpreters();
2050+
20402051
assert(_PyThreadState_GET() == tstate);
20412052

20422053
/* Copy the core config, PyInterpreterState_Delete() free
@@ -2124,9 +2135,6 @@ _Py_Finalize(_PyRuntimeState *runtime)
21242135
_PyImport_FiniExternal(tstate->interp);
21252136
finalize_modules(tstate);
21262137

2127-
/* Clean up any lingering subinterpreters. */
2128-
finalize_subinterpreters();
2129-
21302138
/* Print debug stats if any */
21312139
_PyEval_Fini();
21322140

@@ -2410,9 +2418,8 @@ Py_NewInterpreter(void)
24102418
return tstate;
24112419
}
24122420

2413-
/* Delete an interpreter and its last thread. This requires that the
2414-
given thread state is current, that the thread has no remaining
2415-
frames, and that it is its interpreter's only remaining thread.
2421+
/* Delete an interpreter. This requires that the given thread state
2422+
is current, and that the thread has no remaining frames.
24162423
It is a fatal error to violate these constraints.
24172424
24182425
(Py_FinalizeEx() doesn't have these constraints -- it zaps
@@ -2442,14 +2449,15 @@ Py_EndInterpreter(PyThreadState *tstate)
24422449
_Py_FinishPendingCalls(tstate);
24432450

24442451
_PyAtExit_Call(tstate->interp);
2445-
2446-
if (tstate != interp->threads.head || tstate->next != NULL) {
2447-
Py_FatalError("not the last thread");
2448-
}
2452+
_PyRuntimeState *runtime = interp->runtime;
2453+
_PyEval_StopTheWorldAll(runtime);
2454+
PyThreadState *list = _PyThreadState_RemoveExcept(tstate);
24492455

24502456
/* Remaining daemon threads will automatically exit
24512457
when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
24522458
_PyInterpreterState_SetFinalizing(interp, tstate);
2459+
_PyEval_StartTheWorldAll(runtime);
2460+
_PyThreadState_DeleteList(list, /*is_after_fork=*/0);
24532461

24542462
// XXX Call something like _PyImport_Disable() here?
24552463

@@ -2480,6 +2488,8 @@ finalize_subinterpreters(void)
24802488
PyInterpreterState *main_interp = _PyInterpreterState_Main();
24812489
assert(final_tstate->interp == main_interp);
24822490
_PyRuntimeState *runtime = main_interp->runtime;
2491+
assert(!runtime->stoptheworld.world_stopped);
2492+
assert(_PyRuntimeState_GetFinalizing(runtime) == NULL);
24832493
struct pyinterpreters *interpreters = &runtime->interpreters;
24842494

24852495
/* Get the first interpreter in the list. */
@@ -2508,27 +2518,17 @@ finalize_subinterpreters(void)
25082518

25092519
/* Clean up all remaining subinterpreters. */
25102520
while (interp != NULL) {
2511-
assert(!_PyInterpreterState_IsRunningMain(interp));
2512-
2513-
/* Find the tstate to use for fini. We assume the interpreter
2514-
will have at most one tstate at this point. */
2515-
PyThreadState *tstate = interp->threads.head;
2516-
if (tstate != NULL) {
2517-
/* Ideally we would be able to use tstate as-is, and rely
2518-
on it being in a ready state: no exception set, not
2519-
running anything (tstate->current_frame), matching the
2520-
current thread ID (tstate->thread_id). To play it safe,
2521-
we always delete it and use a fresh tstate instead. */
2522-
assert(tstate != final_tstate);
2523-
_PyThreadState_Attach(tstate);
2524-
PyThreadState_Clear(tstate);
2525-
_PyThreadState_Detach(tstate);
2526-
PyThreadState_Delete(tstate);
2521+
/* Make a tstate for finalization. */
2522+
PyThreadState *tstate = _PyThreadState_NewBound(interp, _PyThreadState_WHENCE_FINI);
2523+
if (tstate == NULL) {
2524+
// XXX Some graceful way to always get a thread state?
2525+
Py_FatalError("thread state allocation failed");
25272526
}
2528-
tstate = _PyThreadState_NewBound(interp, _PyThreadState_WHENCE_FINI);
25292527

2530-
/* Destroy the subinterpreter. */
2528+
/* Enter the subinterpreter. */
25312529
_PyThreadState_Attach(tstate);
2530+
2531+
/* Destroy the subinterpreter. */
25322532
Py_EndInterpreter(tstate);
25332533
assert(_PyThreadState_GET() == NULL);
25342534

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.