sure, safe_set_result should not be public method. i like idea with helper function i just updated code review with helper function.
On Mar 3, 2014, at 7:48 PM, Guido van Rossum <[email protected]> wrote: > It sounds like Nikolay's general problem is that various parts of asyncio are > a bit cavalier about returning Futures and scheduling set_result() on them > without first checking if the Future is cancelled. > > But I don't like the solution of adding a new public method to Future -- I > worry that it will be used to mask bugs. We could have an internal helper API > used for this purpose and passed to call_soon, e.g. > > def _set_result_unless_cancelled(fut, result=None): > if not fut.cancelled(): > fut.set_result(result) > > and then use loop.call_soon(_set_result_unless_cancelled, fut, result) in > places that currently call loop.call_soon(fut.set_result, result). > > Another possibility might be to have an internal helper that suppresses a > specific exception, e.g. > > def _call_ignoring_exception(exc, func, *args): > try: > func(*args) > except exc: > pass > > and then use loop.call_soon(_call_ignoring_exception, fut.set_result, None). > > > On Mon, Mar 3, 2014 at 5:27 PM, Nikolay Kim <[email protected]> wrote: > > On Mar 3, 2014, at 5:07 PM, Victor Stinner <[email protected]> wrote: > > > Hi, > > > > 2014-03-03 23:59 GMT+01:00 Nikolay Kim <[email protected]>: > >> I see "InvalidStateError: CANCELLED: Future<CANCELLED>” exception in my > >> production system. > >> It doesn’t really affect application logic, just annoying. also it is very > >> hard to understand where exception from. > >> so i propose to add safe_set_result() method to Future that will check > >> status before setting result. > >> code review is here https://codereview.appspot.com/69870048/ > > > > I didn't understood with your example what happens. In fact, the > > future is cancelled before future.set_result() is called. Something > > like: > > --- > > fut = Future() > > <do something else> > > fut.cancel() > > <do something else> > > fut.set_result() > > --- > > where "<do something else>" is a side-effect of asynchronous > > programming and "yield from”. > > yes, thats right. but fut.set_result is called by event loop. i see this kind > of exceptions: > Feb 28 00:28:03 ip-10-31-197-58 ERROR [asyncio] Exception in callback > <bound method Future.set_result of Future<CANCELLED>>(None,) > handle: Handle(<bound method Future.set_result of Future<CANCELLED>>, (None,)) > Traceback (most recent call last): > File > "/usr/local/lib/python3.3/dist-packages/asyncio-0.4.1_p1-py3.3.egg/asyncio/events.py", > line 39, in _run > self._callback(*self._args) > File > "/usr/local/lib/python3.3/dist-packages/asyncio-0.4.1_p1-py3.3.egg/asyncio/futures.py", > line 298, in set_result > raise InvalidStateError('{}: {!r}'.format(self._state, self)) > asyncio.futures.InvalidStateError: CANCELLED: Future<CANCELLED> > > > > > Checking the future status before scheduling the call (with > > loop.call_soon) to set_result() would not fix the issue. > > it doesn’t check status of fut before scheduling call with call_soon, it > checks status of fut during set_result call. > > > > > > If I understood correctly, _SelectorSocketTransport constructor > > doesn't call directly set_result() (but use loop.call_soon instead) to > > wait until protocol.connection_made() has been called. So > > loop.create_connection() only returns the socket when > > protocol.connection_made() has been called. Is this correct? > > > _SelectorSocketTransport constructor cannot call directly > > protocol.connection_made()? > > waiter future is used by clients and that’s different topic. > this problem is not directly related to _SelectorSocketTransport, but how it > uses Future.set_result. > i can write similar test for asyncio.sleep as well. > > > > > -- > --Guido van Rossum (python.org/~guido)
