From 4140bcb1cd76dec5cf8d398f4d0e86c438c987d0 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 21 Feb 2022 22:59:04 +0200 Subject: bpo-45390: Propagate CancelledError's message from cancelled task to its awaiter (GH-31383) Co-authored-by: Serhiy Storchaka --- Doc/library/asyncio-task.rst | 3 + Lib/asyncio/futures.py | 5 ++ Lib/test/test_asyncio/test_tasks.py | 66 ++++++++++++++++------ .../2022-02-17-11-00-16.bpo-45390.sVhG6M.rst | 2 + Modules/_asynciomodule.c | 42 +++++++------- 5 files changed, 79 insertions(+), 39 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-02-17-11-00-16.bpo-45390.sVhG6M.rst diff --git a/Doc/library/asyncio-task.rst b/Doc/library/asyncio-task.rst index 731d155..b30b289 100644 --- a/Doc/library/asyncio-task.rst +++ b/Doc/library/asyncio-task.rst @@ -843,6 +843,9 @@ Task Object .. versionchanged:: 3.9 Added the *msg* parameter. + .. versionchanged:: 3.11 + The ``msg`` parameter is propagated from cancelled task to its awaiter. + .. _asyncio_example_task_cancel: The following example illustrates how coroutines can intercept diff --git a/Lib/asyncio/futures.py b/Lib/asyncio/futures.py index 8e8cd87..5a92f17 100644 --- a/Lib/asyncio/futures.py +++ b/Lib/asyncio/futures.py @@ -132,6 +132,11 @@ class Future: This should only be called once when handling a cancellation since it erases the saved context exception value. """ + if self._cancelled_exc is not None: + exc = self._cancelled_exc + self._cancelled_exc = None + return exc + if self._cancel_message is None: exc = exceptions.CancelledError() else: diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 40b33de..b30f8f5 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -124,9 +124,11 @@ class BaseTaskTests: t.cancel('my message') self.assertEqual(t._cancel_message, 'my message') - with self.assertRaises(asyncio.CancelledError): + with self.assertRaises(asyncio.CancelledError) as cm: self.loop.run_until_complete(t) + self.assertEqual('my message', cm.exception.args[0]) + def test_task_cancel_message_setter(self): async def coro(): pass @@ -135,9 +137,11 @@ class BaseTaskTests: t._cancel_message = 'my new message' self.assertEqual(t._cancel_message, 'my new message') - with self.assertRaises(asyncio.CancelledError): + with self.assertRaises(asyncio.CancelledError) as cm: self.loop.run_until_complete(t) + self.assertEqual('my new message', cm.exception.args[0]) + def test_task_del_collect(self): class Evil: def __del__(self): @@ -590,11 +594,11 @@ class BaseTaskTests: with self.assertRaises(asyncio.CancelledError) as cm: loop.run_until_complete(task) exc = cm.exception - self.assertEqual(exc.args, ()) + self.assertEqual(exc.args, expected_args) actual = get_innermost_context(exc) self.assertEqual(actual, - (asyncio.CancelledError, expected_args, 2)) + (asyncio.CancelledError, expected_args, 0)) def test_cancel_with_message_then_future_exception(self): # Test Future.exception() after calling cancel() with a message. @@ -624,11 +628,39 @@ class BaseTaskTests: with self.assertRaises(asyncio.CancelledError) as cm: loop.run_until_complete(task) exc = cm.exception - self.assertEqual(exc.args, ()) + self.assertEqual(exc.args, expected_args) actual = get_innermost_context(exc) self.assertEqual(actual, - (asyncio.CancelledError, expected_args, 2)) + (asyncio.CancelledError, expected_args, 0)) + + def test_cancellation_exception_context(self): + loop = asyncio.new_event_loop() + self.set_event_loop(loop) + fut = loop.create_future() + + async def sleep(): + fut.set_result(None) + await asyncio.sleep(10) + + async def coro(): + inner_task = self.new_task(loop, sleep()) + await fut + loop.call_soon(inner_task.cancel, 'msg') + try: + await inner_task + except asyncio.CancelledError as ex: + raise ValueError("cancelled") from ex + + task = self.new_task(loop, coro()) + with self.assertRaises(ValueError) as cm: + loop.run_until_complete(task) + exc = cm.exception + self.assertEqual(exc.args, ('cancelled',)) + + actual = get_innermost_context(exc) + self.assertEqual(actual, + (asyncio.CancelledError, ('msg',), 1)) def test_cancel_with_message_before_starting_task(self): loop = asyncio.new_event_loop() @@ -648,11 +680,11 @@ class BaseTaskTests: with self.assertRaises(asyncio.CancelledError) as cm: loop.run_until_complete(task) exc = cm.exception - self.assertEqual(exc.args, ()) + self.assertEqual(exc.args, ('my message',)) actual = get_innermost_context(exc) self.assertEqual(actual, - (asyncio.CancelledError, ('my message',), 2)) + (asyncio.CancelledError, ('my message',), 0)) def test_cancel_yield(self): async def task(): @@ -2296,15 +2328,17 @@ class BaseTaskTests: 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)) + self.assertEqual(exc.args, expected_args) + actual = get_innermost_context(exc) + self.assertEqual( + actual, + (asyncio.CancelledError, expected_args, 0), + ) else: - self.fail('gather did not propagate the cancellation ' - 'request') + self.fail( + 'gather() does not propagate CancelledError ' + 'raised by inner task to the gather() caller.' + ) def test_exception_traceback(self): # See http://bugs.python.org/issue28843 diff --git a/Misc/NEWS.d/next/Library/2022-02-17-11-00-16.bpo-45390.sVhG6M.rst b/Misc/NEWS.d/next/Library/2022-02-17-11-00-16.bpo-45390.sVhG6M.rst new file mode 100644 index 0000000..5f1eafa --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-02-17-11-00-16.bpo-45390.sVhG6M.rst @@ -0,0 +1,2 @@ +Propagate :exc:`asyncio.CancelledError` message from inner task to outer +awaiter. diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 6725e2e..0d6b21d 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -77,7 +77,7 @@ typedef enum { int prefix##_blocking; \ PyObject *dict; \ PyObject *prefix##_weakreflist; \ - _PyErr_StackItem prefix##_cancelled_exc_state; + PyObject *prefix##_cancelled_exc; typedef struct { FutureObj_HEAD(fut) @@ -496,7 +496,7 @@ future_init(FutureObj *fut, PyObject *loop) Py_CLEAR(fut->fut_exception); Py_CLEAR(fut->fut_source_tb); Py_CLEAR(fut->fut_cancel_msg); - _PyErr_ClearExcState(&fut->fut_cancelled_exc_state); + Py_CLEAR(fut->fut_cancelled_exc); fut->fut_state = STATE_PENDING; fut->fut_log_tb = 0; @@ -612,25 +612,32 @@ future_set_exception(FutureObj *fut, PyObject *exc) } static PyObject * -create_cancelled_error(PyObject *msg) +create_cancelled_error(FutureObj *fut) { PyObject *exc; + if (fut->fut_cancelled_exc != NULL) { + /* transfer ownership */ + exc = fut->fut_cancelled_exc; + fut->fut_cancelled_exc = NULL; + return exc; + } + PyObject *msg = fut->fut_cancel_msg; if (msg == NULL || msg == Py_None) { exc = PyObject_CallNoArgs(asyncio_CancelledError); } else { exc = PyObject_CallOneArg(asyncio_CancelledError, msg); } + PyException_SetContext(exc, fut->fut_cancelled_exc); + Py_CLEAR(fut->fut_cancelled_exc); return exc; } static void future_set_cancelled_error(FutureObj *fut) { - PyObject *exc = create_cancelled_error(fut->fut_cancel_msg); + PyObject *exc = create_cancelled_error(fut); PyErr_SetObject(asyncio_CancelledError, exc); Py_DECREF(exc); - - _PyErr_ChainStackItem(&fut->fut_cancelled_exc_state); } static int @@ -793,7 +800,7 @@ FutureObj_clear(FutureObj *fut) Py_CLEAR(fut->fut_exception); Py_CLEAR(fut->fut_source_tb); Py_CLEAR(fut->fut_cancel_msg); - _PyErr_ClearExcState(&fut->fut_cancelled_exc_state); + Py_CLEAR(fut->fut_cancelled_exc); Py_CLEAR(fut->dict); return 0; } @@ -809,11 +816,8 @@ FutureObj_traverse(FutureObj *fut, visitproc visit, void *arg) Py_VISIT(fut->fut_exception); Py_VISIT(fut->fut_source_tb); Py_VISIT(fut->fut_cancel_msg); + Py_VISIT(fut->fut_cancelled_exc); Py_VISIT(fut->dict); - - _PyErr_StackItem *exc_state = &fut->fut_cancelled_exc_state; - Py_VISIT(exc_state->exc_value); - return 0; } @@ -1369,15 +1373,7 @@ static PyObject * _asyncio_Future__make_cancelled_error_impl(FutureObj *self) /*[clinic end generated code: output=a5df276f6c1213de input=ac6effe4ba795ecc]*/ { - PyObject *exc = create_cancelled_error(self->fut_cancel_msg); - _PyErr_StackItem *exc_state = &self->fut_cancelled_exc_state; - - if (exc_state->exc_value) { - PyException_SetContext(exc, Py_NewRef(exc_state->exc_value)); - _PyErr_ClearExcState(exc_state); - } - - return exc; + return create_cancelled_error(self); } /*[clinic input] @@ -2677,7 +2673,7 @@ task_step_impl(TaskObj *task, PyObject *exc) if (!exc) { /* exc was not a CancelledError */ - exc = create_cancelled_error(task->task_cancel_msg); + exc = create_cancelled_error((FutureObj*)task); if (!exc) { goto fail; @@ -2751,8 +2747,8 @@ task_step_impl(TaskObj *task, PyObject *exc) Py_XDECREF(et); FutureObj *fut = (FutureObj*)task; - _PyErr_StackItem *exc_state = &fut->fut_cancelled_exc_state; - exc_state->exc_value = ev; + /* transfer ownership */ + fut->fut_cancelled_exc = ev; return future_cancel(fut, NULL); } -- cgit v0.12