diff options
author | Elvis Pranskevichus <elvis@magic.io> | 2018-10-03 14:30:31 (GMT) |
---|---|---|
committer | Yury Selivanov <yury@magic.io> | 2018-10-03 14:30:31 (GMT) |
commit | 0c797a6aca1c293e530e18c5e9fa02c670a9a4ed (patch) | |
tree | 0a7627593801b009925f9909715b8cacabf6c91e | |
parent | 96c593279400693226d5a560c420ae0fcf1731b9 (diff) | |
download | cpython-0c797a6aca1c293e530e18c5e9fa02c670a9a4ed.zip cpython-0c797a6aca1c293e530e18c5e9fa02c670a9a4ed.tar.gz cpython-0c797a6aca1c293e530e18c5e9fa02c670a9a4ed.tar.bz2 |
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.
-rw-r--r-- | Lib/test/test_asyncio/test_tasks.py | 36 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Library/2018-10-02-19-36-34.bpo-34872.yWZRhI.rst | 1 | ||||
-rw-r--r-- | Modules/_asynciomodule.c | 11 |
3 files changed, 45 insertions, 3 deletions
diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index eb6f2f5..0fe7676 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 0000000..cd02710 --- /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 6bf0fd6..eb503fb 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; |