From 0ed5e4eb5ba8aa504ae7b5962d4503b6106bf654 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Wed, 6 May 2020 18:11:15 -0700 Subject: [PATCH 1/8] Add argument clinic changes. --- Modules/clinic/_asynciomodule.c.h | 60 +++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/Modules/clinic/_asynciomodule.c.h b/Modules/clinic/_asynciomodule.c.h index 17eb77334d0a73..3f5023c33a5803 100644 --- a/Modules/clinic/_asynciomodule.c.h +++ b/Modules/clinic/_asynciomodule.c.h @@ -174,7 +174,7 @@ PyDoc_STRVAR(_asyncio_Future_remove_done_callback__doc__, {"remove_done_callback", (PyCFunction)_asyncio_Future_remove_done_callback, METH_O, _asyncio_Future_remove_done_callback__doc__}, PyDoc_STRVAR(_asyncio_Future_cancel__doc__, -"cancel($self, /)\n" +"cancel($self, /, msg=None)\n" "--\n" "\n" "Cancel the future and schedule callbacks.\n" @@ -184,15 +184,34 @@ PyDoc_STRVAR(_asyncio_Future_cancel__doc__, "return True."); #define _ASYNCIO_FUTURE_CANCEL_METHODDEF \ - {"cancel", (PyCFunction)_asyncio_Future_cancel, METH_NOARGS, _asyncio_Future_cancel__doc__}, + {"cancel", (PyCFunction)(void(*)(void))_asyncio_Future_cancel, METH_FASTCALL|METH_KEYWORDS, _asyncio_Future_cancel__doc__}, static PyObject * -_asyncio_Future_cancel_impl(FutureObj *self); +_asyncio_Future_cancel_impl(FutureObj *self, PyObject *msg); static PyObject * -_asyncio_Future_cancel(FutureObj *self, PyObject *Py_UNUSED(ignored)) +_asyncio_Future_cancel(FutureObj *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { - return _asyncio_Future_cancel_impl(self); + PyObject *return_value = NULL; + static const char * const _keywords[] = {"msg", NULL}; + static _PyArg_Parser _parser = {NULL, _keywords, "cancel", 0}; + PyObject *argsbuf[1]; + Py_ssize_t noptargs = nargs + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 0; + PyObject *msg = Py_None; + + args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 0, 1, 0, argsbuf); + if (!args) { + goto exit; + } + if (!noptargs) { + goto skip_optional_pos; + } + msg = args[0]; +skip_optional_pos: + return_value = _asyncio_Future_cancel_impl(self, msg); + +exit: + return return_value; } PyDoc_STRVAR(_asyncio_Future_cancelled__doc__, @@ -413,7 +432,7 @@ _asyncio_Task__repr_info(TaskObj *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(_asyncio_Task_cancel__doc__, -"cancel($self, /)\n" +"cancel($self, /, msg=None)\n" "--\n" "\n" "Request that this task cancel itself.\n" @@ -436,15 +455,34 @@ PyDoc_STRVAR(_asyncio_Task_cancel__doc__, "was not called)."); #define _ASYNCIO_TASK_CANCEL_METHODDEF \ - {"cancel", (PyCFunction)_asyncio_Task_cancel, METH_NOARGS, _asyncio_Task_cancel__doc__}, + {"cancel", (PyCFunction)(void(*)(void))_asyncio_Task_cancel, METH_FASTCALL|METH_KEYWORDS, _asyncio_Task_cancel__doc__}, static PyObject * -_asyncio_Task_cancel_impl(TaskObj *self); +_asyncio_Task_cancel_impl(TaskObj *self, PyObject *msg); static PyObject * -_asyncio_Task_cancel(TaskObj *self, PyObject *Py_UNUSED(ignored)) +_asyncio_Task_cancel(TaskObj *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { - return _asyncio_Task_cancel_impl(self); + PyObject *return_value = NULL; + static const char * const _keywords[] = {"msg", NULL}; + static _PyArg_Parser _parser = {NULL, _keywords, "cancel", 0}; + PyObject *argsbuf[1]; + Py_ssize_t noptargs = nargs + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 0; + PyObject *msg = Py_None; + + args = _PyArg_UnpackKeywords(args, nargs, NULL, kwnames, &_parser, 0, 1, 0, argsbuf); + if (!args) { + goto exit; + } + if (!noptargs) { + goto skip_optional_pos; + } + msg = args[0]; +skip_optional_pos: + return_value = _asyncio_Task_cancel_impl(self, msg); + +exit: + return return_value; } PyDoc_STRVAR(_asyncio_Task_get_stack__doc__, @@ -832,4 +870,4 @@ _asyncio__leave_task(PyObject *module, PyObject *const *args, Py_ssize_t nargs, exit: return return_value; } -/*[clinic end generated code: output=585ba1f8de5b4103 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=6ed4cfda8fc516ad input=a9049054013a1b77]*/ From 5c8489f337225a066b057b18ce9b23b6c18d29aa Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Thu, 7 May 2020 02:18:14 -0700 Subject: [PATCH 2/8] Add failing tests. --- Lib/test/test_asyncio/test_tasks.py | 141 +++++++++++++++++++++++----- 1 file changed, 116 insertions(+), 25 deletions(-) diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 6eb6b46ec8af75..7ab42956e249d0 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -514,6 +514,86 @@ async def task(): self.assertTrue(t.cancelled()) self.assertFalse(t.cancel()) + def test_cancel_with_message_then_future_result(self): + # Test Future.result() after calling cancel() with a message. + cases = [ + ((), (None,)), + ((None,), (None,)), + (('my message',), ('my message',)), + # Non-string values should roundtrip. + ((5,), (5,)), + ] + for cancel_args, expected_args in cases: + with self.subTest(cancel_args=cancel_args): + loop = asyncio.new_event_loop() + self.set_event_loop(loop) + + async def sleep(): + await asyncio.sleep(10) + + async def coro(): + task = self.new_task(loop, sleep()) + await asyncio.sleep(0) + task.cancel(*cancel_args) + done, pending = await asyncio.wait([task]) + task.result() + + task = self.new_task(loop, coro()) + with self.assertRaises(asyncio.CancelledError) as cm: + loop.run_until_complete(task) + exc = cm.exception + self.assertEqual(exc.args, expected_args) + + def test_cancel_with_message_then_future_exception(self): + # Test Future.exception() after calling cancel() with a message. + cases = [ + ((), (None,)), + ((None,), (None,)), + (('my message',), ('my message',)), + # Non-string values should roundtrip. + ((5,), (5,)), + ] + for cancel_args, expected_args in cases: + with self.subTest(cancel_args=cancel_args): + loop = asyncio.new_event_loop() + self.set_event_loop(loop) + + async def sleep(): + await asyncio.sleep(10) + + async def coro(): + task = self.new_task(loop, sleep()) + await asyncio.sleep(0) + task.cancel(*cancel_args) + done, pending = await asyncio.wait([task]) + task.exception() + + task = self.new_task(loop, coro()) + with self.assertRaises(asyncio.CancelledError) as cm: + loop.run_until_complete(task) + exc = cm.exception + self.assertEqual(exc.args, expected_args) + + def test_cancel_with_message_before_starting_task(self): + loop = asyncio.new_event_loop() + self.set_event_loop(loop) + + async def sleep(): + await asyncio.sleep(10) + + async def coro(): + task = self.new_task(loop, sleep()) + # We deliberately leave out the sleep here. + task.cancel('my message') + done, pending = await asyncio.wait([task]) + task.exception() + + task = self.new_task(loop, coro()) + with self.assertRaises(asyncio.CancelledError) as cm: + loop.run_until_complete(task) + exc = cm.exception + self.assertEqual(exc.args, ('my message',)) + def test_cancel_yield(self): with self.assertWarns(DeprecationWarning): @asyncio.coroutine @@ -2238,31 +2318,42 @@ def cancelling_callback(_): self.assertEqual(gather_task.result(), [42]) def test_cancel_gather_2(self): - loop = asyncio.new_event_loop() - self.addCleanup(loop.close) - - async def test(): - time = 0 - while True: - time += 0.05 - await asyncio.gather(asyncio.sleep(0.05), - return_exceptions=True, - loop=loop) - if time > 1: - return - - async def main(): - qwe = self.new_task(loop, test()) - await asyncio.sleep(0.2) - qwe.cancel() - try: - await qwe - except asyncio.CancelledError: - pass - else: - self.fail('gather did not propagate the cancellation request') - - loop.run_until_complete(main()) + cases = [ + ((), (None,)), + ((None,), (None,)), + (('my message',), ('my message',)), + # Non-string values should roundtrip. + ((5,), (5,)), + ] + for cancel_args, expected_args in cases: + with self.subTest(cancel_args=cancel_args): + + loop = asyncio.new_event_loop() + self.addCleanup(loop.close) + + async def test(): + time = 0 + while True: + time += 0.05 + await asyncio.gather(asyncio.sleep(0.05), + return_exceptions=True, + loop=loop) + if time > 1: + return + + 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()) def test_exception_traceback(self): # See http://bugs.python.org/issue28843 From 58a2257bd925b858afa00047fd478780d6f2cbea Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Wed, 6 May 2020 18:30:33 -0700 Subject: [PATCH 3/8] Add C implementation. --- Modules/_asynciomodule.c | 112 +++++++++++++++++++++++++++++++++------ 1 file changed, 96 insertions(+), 16 deletions(-) diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index cc211a8895a8e8..5097f543c290d1 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -67,6 +67,7 @@ typedef enum { PyObject *prefix##_exception; \ PyObject *prefix##_result; \ PyObject *prefix##_source_tb; \ + PyObject *prefix##_cancel_msg; \ fut_state prefix##_state; \ int prefix##_log_tb; \ int prefix##_blocking; \ @@ -480,6 +481,7 @@ future_init(FutureObj *fut, PyObject *loop) Py_CLEAR(fut->fut_result); Py_CLEAR(fut->fut_exception); Py_CLEAR(fut->fut_source_tb); + Py_CLEAR(fut->fut_cancel_msg); fut->fut_state = STATE_PENDING; fut->fut_log_tb = 0; @@ -594,11 +596,30 @@ future_set_exception(FutureObj *fut, PyObject *exc) Py_RETURN_NONE; } +static PyObject * +create_cancelled_error(PyObject *msg) +{ + if (msg == NULL) { + msg = Py_None; + } + PyObject *exc = PyObject_CallOneArg(asyncio_CancelledError, msg); + + return exc; +} + +static void +set_cancelled_error(PyObject *msg) +{ + PyObject *exc = create_cancelled_error(msg); + PyErr_SetObject(asyncio_CancelledError, exc); + Py_DECREF(exc); +} + static int future_get_result(FutureObj *fut, PyObject **result) { if (fut->fut_state == STATE_CANCELLED) { - PyErr_SetNone(asyncio_CancelledError); + set_cancelled_error(fut->fut_cancel_msg); return -1; } @@ -695,7 +716,7 @@ future_add_done_callback(FutureObj *fut, PyObject *arg, PyObject *ctx) } static PyObject * -future_cancel(FutureObj *fut) +future_cancel(FutureObj *fut, PyObject *msg) { fut->fut_log_tb = 0; @@ -704,6 +725,9 @@ future_cancel(FutureObj *fut) } fut->fut_state = STATE_CANCELLED; + Py_XINCREF(msg); + Py_XSETREF(fut->fut_cancel_msg, msg); + if (future_schedule_callbacks(fut) == -1) { return NULL; } @@ -749,6 +773,7 @@ FutureObj_clear(FutureObj *fut) Py_CLEAR(fut->fut_result); Py_CLEAR(fut->fut_exception); Py_CLEAR(fut->fut_source_tb); + Py_CLEAR(fut->fut_cancel_msg); Py_CLEAR(fut->dict); return 0; } @@ -763,6 +788,7 @@ FutureObj_traverse(FutureObj *fut, visitproc visit, void *arg) Py_VISIT(fut->fut_result); Py_VISIT(fut->fut_exception); Py_VISIT(fut->fut_source_tb); + Py_VISIT(fut->fut_cancel_msg); Py_VISIT(fut->dict); return 0; } @@ -828,7 +854,7 @@ _asyncio_Future_exception_impl(FutureObj *self) } if (self->fut_state == STATE_CANCELLED) { - PyErr_SetNone(asyncio_CancelledError); + set_cancelled_error(self->fut_cancel_msg); return NULL; } @@ -1029,6 +1055,8 @@ _asyncio_Future_remove_done_callback(FutureObj *self, PyObject *fn) /*[clinic input] _asyncio.Future.cancel + msg: object = None + Cancel the future and schedule callbacks. If the future is already done or cancelled, return False. Otherwise, @@ -1037,11 +1065,11 @@ return True. [clinic start generated code]*/ static PyObject * -_asyncio_Future_cancel_impl(FutureObj *self) -/*[clinic end generated code: output=e45b932ba8bd68a1 input=515709a127995109]*/ +_asyncio_Future_cancel_impl(FutureObj *self, PyObject *msg) +/*[clinic end generated code: output=3edebbc668e5aba3 input=925eb545251f2c5a]*/ { ENSURE_FUTURE_ALIVE(self) - return future_cancel(self); + return future_cancel(self, msg); } /*[clinic input] @@ -1254,6 +1282,29 @@ FutureObj_get_source_traceback(FutureObj *fut, void *Py_UNUSED(ignored)) return fut->fut_source_tb; } +static PyObject * +FutureObj_get_cancel_message(FutureObj *fut, void *Py_UNUSED(ignored)) +{ + if (fut->fut_cancel_msg == NULL) { + Py_RETURN_NONE; + } + Py_INCREF(fut->fut_cancel_msg); + return fut->fut_cancel_msg; +} + +static int +FutureObj_set_cancel_message(FutureObj *fut, PyObject *msg, + void *Py_UNUSED(ignored)) +{ + if (msg == NULL) { + PyErr_SetString(PyExc_AttributeError, "cannot delete attribute"); + return -1; + } + Py_INCREF(msg); + Py_XSETREF(fut->fut_cancel_msg, msg); + return 0; +} + static PyObject * FutureObj_get_state(FutureObj *fut, void *Py_UNUSED(ignored)) { @@ -1422,7 +1473,10 @@ static PyMethodDef FutureType_methods[] = { {"_exception", (getter)FutureObj_get_exception, NULL, NULL}, \ {"_log_traceback", (getter)FutureObj_get_log_traceback, \ (setter)FutureObj_set_log_traceback, NULL}, \ - {"_source_traceback", (getter)FutureObj_get_source_traceback, NULL, NULL}, + {"_source_traceback", (getter)FutureObj_get_source_traceback, \ + NULL, NULL}, \ + {"_cancel_message", (getter)FutureObj_get_cancel_message, \ + (setter)FutureObj_set_cancel_message, NULL}, static PyGetSetDef FutureType_getsetlist[] = { FUTURE_COMMON_GETSETLIST @@ -2189,6 +2243,8 @@ _asyncio_Task__repr_info_impl(TaskObj *self) /*[clinic input] _asyncio.Task.cancel + msg: object = None + Request that this task cancel itself. This arranges for a CancelledError to be thrown into the @@ -2210,8 +2266,8 @@ was not called). [clinic start generated code]*/ static PyObject * -_asyncio_Task_cancel_impl(TaskObj *self) -/*[clinic end generated code: output=6bfc0479da9d5757 input=13f9bf496695cb52]*/ +_asyncio_Task_cancel_impl(TaskObj *self, PyObject *msg) +/*[clinic end generated code: output=c66b60d41c74f9f1 input=f4ff8e8ffc5f1c00]*/ { self->task_log_tb = 0; @@ -2223,7 +2279,8 @@ _asyncio_Task_cancel_impl(TaskObj *self) PyObject *res; int is_true; - res = _PyObject_CallMethodIdNoArgs(self->task_fut_waiter, &PyId_cancel); + res = _PyObject_CallMethodIdOneArg(self->task_fut_waiter, + &PyId_cancel, msg); if (res == NULL) { return NULL; } @@ -2240,6 +2297,8 @@ _asyncio_Task_cancel_impl(TaskObj *self) } self->task_must_cancel = 1; + Py_XINCREF(msg); + Py_XSETREF(self->task_cancel_msg, msg); Py_RETURN_TRUE; } @@ -2623,7 +2682,8 @@ task_step_impl(TaskObj *task, PyObject *exc) if (!exc) { /* exc was not a CancelledError */ - exc = PyObject_CallNoArgs(asyncio_CancelledError); + exc = create_cancelled_error(task->task_cancel_msg); + if (!exc) { goto fail; } @@ -2672,7 +2732,7 @@ task_step_impl(TaskObj *task, PyObject *exc) if (task->task_must_cancel) { // Task is cancelled right before coro stops. task->task_must_cancel = 0; - res = future_cancel((FutureObj*)task); + res = future_cancel((FutureObj*)task, task->task_cancel_msg); } else { res = future_set_result((FutureObj*)task, o); @@ -2689,8 +2749,26 @@ task_step_impl(TaskObj *task, PyObject *exc) if (PyErr_ExceptionMatches(asyncio_CancelledError)) { /* CancelledError */ - PyErr_Clear(); - return future_cancel((FutureObj*)task); + 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; + } + + Py_DECREF(et); + Py_XDECREF(tb); + Py_XDECREF(ev); + + return future_cancel((FutureObj*)task, cancel_msg); } /* Some other exception; pop it and call Task.set_exception() */ @@ -2770,7 +2848,8 @@ task_step_impl(TaskObj *task, PyObject *exc) if (task->task_must_cancel) { PyObject *r; int is_true; - r = _PyObject_CallMethodIdNoArgs(result, &PyId_cancel); + r = _PyObject_CallMethodIdOneArg(result, &PyId_cancel, + task->task_cancel_msg); if (r == NULL) { return NULL; } @@ -2861,7 +2940,8 @@ task_step_impl(TaskObj *task, PyObject *exc) if (task->task_must_cancel) { PyObject *r; int is_true; - r = _PyObject_CallMethodIdNoArgs(result, &PyId_cancel); + r = _PyObject_CallMethodIdOneArg(result, &PyId_cancel, + task->task_cancel_msg); if (r == NULL) { return NULL; } From 86e13df23403fc5cefbd36a8f39a9a32615f4d68 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Wed, 6 May 2020 18:26:13 -0700 Subject: [PATCH 4/8] Add pure Python implementation. --- Lib/asyncio/futures.py | 8 +++++--- Lib/asyncio/tasks.py | 31 +++++++++++++++++++------------ Lib/asyncio/windows_events.py | 8 ++++---- 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/Lib/asyncio/futures.py b/Lib/asyncio/futures.py index a3cf379ee81705..536d652dda8a9a 100644 --- a/Lib/asyncio/futures.py +++ b/Lib/asyncio/futures.py @@ -51,6 +51,7 @@ class Future: _exception = None _loop = None _source_traceback = None + _cancel_message = None # This field is used for a dual purpose: # - Its presence is a marker to declare that a class implements @@ -123,7 +124,7 @@ def get_loop(self): raise RuntimeError("Future object is not initialized.") return loop - def cancel(self): + def cancel(self, msg=None): """Cancel the future and schedule callbacks. If the future is already done or cancelled, return False. Otherwise, @@ -134,6 +135,7 @@ def cancel(self): if self._state != _PENDING: return False self._state = _CANCELLED + self._cancel_message = msg self.__schedule_callbacks() return True @@ -173,7 +175,7 @@ def result(self): the future is done and has an exception set, this exception is raised. """ if self._state == _CANCELLED: - raise exceptions.CancelledError + raise exceptions.CancelledError(self._cancel_message) if self._state != _FINISHED: raise exceptions.InvalidStateError('Result is not ready.') self.__log_traceback = False @@ -190,7 +192,7 @@ def exception(self): InvalidStateError. """ if self._state == _CANCELLED: - raise exceptions.CancelledError + raise exceptions.CancelledError(self._cancel_message) if self._state != _FINISHED: raise exceptions.InvalidStateError('Exception is not set.') self.__log_traceback = False diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 717837d8568436..91610906e74295 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -230,7 +230,7 @@ def print_stack(self, *, limit=None, file=None): """ return base_tasks._task_print_stack(self, limit, file) - def cancel(self): + def cancel(self, msg=None): """Request that this task cancel itself. This arranges for a CancelledError to be thrown into the @@ -254,13 +254,14 @@ def cancel(self): if self.done(): return False if self._fut_waiter is not None: - if self._fut_waiter.cancel(): + if self._fut_waiter.cancel(msg=msg): # Leave self._fut_waiter; it may be a Task that # catches and ignores the cancellation so we may have # to cancel it again later. return True # It must be the case that self.__step is already scheduled. self._must_cancel = True + self._cancel_message = msg return True def __step(self, exc=None): @@ -269,7 +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() + exc = exceptions.CancelledError(self._cancel_message) self._must_cancel = False coro = self._coro self._fut_waiter = None @@ -287,11 +288,15 @@ def __step(self, exc=None): if self._must_cancel: # Task is cancelled right before coro stops. self._must_cancel = False - super().cancel() + super().cancel(msg=self._cancel_message) else: super().set_result(exc.value) - except exceptions.CancelledError: - super().cancel() # I.e., Future.cancel(self). + except exceptions.CancelledError as exc: + if exc.args: + cancel_msg = exc.args[0] + else: + cancel_msg = None + super().cancel(msg=cancel_msg) # I.e., Future.cancel(self). except (KeyboardInterrupt, SystemExit) as exc: super().set_exception(exc) raise @@ -319,7 +324,8 @@ def __step(self, exc=None): self.__wakeup, context=self._context) self._fut_waiter = result if self._must_cancel: - if self._fut_waiter.cancel(): + if self._fut_waiter.cancel( + msg=self._cancel_message): self._must_cancel = False else: new_exc = RuntimeError( @@ -708,12 +714,12 @@ def __init__(self, children, *, loop=None): self._children = children self._cancel_requested = False - def cancel(self): + def cancel(self, msg=None): if self.done(): return False ret = False for child in self._children: - if child.cancel(): + if child.cancel(msg=msg): ret = True if ret: # If any child tasks were actually cancelled, we should @@ -772,7 +778,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() + exc = exceptions.CancelledError(fut._cancel_message) outer.set_exception(exc) return else: @@ -791,7 +797,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() + res = exceptions.CancelledError(fut._cancel_message) else: res = fut.exception() if res is None: @@ -802,7 +808,8 @@ def _done_callback(fut): # If gather is being cancelled we must propagate the # cancellation regardless of *return_exceptions* argument. # See issue 32684. - outer.set_exception(exceptions.CancelledError()) + outer.set_exception( + exceptions.CancelledError(fut._cancel_message)) else: outer.set_result(results) diff --git a/Lib/asyncio/windows_events.py b/Lib/asyncio/windows_events.py index ac51109ff1a83d..c07fe3241c569a 100644 --- a/Lib/asyncio/windows_events.py +++ b/Lib/asyncio/windows_events.py @@ -75,9 +75,9 @@ def _cancel_overlapped(self): self._loop.call_exception_handler(context) self._ov = None - def cancel(self): + def cancel(self, msg=None): self._cancel_overlapped() - return super().cancel() + return super().cancel(msg=msg) def set_exception(self, exception): super().set_exception(exception) @@ -149,9 +149,9 @@ def _unregister_wait(self): self._unregister_wait_cb(None) - def cancel(self): + def cancel(self, msg=None): self._unregister_wait() - return super().cancel() + return super().cancel(msg=msg) def set_exception(self, exception): self._unregister_wait() From 403baa28f49019a7c948a68f6d3882580f40dee5 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Thu, 7 May 2020 06:49:53 -0700 Subject: [PATCH 5/8] Add docs and NEWS entry. --- Doc/library/asyncio-future.rst | 8 +++++++- Doc/library/asyncio-task.rst | 5 ++++- .../next/Library/2020-05-07-06-41-20.bpo-31033.waCj3n.rst | 1 + 3 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-05-07-06-41-20.bpo-31033.waCj3n.rst diff --git a/Doc/library/asyncio-future.rst b/Doc/library/asyncio-future.rst index 832d58119b7b07..e1ac18eaf09882 100644 --- a/Doc/library/asyncio-future.rst +++ b/Doc/library/asyncio-future.rst @@ -170,7 +170,7 @@ Future Object Returns the number of callbacks removed, which is typically 1, unless a callback was added more than once. - .. method:: cancel() + .. method:: cancel(msg=None) Cancel the Future and schedule callbacks. @@ -178,6 +178,9 @@ Future Object Otherwise, change the Future's state to *cancelled*, schedule the callbacks, and return ``True``. + .. versionchanged:: 3.9 + Added the ``msg`` parameter. + .. method:: exception() Return the exception that was set on this Future. @@ -255,3 +258,6 @@ the Future has a result:: - asyncio Future is not compatible with the :func:`concurrent.futures.wait` and :func:`concurrent.futures.as_completed` functions. + + - :meth:`asyncio.Future.cancel` accepts an optional ``msg`` argument, + but :func:`concurrent.futures.cancel` does not. diff --git a/Doc/library/asyncio-task.rst b/Doc/library/asyncio-task.rst index 42e2b4e2fc5b91..ecd19cb247b95e 100644 --- a/Doc/library/asyncio-task.rst +++ b/Doc/library/asyncio-task.rst @@ -723,7 +723,7 @@ Task Object .. deprecated-removed:: 3.8 3.10 The *loop* parameter. - .. method:: cancel() + .. method:: cancel(msg=None) Request the Task to be cancelled. @@ -738,6 +738,9 @@ Task Object suppressing cancellation completely is not common and is actively discouraged. + .. versionchanged:: 3.9 + Added the ``msg`` parameter. + .. _asyncio_example_task_cancel: The following example illustrates how coroutines can intercept diff --git a/Misc/NEWS.d/next/Library/2020-05-07-06-41-20.bpo-31033.waCj3n.rst b/Misc/NEWS.d/next/Library/2020-05-07-06-41-20.bpo-31033.waCj3n.rst new file mode 100644 index 00000000000000..e3d35a04aab513 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-05-07-06-41-20.bpo-31033.waCj3n.rst @@ -0,0 +1 @@ +Add a ``msg`` argument to :meth:`Future.cancel` and :meth:`Task.cancel`. From 6b133a2bc74d1211a7038b205d9d939a8c4e15c2 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Thu, 7 May 2020 17:29:49 -0700 Subject: [PATCH 6/8] Make None msg result in no arguments. --- Lib/asyncio/futures.py | 13 +++++++++++-- Lib/asyncio/tasks.py | 8 ++++---- Lib/test/test_asyncio/test_tasks.py | 12 ++++++------ Modules/_asynciomodule.c | 9 +++++---- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Lib/asyncio/futures.py b/Lib/asyncio/futures.py index 536d652dda8a9a..82367d6883d696 100644 --- a/Lib/asyncio/futures.py +++ b/Lib/asyncio/futures.py @@ -175,7 +175,7 @@ def result(self): the future is done and has an exception set, this exception is raised. """ if self._state == _CANCELLED: - raise exceptions.CancelledError(self._cancel_message) + raise _create_cancelled_error(self._cancel_message) if self._state != _FINISHED: raise exceptions.InvalidStateError('Result is not ready.') self.__log_traceback = False @@ -192,7 +192,7 @@ def exception(self): InvalidStateError. """ if self._state == _CANCELLED: - raise exceptions.CancelledError(self._cancel_message) + raise _create_cancelled_error(self._cancel_message) if self._state != _FINISHED: raise exceptions.InvalidStateError('Exception is not set.') self.__log_traceback = False @@ -293,6 +293,15 @@ def _set_result_unless_cancelled(fut, result): fut.set_result(result) +# This provides a nicer-looking traceback when no msg is passed, which +# has been asyncio's default behavior. +def _create_cancelled_error(msg=None): + if msg is None: + return exceptions.CancelledError() + + return exceptions.CancelledError(msg) + + def _convert_future_exc(exc): exc_class = type(exc) if exc_class is concurrent.futures.CancelledError: diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 91610906e74295..f8a06e117889ae 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -270,7 +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(self._cancel_message) + exc = futures._create_cancelled_error(self._cancel_message) self._must_cancel = False coro = self._coro self._fut_waiter = None @@ -778,7 +778,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(fut._cancel_message) + exc = futures._create_cancelled_error(fut._cancel_message) outer.set_exception(exc) return else: @@ -797,7 +797,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(fut._cancel_message) + res = futures._create_cancelled_error(fut._cancel_message) else: res = fut.exception() if res is None: @@ -809,7 +809,7 @@ def _done_callback(fut): # cancellation regardless of *return_exceptions* argument. # See issue 32684. outer.set_exception( - exceptions.CancelledError(fut._cancel_message)) + futures._create_cancelled_error(fut._cancel_message)) else: outer.set_result(results) diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 7ab42956e249d0..1c6004cfd8fc1d 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -517,8 +517,8 @@ async def task(): def test_cancel_with_message_then_future_result(self): # Test Future.result() after calling cancel() with a message. cases = [ - ((), (None,)), - ((None,), (None,)), + ((), ()), + ((None,), ()), (('my message',), ('my message',)), # Non-string values should roundtrip. ((5,), (5,)), @@ -547,8 +547,8 @@ async def coro(): def test_cancel_with_message_then_future_exception(self): # Test Future.exception() after calling cancel() with a message. cases = [ - ((), (None,)), - ((None,), (None,)), + ((), ()), + ((None,), ()), (('my message',), ('my message',)), # Non-string values should roundtrip. ((5,), (5,)), @@ -2319,8 +2319,8 @@ def cancelling_callback(_): def test_cancel_gather_2(self): cases = [ - ((), (None,)), - ((None,), (None,)), + ((), ()), + ((None,), ()), (('my message',), ('my message',)), # Non-string values should roundtrip. ((5,), (5,)), diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 5097f543c290d1..7d2983b899a030 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -599,11 +599,12 @@ future_set_exception(FutureObj *fut, PyObject *exc) static PyObject * create_cancelled_error(PyObject *msg) { - if (msg == NULL) { - msg = Py_None; + PyObject *exc; + if (msg == NULL || msg == Py_None) { + exc = PyObject_CallNoArgs(asyncio_CancelledError); + } else { + exc = PyObject_CallOneArg(asyncio_CancelledError, msg); } - PyObject *exc = PyObject_CallOneArg(asyncio_CancelledError, msg); - return exc; } From a0434c9c0cb22da504be23aa1402b3197d5f0d83 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Fri, 8 May 2020 04:50:51 -0700 Subject: [PATCH 7/8] Test the getter and setter. --- Lib/test/test_asyncio/test_futures.py | 21 +++++++++++++++++++++ Lib/test/test_asyncio/test_tasks.py | 25 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/Lib/test/test_asyncio/test_futures.py b/Lib/test/test_asyncio/test_futures.py index ee5edd5bd311fb..ec00896cc620b3 100644 --- a/Lib/test/test_asyncio/test_futures.py +++ b/Lib/test/test_asyncio/test_futures.py @@ -201,6 +201,27 @@ def test_uninitialized(self): self.assertFalse(fut.cancelled()) self.assertFalse(fut.done()) + def test_future_cancel_message_getter(self): + f = self._new_future(loop=self.loop) + self.assertTrue(hasattr(f, '_cancel_message')) + self.assertEqual(f._cancel_message, None) + + f.cancel('my message') + with self.assertRaises(asyncio.CancelledError): + self.loop.run_until_complete(f) + self.assertEqual(f._cancel_message, 'my message') + + def test_future_cancel_message_setter(self): + f = self._new_future(loop=self.loop) + f.cancel('my message') + f._cancel_message = 'my new message' + self.assertEqual(f._cancel_message, 'my new message') + + # Also check that the value is used for cancel(). + with self.assertRaises(asyncio.CancelledError): + self.loop.run_until_complete(f) + self.assertEqual(f._cancel_message, 'my new message') + def test_cancel(self): f = self._new_future(loop=self.loop) self.assertTrue(f.cancel()) diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 1c6004cfd8fc1d..e8cb73c8302e79 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -97,6 +97,31 @@ def setUp(self): self.loop.set_task_factory(self.new_task) self.loop.create_future = lambda: self.new_future(self.loop) + def test_task_cancel_message_getter(self): + async def coro(): + pass + t = self.new_task(self.loop, coro()) + self.assertTrue(hasattr(t, '_cancel_message')) + self.assertEqual(t._cancel_message, None) + + t.cancel('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(): + pass + t = self.new_task(self.loop, coro()) + t.cancel('my message') + 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: def __del__(self): From 22a54cc448747b7e6d0ee57b5c4cc93d2aead540 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Thu, 14 May 2020 17:16:10 -0700 Subject: [PATCH 8/8] Remove the _create_cancelled_error() helper function. --- Lib/asyncio/futures.py | 16 +++++----------- Lib/asyncio/tasks.py | 15 ++++++++++----- Lib/test/test_asyncio/test_tasks.py | 12 ++++++------ Modules/_asynciomodule.c | 4 +++- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/Lib/asyncio/futures.py b/Lib/asyncio/futures.py index 82367d6883d696..889f3e6eb86b03 100644 --- a/Lib/asyncio/futures.py +++ b/Lib/asyncio/futures.py @@ -175,7 +175,9 @@ def result(self): the future is done and has an exception set, this exception is raised. """ if self._state == _CANCELLED: - raise _create_cancelled_error(self._cancel_message) + raise exceptions.CancelledError( + '' if self._cancel_message is None else self._cancel_message) + if self._state != _FINISHED: raise exceptions.InvalidStateError('Result is not ready.') self.__log_traceback = False @@ -192,7 +194,8 @@ def exception(self): InvalidStateError. """ if self._state == _CANCELLED: - raise _create_cancelled_error(self._cancel_message) + raise exceptions.CancelledError( + '' if self._cancel_message is None else self._cancel_message) if self._state != _FINISHED: raise exceptions.InvalidStateError('Exception is not set.') self.__log_traceback = False @@ -293,15 +296,6 @@ def _set_result_unless_cancelled(fut, result): fut.set_result(result) -# This provides a nicer-looking traceback when no msg is passed, which -# has been asyncio's default behavior. -def _create_cancelled_error(msg=None): - if msg is None: - return exceptions.CancelledError() - - return exceptions.CancelledError(msg) - - def _convert_future_exc(exc): exc_class = type(exc) if exc_class is concurrent.futures.CancelledError: diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index f8a06e117889ae..d469084f4b0239 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -270,7 +270,8 @@ def __step(self, exc=None): f'_step(): already done: {self!r}, {exc!r}') if self._must_cancel: if not isinstance(exc, exceptions.CancelledError): - exc = futures._create_cancelled_error(self._cancel_message) + exc = exceptions.CancelledError('' + if self._cancel_message is None else self._cancel_message) self._must_cancel = False coro = self._coro self._fut_waiter = None @@ -778,7 +779,8 @@ def _done_callback(fut): # Check if 'fut' is cancelled first, as # 'fut.exception()' will *raise* a CancelledError # instead of returning it. - exc = futures._create_cancelled_error(fut._cancel_message) + exc = exceptions.CancelledError('' + if fut._cancel_message is None else fut._cancel_message) outer.set_exception(exc) return else: @@ -797,7 +799,9 @@ def _done_callback(fut): # Check if 'fut' is cancelled first, as # 'fut.exception()' will *raise* a CancelledError # instead of returning it. - res = futures._create_cancelled_error(fut._cancel_message) + res = exceptions.CancelledError( + '' if fut._cancel_message is None else + fut._cancel_message) else: res = fut.exception() if res is None: @@ -808,8 +812,9 @@ def _done_callback(fut): # If gather is being cancelled we must propagate the # cancellation regardless of *return_exceptions* argument. # See issue 32684. - outer.set_exception( - futures._create_cancelled_error(fut._cancel_message)) + exc = exceptions.CancelledError('' + if fut._cancel_message is None else fut._cancel_message) + outer.set_exception(exc) else: outer.set_result(results) diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index e8cb73c8302e79..1997e0d2a6ac9d 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -542,8 +542,8 @@ async def task(): def test_cancel_with_message_then_future_result(self): # Test Future.result() after calling cancel() with a message. cases = [ - ((), ()), - ((None,), ()), + ((), ('',)), + ((None,), ('',)), (('my message',), ('my message',)), # Non-string values should roundtrip. ((5,), (5,)), @@ -572,8 +572,8 @@ async def coro(): def test_cancel_with_message_then_future_exception(self): # Test Future.exception() after calling cancel() with a message. cases = [ - ((), ()), - ((None,), ()), + ((), ('',)), + ((None,), ('',)), (('my message',), ('my message',)), # Non-string values should roundtrip. ((5,), (5,)), @@ -2344,8 +2344,8 @@ def cancelling_callback(_): def test_cancel_gather_2(self): cases = [ - ((), ()), - ((None,), ()), + ((), ('',)), + ((None,), ('',)), (('my message',), ('my message',)), # Non-string values should roundtrip. ((5,), (5,)), diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index 7d2983b899a030..ff1b2b8b909c70 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -601,7 +601,9 @@ create_cancelled_error(PyObject *msg) { PyObject *exc; if (msg == NULL || msg == Py_None) { - exc = PyObject_CallNoArgs(asyncio_CancelledError); + msg = PyUnicode_FromString(""); + exc = PyObject_CallOneArg(asyncio_CancelledError, msg); + Py_DECREF(msg); } else { exc = PyObject_CallOneArg(asyncio_CancelledError, msg); }