From 0cf16f9ea014b17d398ee3971d4976c698533318 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Mon, 25 Dec 2017 10:48:15 -0500 Subject: bpo-32363: Disable Task.set_exception() and Task.set_result() (#4923) --- Lib/asyncio/futures.py | 7 +- Lib/asyncio/tasks.py | 24 ++++-- Lib/test/test_asyncio/test_futures.py | 3 +- Lib/test/test_asyncio/test_tasks.py | 93 +++++++++++++++++----- .../2017-12-19-00-37-28.bpo-32363.YTeGU0.rst | 4 + Modules/_asynciomodule.c | 51 +++++++++--- Modules/clinic/_asynciomodule.c.h | 20 ++++- 7 files changed, 158 insertions(+), 44 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-12-19-00-37-28.bpo-32363.YTeGU0.rst diff --git a/Lib/asyncio/futures.py b/Lib/asyncio/futures.py index 24843c0..13fb47c 100644 --- a/Lib/asyncio/futures.py +++ b/Lib/asyncio/futures.py @@ -239,14 +239,15 @@ class Future: self._schedule_callbacks() self._log_traceback = True - def __iter__(self): + def __await__(self): if not self.done(): self._asyncio_future_blocking = True yield self # This tells Task to wait for completion. - assert self.done(), "await wasn't used with future" + if not self.done(): + raise RuntimeError("await wasn't used with future") return self.result() # May raise too. - __await__ = __iter__ # make compatible with 'await' expression + __iter__ = __await__ # make compatible with 'yield from'. # Needed for testing purposes. diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 572e707..b118088 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -37,7 +37,9 @@ def all_tasks(loop=None): return {t for t in _all_tasks if futures._get_loop(t) is loop} -class Task(futures.Future): +class Task(futures._PyFuture): # Inherit Python Task implementation + # from a Python Future implementation. + """A coroutine wrapped in a Future.""" # An important invariant maintained while a Task not done: @@ -107,11 +109,17 @@ class Task(futures.Future): if self._source_traceback: context['source_traceback'] = self._source_traceback self._loop.call_exception_handler(context) - futures.Future.__del__(self) + super().__del__() def _repr_info(self): return base_tasks._task_repr_info(self) + def set_result(self, result): + raise RuntimeError('Task does not support set_result operation') + + def set_exception(self, exception): + raise RuntimeError('Task does not support set_exception operation') + def get_stack(self, *, limit=None): """Return the list of stack frames for this task's coroutine. @@ -180,7 +188,9 @@ class Task(futures.Future): return True def _step(self, exc=None): - assert not self.done(), f'_step(): already done: {self!r}, {exc!r}' + if self.done(): + raise futures.InvalidStateError( + f'_step(): already done: {self!r}, {exc!r}') if self._must_cancel: if not isinstance(exc, futures.CancelledError): exc = futures.CancelledError() @@ -201,15 +211,15 @@ class Task(futures.Future): if self._must_cancel: # Task is cancelled right before coro stops. self._must_cancel = False - self.set_exception(futures.CancelledError()) + super().set_exception(futures.CancelledError()) else: - self.set_result(exc.value) + super().set_result(exc.value) except futures.CancelledError: super().cancel() # I.e., Future.cancel(self). except Exception as exc: - self.set_exception(exc) + super().set_exception(exc) except BaseException as exc: - self.set_exception(exc) + super().set_exception(exc) raise else: blocking = getattr(result, '_asyncio_future_blocking', None) diff --git a/Lib/test/test_asyncio/test_futures.py b/Lib/test/test_asyncio/test_futures.py index f777a42..d339616 100644 --- a/Lib/test/test_asyncio/test_futures.py +++ b/Lib/test/test_asyncio/test_futures.py @@ -370,7 +370,8 @@ class BaseFutureTests: def test(): arg1, arg2 = coro() - self.assertRaises(AssertionError, test) + with self.assertRaisesRegex(RuntimeError, "await wasn't used"): + test() fut.cancel() @mock.patch('asyncio.base_events.logger') diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 4cf694f..0dc6c32 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -1332,17 +1332,23 @@ class BaseTaskTests: self.assertIsNone(task._fut_waiter) self.assertTrue(fut.cancelled()) - def test_step_in_completed_task(self): + def test_task_set_methods(self): @asyncio.coroutine def notmuch(): return 'ko' gen = notmuch() task = self.new_task(self.loop, gen) - task.set_result('ok') - self.assertRaises(AssertionError, task._step) - gen.close() + with self.assertRaisesRegex(RuntimeError, 'not support set_result'): + task.set_result('ok') + + with self.assertRaisesRegex(RuntimeError, 'not support set_exception'): + task.set_exception(ValueError()) + + self.assertEqual( + self.loop.run_until_complete(task), + 'ko') def test_step_result(self): @asyncio.coroutine @@ -2231,10 +2237,59 @@ def add_subclass_tests(cls): return cls +class SetMethodsTest: + + def test_set_result_causes_invalid_state(self): + Future = type(self).Future + self.loop.call_exception_handler = exc_handler = mock.Mock() + + async def foo(): + await asyncio.sleep(0.1, loop=self.loop) + return 10 + + task = self.new_task(self.loop, foo()) + Future.set_result(task, 'spam') + + self.assertEqual( + self.loop.run_until_complete(task), + 'spam') + + exc_handler.assert_called_once() + exc = exc_handler.call_args[0][0]['exception'] + with self.assertRaisesRegex(asyncio.InvalidStateError, + r'step\(\): already done'): + raise exc + + def test_set_exception_causes_invalid_state(self): + class MyExc(Exception): + pass + + Future = type(self).Future + self.loop.call_exception_handler = exc_handler = mock.Mock() + + async def foo(): + await asyncio.sleep(0.1, loop=self.loop) + return 10 + + task = self.new_task(self.loop, foo()) + Future.set_exception(task, MyExc()) + + with self.assertRaises(MyExc): + self.loop.run_until_complete(task) + + exc_handler.assert_called_once() + exc = exc_handler.call_args[0][0]['exception'] + with self.assertRaisesRegex(asyncio.InvalidStateError, + r'step\(\): already done'): + raise exc + + @unittest.skipUnless(hasattr(futures, '_CFuture') and hasattr(tasks, '_CTask'), 'requires the C _asyncio module') -class CTask_CFuture_Tests(BaseTaskTests, test_utils.TestCase): +class CTask_CFuture_Tests(BaseTaskTests, SetMethodsTest, + test_utils.TestCase): + Task = getattr(tasks, '_CTask', None) Future = getattr(futures, '_CFuture', None) @@ -2245,11 +2300,8 @@ class CTask_CFuture_Tests(BaseTaskTests, test_utils.TestCase): @add_subclass_tests class CTask_CFuture_SubclassTests(BaseTaskTests, test_utils.TestCase): - class Task(tasks._CTask): - pass - - class Future(futures._CFuture): - pass + Task = getattr(tasks, '_CTask', None) + Future = getattr(futures, '_CFuture', None) @unittest.skipUnless(hasattr(tasks, '_CTask'), @@ -2257,9 +2309,7 @@ class CTask_CFuture_SubclassTests(BaseTaskTests, test_utils.TestCase): @add_subclass_tests class CTaskSubclass_PyFuture_Tests(BaseTaskTests, test_utils.TestCase): - class Task(tasks._CTask): - pass - + Task = getattr(tasks, '_CTask', None) Future = futures._PyFuture @@ -2268,15 +2318,14 @@ class CTaskSubclass_PyFuture_Tests(BaseTaskTests, test_utils.TestCase): @add_subclass_tests class PyTask_CFutureSubclass_Tests(BaseTaskTests, test_utils.TestCase): - class Future(futures._CFuture): - pass - + Future = getattr(futures, '_CFuture', None) Task = tasks._PyTask @unittest.skipUnless(hasattr(tasks, '_CTask'), 'requires the C _asyncio module') class CTask_PyFuture_Tests(BaseTaskTests, test_utils.TestCase): + Task = getattr(tasks, '_CTask', None) Future = futures._PyFuture @@ -2284,22 +2333,22 @@ class CTask_PyFuture_Tests(BaseTaskTests, test_utils.TestCase): @unittest.skipUnless(hasattr(futures, '_CFuture'), 'requires the C _asyncio module') class PyTask_CFuture_Tests(BaseTaskTests, test_utils.TestCase): + Task = tasks._PyTask Future = getattr(futures, '_CFuture', None) -class PyTask_PyFuture_Tests(BaseTaskTests, test_utils.TestCase): +class PyTask_PyFuture_Tests(BaseTaskTests, SetMethodsTest, + test_utils.TestCase): + Task = tasks._PyTask Future = futures._PyFuture @add_subclass_tests class PyTask_PyFuture_SubclassTests(BaseTaskTests, test_utils.TestCase): - class Task(tasks._PyTask): - pass - - class Future(futures._PyFuture): - pass + Task = tasks._PyTask + Future = futures._PyFuture @unittest.skipUnless(hasattr(tasks, '_CTask'), diff --git a/Misc/NEWS.d/next/Library/2017-12-19-00-37-28.bpo-32363.YTeGU0.rst b/Misc/NEWS.d/next/Library/2017-12-19-00-37-28.bpo-32363.YTeGU0.rst new file mode 100644 index 0000000..4c376c5 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-12-19-00-37-28.bpo-32363.YTeGU0.rst @@ -0,0 +1,4 @@ +Make asyncio.Task.set_exception() and set_result() raise +NotImplementedError. Task._step() and Future.__await__() raise proper +exceptions when they are in an invalid state, instead of raising an +AssertionError. diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index f70e345..f8165ab 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -779,7 +779,7 @@ _asyncio_Future_exception_impl(FutureObj *self) /*[clinic input] _asyncio.Future.set_result - res: object + result: object / Mark the future done and set its result. @@ -789,11 +789,11 @@ InvalidStateError. [clinic start generated code]*/ static PyObject * -_asyncio_Future_set_result(FutureObj *self, PyObject *res) -/*[clinic end generated code: output=a620abfc2796bfb6 input=5b9dc180f1baa56d]*/ +_asyncio_Future_set_result(FutureObj *self, PyObject *result) +/*[clinic end generated code: output=1ec2e6bcccd6f2ce input=8b75172c2a7b05f1]*/ { ENSURE_FUTURE_ALIVE(self) - return future_set_result(self, res); + return future_set_result(self, result); } /*[clinic input] @@ -1468,8 +1468,8 @@ FutureIter_iternext(futureiterobject *it) Py_INCREF(fut); return (PyObject *)fut; } - PyErr_SetString(PyExc_AssertionError, - "yield from wasn't used with future"); + PyErr_SetString(PyExc_RuntimeError, + "await wasn't used with future"); return NULL; } @@ -2232,6 +2232,39 @@ _asyncio_Task__wakeup_impl(TaskObj *self, PyObject *fut) return task_wakeup(self, fut); } +/*[clinic input] +_asyncio.Task.set_result + + result: object + / +[clinic start generated code]*/ + +static PyObject * +_asyncio_Task_set_result(TaskObj *self, PyObject *result) +/*[clinic end generated code: output=1dcae308bfcba318 input=9d1a00c07be41bab]*/ +{ + PyErr_SetString(PyExc_RuntimeError, + "Task does not support set_result operation"); + return NULL; +} + +/*[clinic input] +_asyncio.Task.set_exception + + exception: object + / +[clinic start generated code]*/ + +static PyObject * +_asyncio_Task_set_exception(TaskObj *self, PyObject *exception) +/*[clinic end generated code: output=bc377fc28067303d input=9a8f65c83dcf893a]*/ +{ + PyErr_SetString(PyExc_RuntimeError, + "Task doed not support set_exception operation"); + return NULL; +} + + static void TaskObj_finalize(TaskObj *task) { @@ -2304,12 +2337,12 @@ static void TaskObj_dealloc(PyObject *); /* Needs Task_CheckExact */ static PyMethodDef TaskType_methods[] = { _ASYNCIO_FUTURE_RESULT_METHODDEF _ASYNCIO_FUTURE_EXCEPTION_METHODDEF - _ASYNCIO_FUTURE_SET_RESULT_METHODDEF - _ASYNCIO_FUTURE_SET_EXCEPTION_METHODDEF _ASYNCIO_FUTURE_ADD_DONE_CALLBACK_METHODDEF _ASYNCIO_FUTURE_REMOVE_DONE_CALLBACK_METHODDEF _ASYNCIO_FUTURE_CANCELLED_METHODDEF _ASYNCIO_FUTURE_DONE_METHODDEF + _ASYNCIO_TASK_SET_RESULT_METHODDEF + _ASYNCIO_TASK_SET_EXCEPTION_METHODDEF _ASYNCIO_TASK_CURRENT_TASK_METHODDEF _ASYNCIO_TASK_ALL_TASKS_METHODDEF _ASYNCIO_TASK_CANCEL_METHODDEF @@ -2461,7 +2494,7 @@ task_step_impl(TaskObj *task, PyObject *exc) PyObject *o; if (task->task_state != STATE_PENDING) { - PyErr_Format(PyExc_AssertionError, + PyErr_Format(asyncio_InvalidStateError, "_step(): already done: %R %R", task, exc ? exc : Py_None); diff --git a/Modules/clinic/_asynciomodule.c.h b/Modules/clinic/_asynciomodule.c.h index 6a35434..f2e0f40 100644 --- a/Modules/clinic/_asynciomodule.c.h +++ b/Modules/clinic/_asynciomodule.c.h @@ -86,7 +86,7 @@ _asyncio_Future_exception(FutureObj *self, PyObject *Py_UNUSED(ignored)) } PyDoc_STRVAR(_asyncio_Future_set_result__doc__, -"set_result($self, res, /)\n" +"set_result($self, result, /)\n" "--\n" "\n" "Mark the future done and set its result.\n" @@ -536,6 +536,22 @@ exit: return return_value; } +PyDoc_STRVAR(_asyncio_Task_set_result__doc__, +"set_result($self, result, /)\n" +"--\n" +"\n"); + +#define _ASYNCIO_TASK_SET_RESULT_METHODDEF \ + {"set_result", (PyCFunction)_asyncio_Task_set_result, METH_O, _asyncio_Task_set_result__doc__}, + +PyDoc_STRVAR(_asyncio_Task_set_exception__doc__, +"set_exception($self, exception, /)\n" +"--\n" +"\n"); + +#define _ASYNCIO_TASK_SET_EXCEPTION_METHODDEF \ + {"set_exception", (PyCFunction)_asyncio_Task_set_exception, METH_O, _asyncio_Task_set_exception__doc__}, + PyDoc_STRVAR(_asyncio__get_running_loop__doc__, "_get_running_loop($module, /)\n" "--\n" @@ -747,4 +763,4 @@ _asyncio__leave_task(PyObject *module, PyObject *const *args, Py_ssize_t nargs, exit: return return_value; } -/*[clinic end generated code: output=5d100b3d74f2a0f4 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=616e814431893dcc input=a9049054013a1b77]*/ -- cgit v0.12