From 22feeb88b473b288950cdb2f6c5d28692274b5f9 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Wed, 24 Jan 2018 11:31:01 -0500 Subject: [PATCH] bpo-32643: Drop support for a few private Task and Future APIs. (#5293) Specifically, it's not possible to subclass Task/Future classes and override the following methods: * Future._schedule_callbacks * Task._step * Task._wakeup --- Lib/asyncio/futures.py | 8 +- Lib/asyncio/tasks.py | 28 ++--- Lib/test/test_asyncio/test_futures.py | 4 - Lib/test/test_asyncio/test_tasks.py | 28 +---- .../2018-01-24-00-32-58.bpo-32643.VWipsW.rst | 2 + Modules/_asynciomodule.c | 115 ++---------------- Modules/clinic/_asynciomodule.c.h | 77 +----------- 7 files changed, 33 insertions(+), 229 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-01-24-00-32-58.bpo-32643.VWipsW.rst diff --git a/Lib/asyncio/futures.py b/Lib/asyncio/futures.py index 59621ffb6e7..4a56f32c774 100644 --- a/Lib/asyncio/futures.py +++ b/Lib/asyncio/futures.py @@ -131,10 +131,10 @@ class Future: if self._state != _PENDING: return False self._state = _CANCELLED - self._schedule_callbacks() + self.__schedule_callbacks() return True - def _schedule_callbacks(self): + def __schedule_callbacks(self): """Internal: Ask the event loop to call all callbacks. The callbacks are scheduled to be called as soon as possible. Also @@ -234,7 +234,7 @@ class Future: raise InvalidStateError('{}: {!r}'.format(self._state, self)) self._result = result self._state = _FINISHED - self._schedule_callbacks() + self.__schedule_callbacks() def set_exception(self, exception): """Mark the future done and set an exception. @@ -251,7 +251,7 @@ class Future: "and cannot be raised into a Future") self._exception = exception self._state = _FINISHED - self._schedule_callbacks() + self.__schedule_callbacks() self.__log_traceback = True def __await__(self): diff --git a/Lib/asyncio/tasks.py b/Lib/asyncio/tasks.py index 609b8e8a048..3590748c477 100644 --- a/Lib/asyncio/tasks.py +++ b/Lib/asyncio/tasks.py @@ -99,7 +99,7 @@ class Task(futures._PyFuture): # Inherit Python Task implementation self._coro = coro self._context = contextvars.copy_context() - self._loop.call_soon(self._step, context=self._context) + self._loop.call_soon(self.__step, context=self._context) _register_task(self) def __del__(self): @@ -185,11 +185,11 @@ class Task(futures._PyFuture): # Inherit Python Task implementation # 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. + # It must be the case that self.__step is already scheduled. self._must_cancel = True return True - def _step(self, exc=None): + def __step(self, exc=None): if self.done(): raise futures.InvalidStateError( f'_step(): already done: {self!r}, {exc!r}') @@ -232,17 +232,17 @@ class Task(futures._PyFuture): # Inherit Python Task implementation f'Task {self!r} got Future ' f'{result!r} attached to a different loop') self._loop.call_soon( - self._step, new_exc, context=self._context) + self.__step, new_exc, context=self._context) elif blocking: if result is self: new_exc = RuntimeError( f'Task cannot await on itself: {self!r}') self._loop.call_soon( - self._step, new_exc, context=self._context) + self.__step, new_exc, context=self._context) else: result._asyncio_future_blocking = False result.add_done_callback( - self._wakeup, context=self._context) + self.__wakeup, context=self._context) self._fut_waiter = result if self._must_cancel: if self._fut_waiter.cancel(): @@ -252,33 +252,33 @@ class Task(futures._PyFuture): # Inherit Python Task implementation f'yield was used instead of yield from ' f'in task {self!r} with {result!r}') self._loop.call_soon( - self._step, new_exc, context=self._context) + self.__step, new_exc, context=self._context) elif result is None: # Bare yield relinquishes control for one event loop iteration. - self._loop.call_soon(self._step, context=self._context) + self._loop.call_soon(self.__step, context=self._context) elif inspect.isgenerator(result): # Yielding a generator is just wrong. new_exc = RuntimeError( f'yield was used instead of yield from for ' f'generator in task {self!r} with {result}') self._loop.call_soon( - self._step, new_exc, context=self._context) + self.__step, new_exc, context=self._context) else: # Yielding something else is an error. new_exc = RuntimeError(f'Task got bad yield: {result!r}') self._loop.call_soon( - self._step, new_exc, context=self._context) + self.__step, new_exc, context=self._context) finally: _leave_task(self._loop, self) self = None # Needed to break cycles when an exception occurs. - def _wakeup(self, future): + def __wakeup(self, future): try: future.result() except Exception as exc: # This may also be a cancellation. - self._step(exc) + self.__step(exc) else: # Don't pass the value of `future.result()` explicitly, # as `Future.__iter__` and `Future.__await__` don't need it. @@ -286,7 +286,7 @@ class Task(futures._PyFuture): # Inherit Python Task implementation # Python eval loop would use `.send(value)` method call, # instead of `__next__()`, which is slower for futures # that return non-generator iterators from their `__iter__`. - self._step() + self.__step() self = None # Needed to break cycles when an exception occurs. @@ -513,7 +513,7 @@ def __sleep0(): This is a private helper for 'asyncio.sleep()', used when the 'delay' is set to 0. It uses a bare 'yield' - expression (which Task._step knows how to handle) + expression (which Task.__step knows how to handle) instead of creating a Future object. """ yield diff --git a/Lib/test/test_asyncio/test_futures.py b/Lib/test/test_asyncio/test_futures.py index 37f4c6562fe..8c837ad6b62 100644 --- a/Lib/test/test_asyncio/test_futures.py +++ b/Lib/test/test_asyncio/test_futures.py @@ -175,10 +175,6 @@ class BaseFutureTests: with self.assertRaises((RuntimeError, AttributeError)): fut.remove_done_callback(lambda f: None) - fut = self.cls.__new__(self.cls, loop=self.loop) - with self.assertRaises((RuntimeError, AttributeError)): - fut._schedule_callbacks() - fut = self.cls.__new__(self.cls, loop=self.loop) try: repr(fut) diff --git a/Lib/test/test_asyncio/test_tasks.py b/Lib/test/test_asyncio/test_tasks.py index 1c361c8ec17..daa1ff927e6 100644 --- a/Lib/test/test_asyncio/test_tasks.py +++ b/Lib/test/test_asyncio/test_tasks.py @@ -1401,17 +1401,6 @@ class BaseTaskTests: self.assertTrue(t.done()) self.assertIsNone(t.result()) - def test_step_with_baseexception(self): - @asyncio.coroutine - def notmutch(): - raise BaseException() - - task = self.new_task(self.loop, notmutch()) - self.assertRaises(BaseException, task._step) - - self.assertTrue(task.done()) - self.assertIsInstance(task.exception(), BaseException) - def test_baseexception_during_cancel(self): def gen(): @@ -2275,22 +2264,12 @@ def add_subclass_tests(cls): self.calls = collections.defaultdict(lambda: 0) super().__init__(*args, **kwargs) - def _schedule_callbacks(self): - self.calls['_schedule_callbacks'] += 1 - return super()._schedule_callbacks() - def add_done_callback(self, *args, **kwargs): self.calls['add_done_callback'] += 1 return super().add_done_callback(*args, **kwargs) class Task(CommonFuture, BaseTask): - def _step(self, *args): - self.calls['_step'] += 1 - return super()._step(*args) - - def _wakeup(self, *args): - self.calls['_wakeup'] += 1 - return super()._wakeup(*args) + pass class Future(CommonFuture, BaseFuture): pass @@ -2310,12 +2289,11 @@ def add_subclass_tests(cls): self.assertEqual( dict(task.calls), - {'_step': 2, '_wakeup': 1, 'add_done_callback': 1, - '_schedule_callbacks': 1}) + {'add_done_callback': 1}) self.assertEqual( dict(fut.calls), - {'add_done_callback': 1, '_schedule_callbacks': 1}) + {'add_done_callback': 1}) # Add patched Task & Future back to the test case cls.Task = Task diff --git a/Misc/NEWS.d/next/Library/2018-01-24-00-32-58.bpo-32643.VWipsW.rst b/Misc/NEWS.d/next/Library/2018-01-24-00-32-58.bpo-32643.VWipsW.rst new file mode 100644 index 00000000000..7fdd53d8e27 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-01-24-00-32-58.bpo-32643.VWipsW.rst @@ -0,0 +1,2 @@ +Make Task._step, Task._wakeup and Future._schedule_callbacks methods +private. diff --git a/Modules/_asynciomodule.c b/Modules/_asynciomodule.c index a08096942b8..a5cf687d889 100644 --- a/Modules/_asynciomodule.c +++ b/Modules/_asynciomodule.c @@ -18,9 +18,6 @@ _Py_IDENTIFIER(current_task); _Py_IDENTIFIER(get_event_loop); _Py_IDENTIFIER(send); _Py_IDENTIFIER(throw); -_Py_IDENTIFIER(_step); -_Py_IDENTIFIER(_schedule_callbacks); -_Py_IDENTIFIER(_wakeup); /* State of the _asyncio module */ @@ -128,7 +125,6 @@ class _asyncio.Future "FutureObj *" "&Future_Type" /* Get FutureIter from Future */ static PyObject * future_new_iter(PyObject *); -static inline int future_call_schedule_callbacks(FutureObj *); static PyRunningLoopHolder * new_running_loop_holder(PyObject *); @@ -520,7 +516,7 @@ future_set_result(FutureObj *fut, PyObject *res) fut->fut_result = res; fut->fut_state = STATE_FINISHED; - if (future_call_schedule_callbacks(fut) == -1) { + if (future_schedule_callbacks(fut) == -1) { return NULL; } Py_RETURN_NONE; @@ -568,7 +564,7 @@ future_set_exception(FutureObj *fut, PyObject *exc) fut->fut_exception = exc_val; fut->fut_state = STATE_FINISHED; - if (future_call_schedule_callbacks(fut) == -1) { + if (future_schedule_callbacks(fut) == -1) { return NULL; } @@ -686,7 +682,7 @@ future_cancel(FutureObj *fut) } fut->fut_state = STATE_CANCELLED; - if (future_call_schedule_callbacks(fut) == -1) { + if (future_schedule_callbacks(fut) == -1) { return NULL; } @@ -1267,23 +1263,6 @@ _asyncio_Future__repr_info_impl(FutureObj *self) asyncio_future_repr_info_func, self, NULL); } -/*[clinic input] -_asyncio.Future._schedule_callbacks -[clinic start generated code]*/ - -static PyObject * -_asyncio_Future__schedule_callbacks_impl(FutureObj *self) -/*[clinic end generated code: output=5e8958d89ea1c5dc input=4f5f295f263f4a88]*/ -{ - ENSURE_FUTURE_ALIVE(self) - - int ret = future_schedule_callbacks(self); - if (ret == -1) { - return NULL; - } - Py_RETURN_NONE; -} - static PyObject * FutureObj_repr(FutureObj *fut) { @@ -1407,7 +1386,6 @@ static PyMethodDef FutureType_methods[] = { _ASYNCIO_FUTURE_DONE_METHODDEF _ASYNCIO_FUTURE_GET_LOOP_METHODDEF _ASYNCIO_FUTURE__REPR_INFO_METHODDEF - _ASYNCIO_FUTURE__SCHEDULE_CALLBACKS_METHODDEF {NULL, NULL} /* Sentinel */ }; @@ -1452,25 +1430,6 @@ static PyTypeObject FutureType = { .tp_finalize = (destructor)FutureObj_finalize, }; -static inline int -future_call_schedule_callbacks(FutureObj *fut) -{ - if (Future_CheckExact(fut) || Task_CheckExact(fut)) { - return future_schedule_callbacks(fut); - } - else { - /* `fut` is a subclass of Future */ - PyObject *ret = _PyObject_CallMethodId( - (PyObject*)fut, &PyId__schedule_callbacks, NULL); - if (ret == NULL) { - return -1; - } - - Py_DECREF(ret); - return 0; - } -} - static void FutureObj_dealloc(PyObject *self) { @@ -1701,8 +1660,6 @@ class _asyncio.Task "TaskObj *" "&Task_Type" /*[clinic end generated code: output=da39a3ee5e6b4b0d input=719dcef0fcc03b37]*/ static int task_call_step_soon(TaskObj *, PyObject *); -static inline PyObject * task_call_wakeup(TaskObj *, PyObject *); -static inline PyObject * task_call_step(TaskObj *, PyObject *); static PyObject * task_wakeup(TaskObj *, PyObject *); static PyObject * task_step(TaskObj *, PyObject *); @@ -1736,7 +1693,7 @@ TaskStepMethWrapper_call(TaskStepMethWrapper *o, PyErr_SetString(PyExc_TypeError, "function takes no positional arguments"); return NULL; } - return task_call_step(o->sw_task, o->sw_arg); + return task_step(o->sw_task, o->sw_arg); } static int @@ -1812,7 +1769,7 @@ TaskWakeupMethWrapper_call(TaskWakeupMethWrapper *o, return NULL; } - return task_call_wakeup(o->ww_task, fut); + return task_wakeup(o->ww_task, fut); } static int @@ -2287,32 +2244,6 @@ _asyncio_Task_print_stack_impl(TaskObj *self, PyObject *limit, asyncio_task_print_stack_func, self, limit, file, NULL); } -/*[clinic input] -_asyncio.Task._step - - exc: object = None -[clinic start generated code]*/ - -static PyObject * -_asyncio_Task__step_impl(TaskObj *self, PyObject *exc) -/*[clinic end generated code: output=7ed23f0cefd5ae42 input=1e19a985ace87ca4]*/ -{ - return task_step(self, exc == Py_None ? NULL : exc); -} - -/*[clinic input] -_asyncio.Task._wakeup - - fut: object -[clinic start generated code]*/ - -static PyObject * -_asyncio_Task__wakeup_impl(TaskObj *self, PyObject *fut) -/*[clinic end generated code: output=75cb341c760fd071 input=6a0616406f829a7b]*/ -{ - return task_wakeup(self, fut); -} - /*[clinic input] _asyncio.Task.set_result @@ -2429,8 +2360,6 @@ static PyMethodDef TaskType_methods[] = { _ASYNCIO_TASK_CANCEL_METHODDEF _ASYNCIO_TASK_GET_STACK_METHODDEF _ASYNCIO_TASK_PRINT_STACK_METHODDEF - _ASYNCIO_TASK__WAKEUP_METHODDEF - _ASYNCIO_TASK__STEP_METHODDEF _ASYNCIO_TASK__REPR_INFO_METHODDEF {NULL, NULL} /* Sentinel */ }; @@ -2493,32 +2422,6 @@ TaskObj_dealloc(PyObject *self) Py_TYPE(task)->tp_free(task); } -static inline PyObject * -task_call_wakeup(TaskObj *task, PyObject *fut) -{ - if (Task_CheckExact(task)) { - return task_wakeup(task, fut); - } - else { - /* `task` is a subclass of Task */ - return _PyObject_CallMethodIdObjArgs((PyObject*)task, &PyId__wakeup, - fut, NULL); - } -} - -static inline PyObject * -task_call_step(TaskObj *task, PyObject *arg) -{ - if (Task_CheckExact(task)) { - return task_step(task, arg); - } - else { - /* `task` is a subclass of Task */ - return _PyObject_CallMethodIdObjArgs((PyObject*)task, &PyId__step, - arg, NULL); - } -} - static int task_call_step_soon(TaskObj *task, PyObject *arg) { @@ -2964,10 +2867,10 @@ task_wakeup(TaskObj *task, PyObject *o) break; /* exception raised */ case 0: Py_DECREF(fut_result); - return task_call_step(task, NULL); + return task_step(task, NULL); default: assert(res == 1); - result = task_call_step(task, fut_result); + result = task_step(task, fut_result); Py_DECREF(fut_result); return result; } @@ -2976,7 +2879,7 @@ task_wakeup(TaskObj *task, PyObject *o) PyObject *fut_result = PyObject_CallMethod(o, "result", NULL); if (fut_result != NULL) { Py_DECREF(fut_result); - return task_call_step(task, NULL); + return task_step(task, NULL); } /* exception raised */ } @@ -2991,7 +2894,7 @@ task_wakeup(TaskObj *task, PyObject *o) PyErr_NormalizeException(&et, &ev, &tb); } - result = task_call_step(task, ev); + result = task_step(task, ev); Py_DECREF(et); Py_XDECREF(tb); diff --git a/Modules/clinic/_asynciomodule.c.h b/Modules/clinic/_asynciomodule.c.h index 9fc9d6b17c4..d8e1b6a7987 100644 --- a/Modules/clinic/_asynciomodule.c.h +++ b/Modules/clinic/_asynciomodule.c.h @@ -252,23 +252,6 @@ _asyncio_Future__repr_info(FutureObj *self, PyObject *Py_UNUSED(ignored)) return _asyncio_Future__repr_info_impl(self); } -PyDoc_STRVAR(_asyncio_Future__schedule_callbacks__doc__, -"_schedule_callbacks($self, /)\n" -"--\n" -"\n"); - -#define _ASYNCIO_FUTURE__SCHEDULE_CALLBACKS_METHODDEF \ - {"_schedule_callbacks", (PyCFunction)_asyncio_Future__schedule_callbacks, METH_NOARGS, _asyncio_Future__schedule_callbacks__doc__}, - -static PyObject * -_asyncio_Future__schedule_callbacks_impl(FutureObj *self); - -static PyObject * -_asyncio_Future__schedule_callbacks(FutureObj *self, PyObject *Py_UNUSED(ignored)) -{ - return _asyncio_Future__schedule_callbacks_impl(self); -} - PyDoc_STRVAR(_asyncio_Task___init____doc__, "Task(coro, *, loop=None)\n" "--\n" @@ -501,64 +484,6 @@ exit: return return_value; } -PyDoc_STRVAR(_asyncio_Task__step__doc__, -"_step($self, /, exc=None)\n" -"--\n" -"\n"); - -#define _ASYNCIO_TASK__STEP_METHODDEF \ - {"_step", (PyCFunction)_asyncio_Task__step, METH_FASTCALL|METH_KEYWORDS, _asyncio_Task__step__doc__}, - -static PyObject * -_asyncio_Task__step_impl(TaskObj *self, PyObject *exc); - -static PyObject * -_asyncio_Task__step(TaskObj *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) -{ - PyObject *return_value = NULL; - static const char * const _keywords[] = {"exc", NULL}; - static _PyArg_Parser _parser = {"|O:_step", _keywords, 0}; - PyObject *exc = Py_None; - - if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, - &exc)) { - goto exit; - } - return_value = _asyncio_Task__step_impl(self, exc); - -exit: - return return_value; -} - -PyDoc_STRVAR(_asyncio_Task__wakeup__doc__, -"_wakeup($self, /, fut)\n" -"--\n" -"\n"); - -#define _ASYNCIO_TASK__WAKEUP_METHODDEF \ - {"_wakeup", (PyCFunction)_asyncio_Task__wakeup, METH_FASTCALL|METH_KEYWORDS, _asyncio_Task__wakeup__doc__}, - -static PyObject * -_asyncio_Task__wakeup_impl(TaskObj *self, PyObject *fut); - -static PyObject * -_asyncio_Task__wakeup(TaskObj *self, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) -{ - PyObject *return_value = NULL; - static const char * const _keywords[] = {"fut", NULL}; - static _PyArg_Parser _parser = {"O:_wakeup", _keywords, 0}; - PyObject *fut; - - if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, - &fut)) { - goto exit; - } - return_value = _asyncio_Task__wakeup_impl(self, fut); - -exit: - return return_value; -} - PyDoc_STRVAR(_asyncio_Task_set_result__doc__, "set_result($self, result, /)\n" "--\n" @@ -786,4 +711,4 @@ _asyncio__leave_task(PyObject *module, PyObject *const *args, Py_ssize_t nargs, exit: return return_value; } -/*[clinic end generated code: output=bcbaf1b2480f4aa9 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=b6148b0134e7a819 input=a9049054013a1b77]*/