Skip to content

Commit c542ecd

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

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

@@ -3021,6 +3023,39 @@ async def coro():
30213023
self.loop.run_until_complete(task)
30223024
self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10)
30233025

3026+
@threading_helper.requires_working_threading()
3027+
def test_task_created_during_thread_state_clear(self):
3028+
loop = self.loop
3029+
task = None
3030+
var = contextvars.ContextVar('var')
3031+
# Avoid recreating the thread state's context after it is cleared.
3032+
task_context = contextvars.Context()
3033+
3034+
async def noop():
3035+
pass
3036+
3037+
class CreatesTaskOnClear:
3038+
def __del__(self):
3039+
nonlocal task
3040+
task = loop.create_task(noop(), context=task_context)
3041+
3042+
def create_finalizer():
3043+
# gh-159041: Run the finalizer when PyThreadState_Clear()
3044+
# clears the thread's context.
3045+
var.set(CreatesTaskOnClear())
3046+
3047+
# threading.Thread owns its Context, so use _thread to make the
3048+
# Context lifetime match the thread state's lifetime.
3049+
handle = _thread.start_joinable_thread(create_finalizer)
3050+
handle.join(support.SHORT_TIMEOUT)
3051+
self.assertTrue(handle.is_done(), 'thread failed to exit')
3052+
self.assertIsNotNone(task)
3053+
3054+
try:
3055+
self.assertEqual(self.all_tasks(loop), {task})
3056+
finally:
3057+
loop.run_until_complete(task)
3058+
30243059
def test_del__log_destroy_pending_segfault(self):
30253060
async def coro():
30263061
pass

‎Lib/test/test_embed.py‎

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

403+
def test_asyncio_task_during_interpreter_clear(self):
404+
import_helper.import_module('_asyncio')
405+
code = textwrap.dedent("""
406+
import asyncio
407+
import contextvars
408+
import gc
409+
import os
410+
411+
gc.disable()
412+
413+
async def noop():
414+
pass
415+
416+
class Loop:
417+
def get_debug(self):
418+
return False
419+
420+
def call_soon(self, callback, *args, context=None):
421+
self.callback = callback
422+
423+
class CreatesTaskOnClear:
424+
def __del__(self):
425+
asyncio.Task(noop(), loop=Loop(), context=contextvars.Context())
426+
os.write(1, b'task created\\n')
427+
428+
# Keep the finalizer alive until interpreter_clear()'s last GC.
429+
finalizer = CreatesTaskOnClear()
430+
finalizer.cycle = finalizer
431+
del finalizer
432+
""")
433+
# gh-159041: Delete the cleared interpreter with no thread state attached.
434+
out, err = self.run_embedded_interpreter(
435+
"test_interpreter_clear_and_delete", code,
436+
timeout=support.SHORT_TIMEOUT)
437+
self.assertEqual(out, "task created\n")
438+
self.assertEqual(err, "")
439+
403440
def test_simple_initialization_api(self):
404441
# _testembed now uses Py_InitializeFromConfig by default
405442
# 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
@@ -254,6 +254,23 @@ static int test_repeated_init_exec(void)
254254
return 0;
255255
}
256256

