From 0c797a6aca1c293e530e18c5e9fa02c670a9a4ed Mon Sep 17 00:00:00 2001 From: Elvis Pranskevichus Date: Wed, 3 Oct 2018 10:30:31 -0400 Subject: [PATCH] bpo-34872: Fix self-cancellation in C implementation of asyncio.Task (GH-9679) The C implementation of asyncio.Task currently fails to perform the cancellation cleanup correctly in the following scenario. async def task1(): async def task2(): await task3 # task3 is never cancelled asyncio.current_task().cancel() await asyncio.create_task(task2()) The actuall error is a hardcoded call to `future_cancel()` instead of calling the `cancel()` method of a future-like object. Thanks to Vladimir Matveev for noticing the code discrepancy and to Yury Selivanov for coming up with a pathological scenario. --- Lib/test/test_asyncio/test_tasks.py | 36 +++++++++++++++++++ .../2018-10-02-19-36-34.bpo-34872.yWZRhI.rst | 1 + Modules/_asynciomodule.c | 11 ++++-- 3 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-10-02-19-36-34.bpo-34872.yWZRhI.rst diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index eb6f2f5ecb1..0fe767630f1 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -622,6 +622,42 @@ class BaseTaskTests: self.assertFalse(t._must_cancel) # White-box test. self.assertFalse(t.cancel()) + def test_cancel_awaited_task(self): + # This tests for a relatively rare condition when + # a task cancellation is requested for a task which is not + # currently blocked, such as a task cancelling itself. + # In this situation we must ensure that whatever next future + # or task the cancelled task blocks on is cancelled correctly + # as well. See also bpo-34872. + loop = asyncio.new_event_loop() + self.addCleanup(lambda: loop.close()) + + task = nested_task = None + fut = self.new_future(loop) + + async def nested(): + await fut + + async def coro(): + nonlocal nested_task + # Create a sub-task and wait for it to run. + nested_task = self.new_task(loop, nested()) + await asyncio.sleep(0) + + # Request the current task to be cancelled. + task.cancel() + # Block on the nested task, which should be immediately + # cancelled. + await nested_task + + task = self.new_task(loop, coro()) + with self.assertRaises(asyncio.CancelledError): + loop.run_until_complete(task) + + self.assertTrue(task.cancelled()) + self.assertTrue(nested_task.cancelled()) + self.assertTrue(fut.cancelled()) + def test_stop_while_run_in_complete(self): def gen(): diff --git a/Misc/NEWS.d/next/Library/2018-10-02-19-36-34.bpo-34872.yWZRhI.rst b/Misc/NEWS.d/next/Library/2018-10-02-19-36-34.bpo-34872.yWZRhI.rst new file mode 100644 index 00000000000..cd027102d01 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-10-02-19-36-34.bpo-34872.yWZRhI.rst @@ -0,0 +1 @@ +Fix self-cancellation in C implementation of asyncio.Task diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 6bf0fd6b0c9..eb503fb38c1 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -2713,14 +2713,19 @@ set_exception: if (task->task_must_cancel) { PyObject *r; - r = future_cancel(fut); + int is_true; + r = _PyObject_CallMethodId(fut, &PyId_cancel, NULL); if (r == NULL) { return NULL; } - if (r == Py_True) { + is_true = PyObject_IsTrue(r); + Py_DECREF(r); + if (is_true < 0) { + return NULL; + } + else if (is_true) { task->task_must_cancel = 0; } - Py_DECREF(r); } Py_RETURN_NONE;