Skip to content

Commit 0ada998

Browse files
authored
gh-157299: Fix missing awaited-by edge in eager tasks (#157300)
1 parent de9a4fb commit 0ada998

6 files changed

Lines changed: 125 additions & 3 deletions

File tree

‎Doc/library/asyncio-task.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,10 @@ Eager task factory
666666
changes to existing applications. For example,
667667
the application's task execution order is likely to change.
668668

669+
During the eager step, call graph introspection reports the creating
670+
task as an awaiter, even for a plain :func:`create_task` call.
671+
This relationship is removed when the eager step finishes.
672+
669673
.. versionadded:: 3.12
670674

671675
.. function:: create_eager_task_factory(custom_task_constructor)

‎Lib/asyncio/tasks.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,9 +257,12 @@ def __eager_start(self):
257257
prev_task = _py_swap_current_task(self._loop, self)
258258
try:
259259
_py_register_eager_task(self)
260+
# gh-157299: the eager step must record the awaited_by edge
261+
futures.future_add_to_awaited_by(self, prev_task)
260262
try:
261263
self._context.run(self.__step_run_and_handle_result, None)
262264
finally:
265+
futures.future_discard_from_awaited_by(self, prev_task)
263266
_py_unregister_eager_task(self)
264267
finally:
265268
try:

‎Lib/test/test_asyncio/test_eager_task_factory.py‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,20 @@ async def run():
6969

7070
self.assertEqual(self.run_coro(run()), 'my message')
7171

72+
def test_awaited_by_during_eager_step(self):
73+
async def run():
74+
parent = asyncio.current_task()
75+
76+
async def coro():
77+
self.assertEqual(asyncio.current_task()._asyncio_awaited_by,
78+
frozenset({parent}))
79+
80+
t = self.loop.create_task(coro())
81+
self.assertFalse(t._asyncio_awaited_by)
82+
await t
83+
84+
self.run_coro(run())
85+
7286
def test_eager_completion(self):
7387

7488
async def coro():
@@ -292,11 +306,29 @@ def setUp(self):
292306
self._current_task = asyncio.current_task
293307
asyncio.current_task = asyncio.tasks.current_task = asyncio.tasks._py_current_task
294308
asyncio.all_tasks = asyncio.tasks.all_tasks = asyncio.tasks._py_all_tasks
309+
310+
futures = asyncio.futures
311+
312+
self._future_add_to_awaited_by = asyncio.future_add_to_awaited_by
313+
futures.future_add_to_awaited_by = futures._py_future_add_to_awaited_by
314+
asyncio.future_add_to_awaited_by = futures.future_add_to_awaited_by
315+
316+
self._future_discard_from_awaited_by = asyncio.future_discard_from_awaited_by
317+
futures.future_discard_from_awaited_by = futures._py_future_discard_from_awaited_by
318+
asyncio.future_discard_from_awaited_by = futures.future_discard_from_awaited_by
295319
return super().setUp()
296320

297321
def tearDown(self):
298322
asyncio.current_task = asyncio.tasks.current_task = self._current_task
299323
asyncio.all_tasks = asyncio.tasks.all_tasks = self._all_tasks
324+
325+
futures = asyncio.futures
326+
327+
futures.future_discard_from_awaited_by = self._future_discard_from_awaited_by
328+
asyncio.future_discard_from_awaited_by = self._future_discard_from_awaited_by
329+
330+
futures.future_add_to_awaited_by = self._future_add_to_awaited_by
331+
asyncio.future_add_to_awaited_by = self._future_add_to_awaited_by
300332
return super().tearDown()
301333

302334

@@ -318,6 +350,49 @@ def tearDown(self):
318350
asyncio.all_tasks = asyncio.tasks.all_tasks = self._all_tasks
319351
return super().tearDown()
320352

353+
def test_eager_step_with_non_native_parent(self):
354+
async def child():
355+
self.assertFalse(asyncio.current_task()._asyncio_awaited_by)
356+
357+
async def run():
358+
parent = tasks._c_swap_current_task(self.loop, object())
359+
try:
360+
task = self.loop.create_task(child())
361+
finally:
362+
tasks._c_swap_current_task(self.loop, parent)
363+
await task
364+
365+
self.run_coro(run())
366+
367+
def test_eager_step_preserves_exception(self):
368+
# gh-157299: check that the exception is saved and cleared before
369+
# discarding the awaited-by edge, and restored afterwards.
370+
# A default task can still pass with the wrong order since its hash
371+
# is in C. Overriding __hash__ makes cleanup call back into Python.
372+
# If the exception is still pending, this raises SystemError.
373+
class ParentTask(self.Task):
374+
def __hash__(self):
375+
return super().__hash__()
376+
377+
child_tasks = []
378+
exc = SystemExit("eager step")
379+
380+
async def child():
381+
task = asyncio.current_task()
382+
child_tasks.append(task)
383+
asyncio.future_add_to_awaited_by(task, self.loop.create_future())
384+
raise exc
385+
386+
async def parent():
387+
with self.assertRaises(SystemExit) as cm:
388+
self.loop.create_task(child())
389+
task, = child_tasks
390+
self.assertIs(cm.exception, exc)
391+
self.assertIs(task.exception(), exc)
392+
self.assertNotIn(asyncio.current_task(), task._asyncio_awaited_by)
393+
394+
self.loop.run_until_complete(ParentTask(parent(), loop=self.loop))
395+
321396
def test_issue105987(self):
322397
code = """if 1:
323398
from _asyncio import _swap_current_task, _set_running_loop

‎Lib/test/test_asyncio/test_graph.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -969,6 +969,23 @@ async def main():
969969
await main()
970970
self.assertEqual(stack[:3], ['gen', 'middle', 'main'])
971971

972+
def set_eager_task_factory(self):
973+
loop = asyncio.get_running_loop()
974+
loop.set_task_factory(asyncio.create_eager_task_factory(asyncio.Task))
975+
self.addCleanup(loop.set_task_factory, None)
976+
977+
async def test_stack_eager_task(self):
978+
self.set_eager_task_factory()
979+
980+
async def child():
981+
nonlocal stack
982+
stack = capture_test_stack()
983+
984+
stack = None
985+
await asyncio.gather(child())
986+
987+
self.assertEqual(stack[0][2], [['T<anon>', ['a test_stack_eager_task'], []]])
988+
972989

973990
@unittest.skipIf(
974991
not hasattr(asyncio.futures, "_c_future_add_to_awaited_by"),
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix eagerly started :class:`asyncio.Task` objects missing their awaited-by
2+
edge during the first step. During that step, even a plain
3+
:func:`asyncio.create_task` call reports the creating task as an awaiter.
4+
Patch by Timofey Ivankov.

‎Modules/_asynciomodule.c‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3491,17 +3491,36 @@ task_eager_start(_PyThreadStateImpl *ts, asyncio_state *state, TaskObj *task)
34913491

34923492
int retval = 0;
34933493

3494+
// gh-157299: the eager step must record the awaited_by edge
3495+
int eager_edge = TaskOrFuture_Check(state, prevtask);
34943496
PyObject *stepres;
34953497
Py_BEGIN_CRITICAL_SECTION(task);
3496-
stepres = task_step_impl(state, task, NULL);
3497-
Py_END_CRITICAL_SECTION();
3498+
if (eager_edge &&
3499+
future_awaited_by_add(state, (FutureObj *)task, prevtask) < 0)
3500+
{
3501+
eager_edge = 0;
3502+
task->task_log_destroy_pending = 0;
3503+
unregister_task(task);
3504+
stepres = NULL;
3505+
}
3506+
else {
3507+
stepres = task_step_impl(state, task, NULL);
3508+
}
3509+
3510+
PyObject *exc = stepres == NULL ? PyErr_GetRaisedException() : NULL;
3511+
if (eager_edge) {
3512+
int res = future_awaited_by_discard(state, (FutureObj *)task, prevtask);
3513+
if (res) {
3514+
retval = -1;
3515+
}
3516+
}
34983517
if (stepres == NULL) {
3499-
PyObject *exc = PyErr_GetRaisedException();
35003518
_PyErr_ChainExceptions1(exc);
35013519
retval = -1;
35023520
} else {
35033521
Py_DECREF(stepres);
35043522
}
3523+
Py_END_CRITICAL_SECTION();
35053524

35063525
PyObject *curtask = swap_current_task(ts, task->task_loop, prevtask);
35073526
Py_DECREF(prevtask);

0 commit comments

Comments
 (0)