Skip to content

Commit 999855b

Browse files
authored
gh-159041: Fix use-after-free in asyncio task tracking during thread state cleanup (#159073)
1 parent 7dbc797 commit 999855b

5 files changed

Lines changed: 117 additions & 13 deletions

File tree

‎Lib/test/test_asyncio/test_tasks.py‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Tests for tasks.py."""
22

3+
import _thread
34
import collections
45
import contextlib
56
import contextvars
@@ -20,6 +21,7 @@
2021
from asyncio import tasks
2122
from test.test_asyncio import utils as test_utils
2223
from test import support
24+
from test.support import threading_helper
2325
from test.support.script_helper import assert_python_ok
2426

2527

@@ -3095,6 +3097,39 @@ class CTask_CFuture_Tests(BaseTaskTests, SetMethodsTest,
30953097
all_tasks = getattr(tasks, '_c_all_tasks', None)
30963098
current_task = staticmethod(getattr(tasks, '_c_current_task', None))
30973099

3100+
@threading_helper.requires_working_threading()
3101+
def test_task_created_during_thread_state_clear(self):
3102+
loop = self.loop
3103+
task = None
3104+
var = contextvars.ContextVar('var')
3105+
# Avoid recreating the thread state's context after it is cleared.
3106+
task_context = contextvars.Context()
3107+
3108+
async def noop():
3109+
pass
3110+
3111+
class CreatesTaskOnClear:
3112+
def __del__(self):
3113+
nonlocal task
3114+
task = loop.create_task(noop(), context=task_context)
3115+
3116+
def create_finalizer():
3117+
# gh-159041: Run the finalizer when PyThreadState_Clear()
3118+
# clears the thread's context.
3119+
var.set(CreatesTaskOnClear())
3120+
3121+
# threading.Thread owns its Context, so use _thread to make the
3122+
# Context lifetime match the thread state's lifetime.
3123+
handle = _thread.start_joinable_thread(create_finalizer)
3124+
handle.join(support.SHORT_TIMEOUT)
3125+
self.assertTrue(handle.is_done(), 'thread failed to exit')
3126+
self.assertIsNotNone(task)
3127+
3128+
try:
3129+
self.assertEqual(self.all_tasks(loop), {task})
3130+
finally:
3131+
loop.run_until_complete(task)
3132+
30983133
def test_del__log_destroy_pending_segfault(self):
30993134
async def coro():
31003135
pass

‎Lib/test/test_embed.py‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,43 @@ def test_finalize_structseq(self):
440440
out, err = self.run_embedded_interpreter("test_repeated_init_exec", code)
441441
self.assertEqual(out, 'Tests passed\n' * INIT_LOOPS)
442442

443+
def test_asyncio_task_during_interpreter_clear(self):
444+
import_helper.import_module('_asyncio')
445+
code = textwrap.dedent("""
446+
import asyncio
447+
import contextvars
448+
import gc
449+
import os
450+
451+
gc.disable()
452+
453+
async def noop():
454+
pass
455+
456+
class Loop:
457+
def get_debug(self):
458+
return False
459+
460+
def call_soon(self, callback, *args, context=None):
461+
self.callback = callback
462+
463+
class CreatesTaskOnClear:
464+
def __del__(self):
465+
asyncio.Task(noop(), loop=Loop(), context=contextvars.Context())
466+
os.write(1, b'task created\\n')
467+
468+
# Keep the finalizer alive until interpreter_clear()'s last GC.
469+
finalizer = CreatesTaskOnClear()
470+
finalizer.cycle = finalizer
471+
del finalizer
472+
""")
473+
# gh-159041: Delete the cleared interpreter with no thread state attached.
474+
out, err = self.run_embedded_interpreter(
475+
"test_interpreter_clear_and_delete", code,
476+
timeout=support.SHORT_TIMEOUT)
477+
self.assertEqual(out, "task created\n")
478+
self.assertEqual(err, "")
479+
443480
def test_simple_initialization_api(self):
444481
# _testembed now uses Py_InitializeFromConfig by default
445482
# This case specifically checks Py_Initialize(Ex) still works
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a use-after-free in :mod:`asyncio` task bookkeeping when a finalizer
2+
creates a task during thread shutdown.

‎Programs/_testembed.c‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,23 @@ static int test_repeated_init_exec(void)
268268
return 0;
269269
}
270270

271+
272+
static int test_interpreter_clear_and_delete(void)
273+
{
274+
assert(main_argc == 3);
275+
_testembed_initialize();
276+
if (PyRun_SimpleString(main_argv[2]) < 0) {
277+
return 1;
278+
}
279+
280+
PyInterpreterState *interp = PyInterpreterState_Get();
281+
PyInterpreterState_Clear(interp);
282+
PyThreadState_Swap(NULL);
283+
PyInterpreterState_Delete(interp);
284+
// The interpreter has already been deleted, so don't call Py_FinalizeEx().
285+
return 0;
286+
}
287+
271288
/****************************************************************************
272289
* Test the Py_Initialize(Ex) convenience/compatibility wrappers
273290
***************************************************************************/
@@ -3001,6 +3018,7 @@ struct TestCase
30013018
static struct TestCase TestCases[] = {
30023019
// Python initialization
30033020
{"test_repeated_init_exec", test_repeated_init_exec},
3021+
{"test_interpreter_clear_and_delete", test_interpreter_clear_and_delete},
30043022
{"test_repeated_simple_init", test_repeated_simple_init},
30053023
{"test_forced_io_encoding", test_forced_io_encoding},
30063024
{"test_import_in_subinterpreters", test_import_in_subinterpreters},

‎Python/pystate.c‎

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1837,13 +1837,6 @@ PyThreadState_Clear(PyThreadState *tstate)
18371837
Py_CLEAR(((_PyThreadStateImpl *)tstate)->asyncio_running_task);
18381838

18391839

1840-
PyMutex_Lock(&tstate->interp->asyncio_tasks_lock);
1841-
// merge any lingering tasks from thread state to interpreter's
1842-
// tasks list
1843-
llist_concat(&tstate->interp->asyncio_tasks_head,
1844-
&((_PyThreadStateImpl *)tstate)->asyncio_tasks_head);
1845-
PyMutex_Unlock(&tstate->interp->asyncio_tasks_lock);
1846-
18471840
Py_CLEAR(tstate->dict);
18481841
Py_CLEAR(tstate->async_exc);
18491842

@@ -1919,11 +1912,30 @@ PyThreadState_Clear(PyThreadState *tstate)
19191912
static void
19201913
decrement_stoptheworld_countdown(struct _stoptheworld_state *stw);
19211914

1922-
/* Common code for PyThreadState_Delete() and PyThreadState_DeleteCurrent() */
1915+
// `release_gil` releases the current thread's GIL. For states already removed
1916+
// by _PyThreadState_RemoveExcept(), `is_unlinked` limits cleanup to task transfer.
19231917
static void
1924-
tstate_delete_common(PyThreadState *tstate, int release_gil)
1918+
tstate_delete_common(PyThreadState *tstate, int release_gil, int is_unlinked)
19251919
{
19261920
assert(tstate->_status.cleared && !tstate->_status.finalized);
1921+
assert(!is_unlinked || !release_gil);
1922+
1923+
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
1924+
// Finalizers may register tasks during or after PyThreadState_Clear().
1925+
// Transfer them before normal unlinking or GIL release: cross-thread task
1926+
// removal uses stop-the-world or the GIL, not asyncio_tasks_lock.
1927+
if (!llist_empty(&tstate_impl->asyncio_tasks_head)) {
1928+
// Serialize transfers from concurrently exiting threads.
1929+
PyMutex_Lock(&tstate->interp->asyncio_tasks_lock);
1930+
llist_concat(&tstate->interp->asyncio_tasks_head,
1931+
&tstate_impl->asyncio_tasks_head);
1932+
PyMutex_Unlock(&tstate->interp->asyncio_tasks_lock);
1933+
}
1934+
assert(llist_empty(&tstate_impl->asyncio_tasks_head));
1935+
if (is_unlinked) {
1936+
return;
1937+
}
1938+
19271939
tstate_verify_not_active(tstate);
19281940
assert(!_PyThreadState_IsRunningMain(tstate));
19291941

@@ -1959,7 +1971,6 @@ tstate_delete_common(PyThreadState *tstate, int release_gil)
19591971

19601972
#if defined(Py_REF_DEBUG) && defined(Py_GIL_DISABLED)
19611973
// Add our portion of the total refcount to the interpreter's total.
1962-
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
19631974
tstate->interp->object_state.reftotal += tstate_impl->reftotal;
19641975
tstate_impl->reftotal = 0;
19651976
assert(tstate_impl->refcounts.values == NULL);
@@ -2006,7 +2017,7 @@ zapthreads(PyInterpreterState *interp)
20062017
*/
20072018
while ((tstate = interp->threads.head) != NULL) {
20082019
tstate_verify_not_active(tstate);
2009-
tstate_delete_common(tstate, 0);
2020+
tstate_delete_common(tstate, /*release_gil=*/0, /*is_unlinked=*/0);
20102021
free_threadstate((_PyThreadStateImpl *)tstate);
20112022
}
20122023
}
@@ -2017,7 +2028,7 @@ PyThreadState_Delete(PyThreadState *tstate)
20172028
{
20182029
_Py_EnsureTstateNotNULL(tstate);
20192030
tstate_verify_not_active(tstate);
2020-
tstate_delete_common(tstate, 0);
2031+
tstate_delete_common(tstate, /*release_gil=*/0, /*is_unlinked=*/0);
20212032
free_threadstate((_PyThreadStateImpl *)tstate);
20222033
}
20232034

@@ -2033,7 +2044,7 @@ _PyThreadState_DeleteCurrent(PyThreadState *tstate)
20332044
_PyStats_Detach((_PyThreadStateImpl *)tstate);
20342045
#endif
20352046
current_fast_clear(tstate->interp->runtime);
2036-
tstate_delete_common(tstate, 1); // release GIL as part of call
2047+
tstate_delete_common(tstate, /*release_gil=*/1, /*is_unlinked=*/0);
20372048
free_threadstate((_PyThreadStateImpl *)tstate);
20382049
}
20392050

@@ -2103,6 +2114,7 @@ _PyThreadState_DeleteList(PyThreadState *list, int is_after_fork)
21032114
for (p = list; p; p = next) {
21042115
next = p->next;
21052116
PyThreadState_Clear(p);
2117+
tstate_delete_common(p, /*release_gil=*/0, /*is_unlinked=*/1);
21062118
if (is_after_fork) {
21072119
free_threadstate((_PyThreadStateImpl *)p);
21082120
}

0 commit comments

Comments
 (0)