[Python-checkins] [3.7] bpo-34872: Fix self-cancellation in C implementation of asyncio.Task (GH-9679) (GH-9691)

[Miss Islington (bot)]

https://github.com/python/cpython/commit/a67bd53d3f80ac9c518b5426aa35459bd373b2b3
commit: a67bd53d3f80ac9c518b5426aa35459bd373b2b3
branch: 3.7
author: Elvis Pranskevichus <elvis at magic.io>
committer: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
date: 2018-10-03T08:49:00-07:00
summary:

[3.7] bpo-34872: Fix self-cancellation in C implementation of asyncio.Task (GH-9679) (GH-9691)



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..
(cherry picked from commit 548ce9dedd2e90945970671d441436a6a91608ab)

Co-authored-by: Elvis Pranskevichus <elvis at magic.io>


https://bugs.python.org/issue34872

files:
A Misc/NEWS.d/next/Library/2018-10-02-19-36-34.bpo-34872.yWZRhI.rst
M Lib/test/test_asyncio/test_tasks.py
M Modules/_asynciomodule.c

diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py
index 67df3c0f48fd..12c6ef4f619e 100644
--- a/Lib/test/test_asyncio/test_tasks.py
+++ b/Lib/test/test_asyncio/test_tasks.py
@@ -590,6 +590,42 @@ def task():
         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 000000000000..cd027102d012
--- /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 0c161fb2bc7d..9f7500a40ad1 100644
--- a/Modules/_asynciomodule.c
+++ b/Modules/_asynciomodule.c
@@ -2662,14 +2662,19 @@ task_step_impl(TaskObj *task, PyObject *exc)
 
             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;