Don't repeat the cancellation message more than once.

Author: cjerdonek

Lib/asyncio/futures.py

@@ -126,6 +126,12 @@ def get_loop(self):
             raise RuntimeError("Future object is not initialized.")
         return loop
 
+    def _make_cancelled_error(self):
+        exc = exceptions.CancelledError(
+            '' if self._cancel_message is None else self._cancel_message)
+        exc.__context__ = self._cancelled_exc
+        return exc
+
     def cancel(self, msg=None):
         """Cancel the future and schedule callbacks.
 
@@ -177,9 +183,7 @@ def result(self):
         the future is done and has an exception set, this exception is raised.
         """
         if self._state == _CANCELLED:
-            exc = exceptions.CancelledError(
-                '' if self._cancel_message is None else self._cancel_message)
-            exc.__context__ = self._cancelled_exc
+            exc = self._make_cancelled_error()
             raise exc
         if self._state != _FINISHED:
             raise exceptions.InvalidStateError('Result is not ready.')
@@ -197,9 +201,7 @@ def exception(self):
         InvalidStateError.
         """
         if self._state == _CANCELLED:
-            exc = exceptions.CancelledError(
-                '' if self._cancel_message is None else self._cancel_message)
-            exc.__context__ = self._cancelled_exc
+            exc = self._make_cancelled_error()
             raise exc
         if self._state != _FINISHED:
             raise exceptions.InvalidStateError('Exception is not set.')

Lib/asyncio/tasks.py

@@ -270,8 +270,7 @@ def __step(self, exc=None):
                 f'_step(): already done: {self!r}, {exc!r}')
         if self._must_cancel:
             if not isinstance(exc, exceptions.CancelledError):
-                exc = exceptions.CancelledError(''
-                    if self._cancel_message is None else self._cancel_message)
+                exc = self._make_cancelled_error()
             self._must_cancel = False
         coro = self._coro
         self._fut_waiter = None
@@ -295,11 +294,7 @@ def __step(self, exc=None):
         except exceptions.CancelledError as exc:
             # Save the original exception so we can chain it later.
             self._cancelled_exc = exc
-            if exc.args:
-                cancel_msg = exc.args[0]
-            else:
-                cancel_msg = None
-            super().cancel(msg=cancel_msg)  # I.e., Future.cancel(self).
+            super().cancel()  # I.e., Future.cancel(self).
         except (KeyboardInterrupt, SystemExit) as exc:
             super().set_exception(exc)
             raise
@@ -789,8 +784,7 @@ def _done_callback(fut):
                 # Check if 'fut' is cancelled first, as
                 # 'fut.exception()' will *raise* a CancelledError
                 # instead of returning it.
-                exc = exceptions.CancelledError(''
-                    if fut._cancel_message is None else fut._cancel_message)
+                exc = fut._make_cancelled_error()
                 outer.set_exception(exc)
                 return
             else:
@@ -809,9 +803,7 @@ def _done_callback(fut):
                     # Check if 'fut' is cancelled first, as
                     # 'fut.exception()' will *raise* a CancelledError
                     # instead of returning it.
-                    res = exceptions.CancelledError(
-                        '' if fut._cancel_message is None else
-                        fut._cancel_message)
+                    res = fut._make_cancelled_error()
                 else:
                     res = fut.exception()
                     if res is None:
@@ -822,8 +814,7 @@ def _done_callback(fut):
                 # If gather is being cancelled we must propagate the
                 # cancellation regardless of *return_exceptions* argument.
                 # See issue 32684.
-                exc = exceptions.CancelledError(''
-                    if fut._cancel_message is None else fut._cancel_message)
+                exc = fut._make_cancelled_error()
                 outer.set_exception(exc)
             else:
                 outer.set_result(results)

Lib/test/test_asyncio/test_tasks.py

@@ -58,6 +58,22 @@ def format_coroutine(qualname, state, src, source_traceback, generator=False):
         return 'coro=<%s() %s at %s>' % (qualname, state, src)
 
 
+def get_innermost_context(exc):
+    """
+    Return information about the innermost exception context in the chain.
+    """
+    depth = 0
+    while True:
+        context = exc.__context__
+        if context is None:
+            break
+
+        exc = context
+        depth += 1
+
+    return (type(exc), exc.args, depth)
+
+
 class Dummy:
 
     def __repr__(self):
@@ -112,9 +128,10 @@ async def coro():
         self.assertEqual(t._cancel_message, None)
 
         t.cancel('my message')
+        self.assertEqual(t._cancel_message, 'my message')
+
         with self.assertRaises(asyncio.CancelledError):
             self.loop.run_until_complete(t)
-        self.assertEqual(t._cancel_message, 'my message')
 
     def test_task_cancel_message_setter(self):
         async def coro():
@@ -124,10 +141,8 @@ async def coro():
         t._cancel_message = 'my new message'
         self.assertEqual(t._cancel_message, 'my new message')
 
-        # Also check that the value is used for cancel().
         with self.assertRaises(asyncio.CancelledError):
             self.loop.run_until_complete(t)
-        self.assertEqual(t._cancel_message, 'my new message')
 
     def test_task_del_collect(self):
         class Evil:
@@ -574,7 +589,11 @@ async def coro():
                 with self.assertRaises(asyncio.CancelledError) as cm:
                     loop.run_until_complete(task)
                 exc = cm.exception
-                self.assertEqual(exc.args, expected_args)
+                self.assertEqual(exc.args, ('',))
+
+                actual = get_innermost_context(exc)
+                self.assertEqual(actual,
+                    (asyncio.CancelledError, expected_args, 2))
 
     def test_cancel_with_message_then_future_exception(self):
         # Test Future.exception() after calling cancel() with a message.
@@ -604,7 +623,11 @@ async def coro():
                 with self.assertRaises(asyncio.CancelledError) as cm:
                     loop.run_until_complete(task)
                 exc = cm.exception
-                self.assertEqual(exc.args, expected_args)
+                self.assertEqual(exc.args, ('',))
+
+                actual = get_innermost_context(exc)
+                self.assertEqual(actual,
+                    (asyncio.CancelledError, expected_args, 2))
 
     def test_cancel_with_message_before_starting_task(self):
         loop = asyncio.new_event_loop()
@@ -624,7 +647,11 @@ async def coro():
         with self.assertRaises(asyncio.CancelledError) as cm:
             loop.run_until_complete(task)
         exc = cm.exception
-        self.assertEqual(exc.args, ('my message',))
+        self.assertEqual(exc.args, ('',))
+
+        actual = get_innermost_context(exc)
+        self.assertEqual(actual,
+            (asyncio.CancelledError, ('my message',), 2))
 
     def test_cancel_yield(self):
         with self.assertWarns(DeprecationWarning):
@@ -2460,7 +2487,6 @@ def test_cancel_gather_2(self):
         ]
         for cancel_args, expected_args in cases:
             with self.subTest(cancel_args=cancel_args):
-
                 loop = asyncio.new_event_loop()
                 self.addCleanup(loop.close)
 
@@ -2478,15 +2504,20 @@ async def main():
                     qwe = self.new_task(loop, test())
                     await asyncio.sleep(0.2)
                     qwe.cancel(*cancel_args)
-                    try:
-                        await qwe
-                    except asyncio.CancelledError as exc:
-                        self.assertEqual(exc.args, expected_args)
-                    else:
-                        self.fail('gather did not propagate the cancellation '
-                                  'request')
-
-                loop.run_until_complete(main())
+                    await qwe
+
+                try:
+                    loop.run_until_complete(main())
+                except asyncio.CancelledError as exc:
+                    self.assertEqual(exc.args, ('',))
+                    exc_type, exc_args, depth = get_innermost_context(exc)
+                    self.assertEqual((exc_type, exc_args),
+                        (asyncio.CancelledError, expected_args))
+                    # The exact traceback seems to vary in CI.
+                    self.assertIn(depth, (2, 3))
+                else:
+                    self.fail('gather did not propagate the cancellation '
+                              'request')
 
     def test_exception_traceback(self):
         # See http://bugs.python.org/issue28843

Modules/_asynciomodule.c

@@ -1347,6 +1347,23 @@ FutureObj_get_state(FutureObj *fut, void *Py_UNUSED(ignored))
     return ret;
 }
 
+/*[clinic input]
+_asyncio.Future._make_cancelled_error
+
+Create the CancelledError to raise if the Future is cancelled.
+[clinic start generated code]*/
+
+static PyObject *
+_asyncio_Future__make_cancelled_error_impl(FutureObj *self)
+/*[clinic end generated code: output=a5df276f6c1213de input=374e7bf8dd6edc60]*/
+{
+    PyObject *exc = create_cancelled_error(self->fut_cancel_msg);
+    _PyErr_StackItem *exc_state = &self->fut_cancelled_exc_state;
+    Py_XINCREF(exc_state->exc_value);
+    PyException_SetContext(exc, exc_state->exc_value);
+    return exc;
+}
+
 /*[clinic input]
 _asyncio.Future._repr_info
 [clinic start generated code]*/
@@ -1473,6 +1490,7 @@ static PyMethodDef FutureType_methods[] = {
     _ASYNCIO_FUTURE_CANCELLED_METHODDEF
     _ASYNCIO_FUTURE_DONE_METHODDEF
     _ASYNCIO_FUTURE_GET_LOOP_METHODDEF
+    _ASYNCIO_FUTURE__MAKE_CANCELLED_ERROR_METHODDEF
     _ASYNCIO_FUTURE__REPR_INFO_METHODDEF
     {"__class_getitem__", future_cls_getitem, METH_O|METH_CLASS, NULL},
     {NULL, NULL}        /* Sentinel */
@@ -2244,6 +2262,21 @@ _asyncio_Task_all_tasks_impl(PyTypeObject *type, PyObject *loop)
     return res;
 }
 
+/*[clinic input]
+_asyncio.Task._make_cancelled_error
+
+Create the CancelledError to raise if the Task is cancelled.
+[clinic start generated code]*/
+
+static PyObject *
+_asyncio_Task__make_cancelled_error_impl(TaskObj *self)
+/*[clinic end generated code: output=55a819e8b4276fab input=fc5485bb07d5c36b]*/
+{
+    FutureObj *fut = (FutureObj*)self;
+    return _asyncio_Future__make_cancelled_error_impl(fut);
+}
+
+
 /*[clinic input]
 _asyncio.Task._repr_info
 [clinic start generated code]*/
@@ -2551,6 +2584,7 @@ static PyMethodDef TaskType_methods[] = {
     _ASYNCIO_TASK_CANCEL_METHODDEF
     _ASYNCIO_TASK_GET_STACK_METHODDEF
     _ASYNCIO_TASK_PRINT_STACK_METHODDEF
+    _ASYNCIO_TASK__MAKE_CANCELLED_ERROR_METHODDEF
     _ASYNCIO_TASK__REPR_INFO_METHODDEF
     _ASYNCIO_TASK_GET_NAME_METHODDEF
     _ASYNCIO_TASK_SET_NAME_METHODDEF
@@ -2766,26 +2800,13 @@ task_step_impl(TaskObj *task, PyObject *exc)
             /* CancelledError */
             PyErr_Fetch(&et, &ev, &tb);
 
-            PyObject *cancel_msg;
-            if (ev != NULL && PyExceptionInstance_Check(ev)) {
-                PyObject *exc_args = ((PyBaseExceptionObject*)ev)->args;
-                Py_ssize_t size = PyTuple_GET_SIZE(exc_args);
-                if (size > 0) {
-                    cancel_msg = PyTuple_GET_ITEM(exc_args, 0);
-                } else {
-                    cancel_msg = NULL;
-                }
-            } else {
-                cancel_msg = ev;
-            }
-
             FutureObj *fut = (FutureObj*)task;
             _PyErr_StackItem *exc_state = &fut->fut_cancelled_exc_state;
             exc_state->exc_type = et;
             exc_state->exc_value = ev;
             exc_state->exc_traceback = tb;
 
-            return future_cancel(fut, cancel_msg);
+            return future_cancel(fut, NULL);
         }
 
         /* Some other exception; pop it and call Task.set_exception() */