Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpo-45390: Propagate CancelledError's message from cancelled task to its awaiter #31383

Merged
merged 7 commits into from
Feb 21, 2022
Merged

bpo-45390: Propagate CancelledError's message from cancelled task to its awaiter #31383

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0b8b4d0
Propagated CancelledError's message from cancelled task to its awaiter
asvetlov Feb 17, 2022
1d43c63
Update Modules/_asynciomodule.c
asvetlov Feb 17, 2022
64e764d
Merge branch 'main' into fix_45390
asvetlov Feb 18, 2022
3c74176
Switch to re-raising stored CancelledError strategy
asvetlov Feb 18, 2022
2cd9c75
Update fail note
asvetlov Feb 21, 2022
0e99dc8
Add a test
asvetlov Feb 21, 2022
b13b39d
Fix notes
asvetlov Feb 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions Doc/library/asyncio-task.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,9 @@ Task Object
.. versionchanged:: 3.9
Added the ``msg`` parameter.

.. versionchanged:: 3.11
The ``msg`` parameter is propagated from cancelled task to its awaiter.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the msg parameter or the exception? Is the latter an implementation detail?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC there are no guarantees about the identity of the exception object.

My personal opinion is that the whole "cancelled message" concept was a mistake, but it's too late to roll it back.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think msg needs to be mentioned in the versionchanged, IMO. The PR is more about eliminating unneeded intermediate CancelledError's, resulting in a more compact traceback, etc. In other words, this PR would be useful even without the msg argument. It's more a side effect that (in most cases), the msg will bubble out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you propose better text, please?
The change is user-faced, the most visible difference is keeping/swallowing the cancellation message on crossing tasks border.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, then I'm okay with what you had.


.. _asyncio_example_task_cancel:

The following example illustrates how coroutines can intercept
Expand Down
6 changes: 5 additions & 1 deletion Lib/asyncio/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,11 @@ def __step(self, exc=None):
except exceptions.CancelledError as exc:
# Save the original exception so we can chain it later.
self._cancelled_exc = exc
super().cancel() # I.e., Future.cancel(self).
args = exc.args
if args:
super().cancel(args[0]) # I.e., Future.cancel(self, msg).
else:
super().cancel() # I.e., Future.cancel(self).
except (KeyboardInterrupt, SystemExit) as exc:
super().set_exception(exc)
raise
Expand Down
16 changes: 10 additions & 6 deletions Lib/test/test_asyncio/test_tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,11 @@ async def coro():
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
Expand All @@ -135,9 +137,11 @@ async def coro():
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):
Expand Down Expand Up @@ -590,7 +594,7 @@ async def coro():
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,
Expand Down Expand Up @@ -624,7 +628,7 @@ async def coro():
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,
Expand All @@ -648,7 +652,7 @@ async def coro():
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,
Expand Down Expand Up @@ -2296,7 +2300,7 @@ async def main():
try:
loop.run_until_complete(main())
except asyncio.CancelledError as exc:
self.assertEqual(exc.args, ())
self.assertEqual(exc.args, expected_args)
exc_type, exc_args, depth = get_innermost_context(exc)
self.assertEqual((exc_type, exc_args),
(asyncio.CancelledError, expected_args))
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2022-02-17-11-00-16.bpo-45390.sVhG6M.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Propagate :exc:`asyncio.CancelledError` message from inner task to outer
awaiter.
13 changes: 12 additions & 1 deletion Modules/_asynciomodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2740,6 +2740,7 @@ task_step_impl(TaskObj *task, PyObject *exc)
}

if (PyErr_ExceptionMatches(asyncio_CancelledError)) {
PyObject *args;
/* CancelledError */
PyErr_Fetch(&et, &ev, &tb);
assert(et);
Expand All @@ -2754,7 +2755,17 @@ task_step_impl(TaskObj *task, PyObject *exc)
_PyErr_StackItem *exc_state = &fut->fut_cancelled_exc_state;
exc_state->exc_value = ev;

return future_cancel(fut, NULL);
args = ((PyBaseExceptionObject*)ev)->args;
if (PyTuple_Size(args) > 0) {
PyObject *msg = PyTuple_GetItem(args, 0);
if (msg == NULL) {
return NULL;
}
return future_cancel(fut, msg);
}
else {
return future_cancel(fut, NULL);
}
asvetlov marked this conversation as resolved.
Show resolved Hide resolved
}

/* Some other exception; pop it and call Task.set_exception() */
Expand Down