Skip to content

Commit faf16ef

Browse files
[3.15] gh-159041: Fix use-after-free in asyncio task tracking during thread state cleanup (GH-159073) (#159694)
gh-159041: Fix use-after-free in asyncio task tracking during thread state cleanup (GH-159073) (cherry picked from commit 999855b) Co-authored-by: Nathan Goldbaum <nathan.goldbaum@gmail.com>
1 parent c9e0918 commit faf16ef

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
@@ -19,6 +20,7 @@
1920
from asyncio import tasks
2021
from test.test_asyncio import utils as test_utils
2122
from test import support
23+
from test.support import threading_helper
2224
from test.support.script_helper import assert_python_ok
2325
from test.support.warnings_helper import ignore_warnings
2426

@@ -3059,6 +3061,39 @@ class CTask_CFuture_Tests(BaseTaskTests, SetMethodsTest,
30593061
all_tasks = getattr(tasks, '_c_all_tasks', None)
30603062
current_task = staticmethod(getattr(tasks, '_c_current_task', None))
30613063

3064+
@threading_helper.requires_working_threading()
3065+
def test_task_created_during_thread_state_clear(self):
3066+
loop = self.loop
3067+
task = None
3068+
var = contextvars.ContextVar('var')
3069+
# Avoid recreating the thread state's context after it is cleared.
3070+
task_context = contextvars.Context()
3071+
3072+
async def noop():
3073+
pass
3074+
3075+
class CreatesTaskOnClear:
3076+
def __del__(self):
3077+
nonlocal task
3078+
task = loop.create_task(noop(), context=task_context)
3079+
3080+
def create_finalizer():
3081+
# gh-159041: Run the finalizer when PyThreadState_Clear()
3082+
# clears the thread's context.
3083+
var.set(CreatesTaskOnClear())
3084+
3085+
# threading.Thread owns its Context, so use _thread to make the
3086+
# Context lifetime match the thread state's lifetime.
3087+
handle = _thread.start_joinable_thread(create_finalizer)
3088+
handle.join(support.SHORT_TIMEOUT)
3089+
self.assertTrue(handle.is_done(), 'thread failed to exit')
3090+
self.assertIsNotNone(task)
3091+
3092+
try:
3093+
self.assertEqual(self.all_tasks(loop), {task})
3094+
finally:
3095+
loop.run_until_complete(task)
3096+
30623097
def test_del__log_destroy_pending_segfault(self):
30633098
async def coro():
30643099
pass

‎Lib/test/test_embed.py‎

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

418+
def test_asyncio_task_during_interpreter_clear(self):
419+
import_helper.import_module('_asyncio')
420+
code = textwrap.dedent("""
421+
import asyncio
422+
import contextvars
423+
import gc
424+
import os
425+
426+
gc.disable()
427+
428+
async def noop():
429+
pass
430+
431+
class Loop:
432+
def get_debug(self):
433+
return False
434+
435+
def call_soon(self, callback, *args, context=None):
436+
self.callback = callback
437+
438+
class CreatesTaskOnClear:
439+
def __del__(self):
440+
asyncio.Task(noop(), loop=Loop(), context=contextvars.Context())
441+
os.write(1, b'task created\\n')
442+
443+
# Keep the finalizer alive until interpreter_clear()'s last GC.
444+
finalizer = CreatesTaskOnClear()
445+
finalizer.cycle = finalizer
446+
del finalizer
447+
""")
448+
# gh-159041: Delete the cleared interpreter with no thread state attached.
449+
out, err = self.run_embedded_interpreter(
450+
"test_interpreter_clear_and_delete", code,
451+
timeout=support.SHORT_TIMEOUT)
452+
self.assertEqual(out, "task created\n")
453+
self.assertEqual(err, "")
454+
418455
def test_simple_initialization_api(self):
419456
# _testembed now uses Py_InitializeFromConfig by default
420457
# 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
@@ -256,6 +256,23 @@ static int test_repeated_init_exec(void)
256256
return 0;
257257
}
258258

259+
260+
static int test_interpreter_clear_and_delete(void)
261+
{
262+
assert(main_argc == 3);
263+
_testembed_initialize();
264+
if (PyRun_SimpleString(main_argv[2]) < 0) {
265+
return 1;
266+
}
267+
268+
PyInterpreterState *interp = PyInterpreterState_Get();
269+
PyInterpreterState_Clear(interp);
270+
PyThreadState_Swap(NULL);
271+
PyInterpreterState_Delete(interp);
272+
// The interpreter has already been deleted, so don't call Py_FinalizeEx().
273+
return 0;
274+
}
275+
259276
/****************************************************************************
260277
* Test the Py_Initialize(Ex) convenience/compatibility wrappers
261278
***************************************************************************/
@@ -3049,6 +3066,7 @@ struct TestCase
30493066
static struct TestCase TestCases[] = {
30503067
// Python initialization
30513068
{"test_repeated_init_exec", test_repeated_init_exec},
3069+
{"test_interpreter_clear_and_delete", test_interpreter_clear_and_delete},
30523070
{"test_repeated_simple_init", test_repeated_simple_init},
30533071
{"test_forced_io_encoding", test_forced_io_encoding},
30543072
{"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
@@ -1808,13 +1808,6 @@ PyThreadState_Clear(PyThreadState *tstate)
18081808
Py_CLEAR(((_PyThreadStateImpl *)tstate)->asyncio_running_task);
18091809

18101810

1811-
PyMutex_Lock(&tstate->interp->asyncio_tasks_lock);
1812-
// merge any lingering tasks from thread state to interpreter's
1813-
// tasks list
1814-
llist_concat(&tstate->interp->asyncio_tasks_head,
1815-
&((_PyThreadStateImpl *)tstate)->asyncio_tasks_head);
1816-
PyMutex_Unlock(&tstate->interp->asyncio_tasks_lock);
1817-
18181811
Py_CLEAR(tstate->dict);
18191812
Py_CLEAR(tstate->async_exc);
18201813

@@ -1890,11 +1883,30 @@ PyThreadState_Clear(PyThreadState *tstate)
18901883
static void
18911884
decrement_stoptheworld_countdown(struct _stoptheworld_state *stw);
18921885

1893-
/* Common code for PyThreadState_Delete() and PyThreadState_DeleteCurrent() */
1886+
// `release_gil` releases the current thread's GIL. For states already removed
1887+
// by _PyThreadState_RemoveExcept(), `is_unlinked` limits cleanup to task transfer.
18941888
static void
1895-
tstate_delete_common(PyThreadState *tstate, int release_gil)
1889+
tstate_delete_common(PyThreadState *tstate, int release_gil, int is_unlinked)
18961890
{
18971891
assert(tstate->_status.cleared && !tstate->_status.finalized);
1892+
assert(!is_unlinked || !release_gil);
1893+
1894+
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
1895+
// Finalizers may register tasks during or after PyThreadState_Clear().
1896+
// Transfer them before normal unlinking or GIL release: cross-thread task
1897+
// removal uses stop-the-world or the GIL, not asyncio_tasks_lock.
1898+
if (!llist_empty(&tstate_impl->asyncio_tasks_head)) {
1899+
// Serialize transfers from concurrently exiting threads.
1900+
PyMutex_Lock(&tstate->interp->asyncio_tasks_lock);
1901+
llist_concat(&tstate->interp->asyncio_tasks_head,
1902+
&tstate_impl->asyncio_tasks_head);
1903+
PyMutex_Unlock(&tstate->interp->asyncio_tasks_lock);
1904+
}
1905+
assert(llist_empty(&tstate_impl->asyncio_tasks_head));
1906+
if (is_unlinked) {
1907+
return;
1908+
}
1909+
18981910
tstate_verify_not_active(tstate);
18991911
assert(!_PyThreadState_IsRunningMain(tstate));
19001912

@@ -1930,7 +1942,6 @@ tstate_delete_common(PyThreadState *tstate, int release_gil)
19301942

19311943
#if defined(Py_REF_DEBUG) && defined(Py_GIL_DISABLED)
19321944
// Add our portion of the total refcount to the interpreter's total.
1933-
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
19341945
tstate->interp->object_state.reftotal += tstate_impl->reftotal;
19351946
tstate_impl->reftotal = 0;
19361947
assert(tstate_impl->refcounts.values == NULL);
@@ -1977,7 +1988,7 @@ zapthreads(PyInterpreterState *interp)
19771988
*/
19781989
while ((tstate = interp->threads.head) != NULL) {
19791990
tstate_verify_not_active(tstate);
1980-
tstate_delete_common(tstate, 0);
1991+
tstate_delete_common(tstate, /*release_gil=*/0, /*is_unlinked=*/0);
19811992
free_threadstate((_PyThreadStateImpl *)tstate);
19821993
}
19831994
}
@@ -1988,7 +1999,7 @@ PyThreadState_Delete(PyThreadState *tstate)
19881999
{
19892000
_Py_EnsureTstateNotNULL(tstate);
19902001
tstate_verify_not_active(tstate);
1991-
tstate_delete_common(tstate, 0);
2002+
tstate_delete_common(tstate, /*release_gil=*/0, /*is_unlinked=*/0);
19922003
free_threadstate((_PyThreadStateImpl *)tstate);
19932004
}
19942005

@@ -2004,7 +2015,7 @@ _PyThreadState_DeleteCurrent(PyThreadState *tstate)
20042015
_PyStats_Detach((_PyThreadStateImpl *)tstate);
20052016
#endif
20062017
current_fast_clear(tstate->interp->runtime);
2007-
tstate_delete_common(tstate, 1); // release GIL as part of call
2018+
tstate_delete_common(tstate, /*release_gil=*/1, /*is_unlinked=*/0);
20082019
free_threadstate((_PyThreadStateImpl *)tstate);
20092020
}
20102021

@@ -2074,6 +2085,7 @@ _PyThreadState_DeleteList(PyThreadState *list, int is_after_fork)
20742085
for (p = list; p; p = next) {
20752086
next = p->next;
20762087
PyThreadState_Clear(p);
2088+
tstate_delete_common(p, /*release_gil=*/0, /*is_unlinked=*/1);
20772089
if (is_after_fork) {
20782090
free_threadstate((_PyThreadStateImpl *)p);
20792091
}

0 commit comments

Comments
 (0)