257+
258+
static int test_interpreter_clear_and_delete(void)
259+
{
260+
assert(main_argc == 3);
261+
_testembed_Py_InitializeFromConfig();
262+
if (PyRun_SimpleString(main_argv[2]) < 0) {
263+
return 1;
264+
}
265+
266+
PyInterpreterState *interp = PyInterpreterState_Get();
267+
PyInterpreterState_Clear(interp);
268+
PyThreadState_Swap(NULL);
269+
PyInterpreterState_Delete(interp);
270+
// The interpreter has already been deleted, so don't call Py_FinalizeEx().
271+
return 0;
272+
}
273+
257274
/****************************************************************************
258275
* Test the Py_Initialize(Ex) convenience/compatibility wrappers
259276
***************************************************************************/
@@ -2640,6 +2657,7 @@ struct TestCase
26402657
static struct TestCase TestCases[] = {
26412658
// Python initialization
26422659
{"test_repeated_init_exec", test_repeated_init_exec},
2660+
{"test_interpreter_clear_and_delete", test_interpreter_clear_and_delete},
26432661
{"test_repeated_simple_init", test_repeated_simple_init},
26442662
{"test_forced_io_encoding", test_forced_io_encoding},
26452663
{"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
@@ -1788,13 +1788,6 @@ PyThreadState_Clear(PyThreadState *tstate)
17881788
Py_CLEAR(((_PyThreadStateImpl *)tstate)->asyncio_running_task);
17891789

17901790

1791-
PyMutex_Lock(&tstate->interp->asyncio_tasks_lock);
1792-
// merge any lingering tasks from thread state to interpreter's
1793-
// tasks list
1794-
llist_concat(&tstate->interp->asyncio_tasks_head,
1795-
&((_PyThreadStateImpl *)tstate)->asyncio_tasks_head);
1796-
PyMutex_Unlock(&tstate->interp->asyncio_tasks_lock);
1797-
17981791
Py_CLEAR(tstate->dict);
17991792
Py_CLEAR(tstate->async_exc);
18001793

@@ -1863,11 +1856,30 @@ PyThreadState_Clear(PyThreadState *tstate)
18631856
static void
18641857
decrement_stoptheworld_countdown(struct _stoptheworld_state *stw);
18651858

1866-
/* Common code for PyThreadState_Delete() and PyThreadState_DeleteCurrent() */
1859+
// `release_gil` releases the current thread's GIL. For states already removed
1860+
// by _PyThreadState_RemoveExcept(), `is_unlinked` limits cleanup to task transfer.
18671861
static void
1868-
tstate_delete_common(PyThreadState *tstate, int release_gil)
1862+
tstate_delete_common(PyThreadState *tstate, int release_gil, int is_unlinked)
18691863
{
18701864
assert(tstate->_status.cleared && !tstate->_status.finalized);
1865+
assert(!is_unlinked || !release_gil);
1866+
1867+
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
1868+
// Finalizers may register tasks during or after PyThreadState_Clear().
1869+
// Transfer them before normal unlinking or GIL release: cross-thread task
1870+
// removal uses stop-the-world or the GIL, not asyncio_tasks_lock.
1871+
if (!llist_empty(&tstate_impl->asyncio_tasks_head)) {
1872+
// Serialize transfers from concurrently exiting threads.
1873+
PyMutex_Lock(&tstate->interp->asyncio_tasks_lock);
1874+
llist_concat(&tstate->interp->asyncio_tasks_head,
1875+
&tstate_impl->asyncio_tasks_head);
1876+
PyMutex_Unlock(&tstate->interp->asyncio_tasks_lock);
1877+
}
1878+
assert(llist_empty(&tstate_impl->asyncio_tasks_head));
1879+
if (is_unlinked) {
1880+
return;
1881+
}
1882+
18711883
tstate_verify_not_active(tstate);
18721884
assert(!_PyThreadState_IsRunningMain(tstate));
18731885

@@ -1900,7 +1912,6 @@ tstate_delete_common(PyThreadState *tstate, int release_gil)
19001912

19011913
#if defined(Py_REF_DEBUG) && defined(Py_GIL_DISABLED)
19021914
// Add our portion of the total refcount to the interpreter's total.
1903-
_PyThreadStateImpl *tstate_impl = (_PyThreadStateImpl *)tstate;
19041915
tstate->interp->object_state.reftotal += tstate_impl->reftotal;
19051916
tstate_impl->reftotal = 0;
19061917
assert(tstate_impl->refcounts.values == NULL);
@@ -1943,7 +1954,7 @@ zapthreads(PyInterpreterState *interp)
19431954
*/
19441955
while ((tstate = interp->threads.head) != NULL) {
19451956
tstate_verify_not_active(tstate);
1946-
tstate_delete_common(tstate, 0);
1957+
tstate_delete_common(tstate, /*release_gil=*/0, /*is_unlinked=*/0);
19471958
free_threadstate((_PyThreadStateImpl *)tstate);
19481959
}
19491960
}
@@ -1954,7 +1965,7 @@ PyThreadState_Delete(PyThreadState *tstate)
19541965
{
19551966
_Py_EnsureTstateNotNULL(tstate);
19561967
tstate_verify_not_active(tstate);
1957-
tstate_delete_common(tstate, 0);
1968+
tstate_delete_common(tstate, /*release_gil=*/0, /*is_unlinked=*/0);
19581969
free_threadstate((_PyThreadStateImpl *)tstate);
19591970
}
19601971

@@ -1967,7 +1978,7 @@ _PyThreadState_DeleteCurrent(PyThreadState *tstate)
19671978
_Py_qsbr_detach(((_PyThreadStateImpl *)tstate)->qsbr);
19681979
#endif
19691980
current_fast_clear(tstate->interp->runtime);
1970-
tstate_delete_common(tstate, 1); // release GIL as part of call
1981+
tstate_delete_common(tstate, /*release_gil=*/1, /*is_unlinked=*/0);
19711982
free_threadstate((_PyThreadStateImpl *)tstate);
19721983
}
19731984

@@ -2037,6 +2048,7 @@ _PyThreadState_DeleteList(PyThreadState *list, int is_after_fork)
20372048
for (p = list; p; p = next) {
20382049
next = p->next;
20392050
PyThreadState_Clear(p);
2051+
tstate_delete_common(p, /*release_gil=*/0, /*is_unlinked=*/1);
20402052
if (is_after_fork) {
20412053
free_threadstate((_PyThreadStateImpl *)p);
20422054
}

0 commit comments

Comments
 (0)