[Sorry about the repost Guido, I forgot to CC the list]
On 14-05-26 06:21 PM, Guido van Rossum wrote:
> Re 1: I'm not sure I follow. Have you found specific code in Tulip
> that suppresses logging of cancellation? If so, are you sure you are
> hitting this code? Or do you at least have example code that appears
> to indicate this is the case? Is it perhaps the case that what you are
> seeing is a cancel() call being ignored because the Future/Task being
> cancelled is already completed?
Consider this example:
import asyncio
@asyncio.coroutine
def some_long_coroutine():
print('starting long task')
yield from asyncio.sleep(10)
print('done long task')
@asyncio.coroutine
def go(task):
print('Waiting for task')
yield from task
print('done waiting for task')
loop = asyncio.get_event_loop()
task = asyncio.Task(some_long_coroutine())
loop.call_later(1, task.cancel)
loop.call_soon(asyncio.async, go(task))
loop.run_forever()
Here the task is cancelled, which cancels the coroutine go().
CancelledError is raised inside of go() as you'd expect, and I can catch
it at the 'yield from task' point if I want. But if I don't catch it,
as in this example, nothing is logged.
This is the part I find violates the principle of least astonishment. :)
I would guess this is working by design if not by intention, because
Future.cancel() doesn't set Future._log_traceback and Future.__del__
only looks at _log_traceback.
> Re 2: I suppose there are some philosophical differences between
> cancellation Tulip and abort in Kaa. Does Kaa support Futures that
> aren't tied to a coroutine? (The Future class in Tulip has no
> coroutine -- only its Task subclass does.) What do you suppose should
> happen to a non-Task Future when it gets cancelled?
You're right that Kaa doesn't have a notion of bare coroutines, and
every coroutine returns a Future object. I appreciate this
optimization decision that went into Tulip.
These are the rules that seem intuitive to me:
1. The signature looks like Future.cancel(exc=None). If exc is passed,
it must be an instance or class that subclasses CancelledError.
2. If Future.result() or Future.exception() is called, it will raise
the exception passed to Future.cancel(). If no exception was
passed, it will raise CancelledError as today.
3. If a Task is cancelled with a custom exception, that exception is
raised inside the coroutine and bubbled up the call stack.
4. If a Gathering Future is cancelled with a custom exception, all the
child futures are cancelled with that exception.
> There's a zen of Python line that could possibly apply: "If the
> implementation is easy to explain, it may be a good idea." So if you
> can come up with a patch that looks particularly clean and elegant you
> may convince me.
I gave it a whirl. Attached. Consider it a first approximation. I
didn't verify the windows bits and a proper patch would have unit tests.
But I submit this for your consideration. If you aren't too aghast with
what you see, then I'll open an issue and polish things up.
Thanks,
Jason.
diff -r 61fdb7985c13 asyncio/futures.py
--- a/asyncio/futures.py Tue May 20 15:56:33 2014 +0200
+++ b/asyncio/futures.py Mon May 26 21:22:49 2014 -0400
@@ -183,7 +183,7 @@
}
self._loop.call_exception_handler(context)
- def cancel(self):
+ def cancel(self, exc=None):
"""Cancel the future and schedule callbacks.
If the future is already done or cancelled, return False. Otherwise,
@@ -192,6 +192,13 @@
"""
if self._state != _PENDING:
return False
+ if exc is None:
+ exc = CancelledError
+ elif (not isinstance(exc, CancelledError) and
+ not isinstance(exc, type) and
+ not issubclass(exc, CancelledError)):
+ raise ValueError('exc must subclass CancelledError')
+ self._exception = exc
self._state = _CANCELLED
self._schedule_callbacks()
return True
@@ -232,7 +239,7 @@
the future is done and has an exception set, this exception is raised.
"""
if self._state == _CANCELLED:
- raise CancelledError
+ raise self._exception
if self._state != _FINISHED:
raise InvalidStateError('Result is not ready.')
self._log_traceback = False
@@ -252,7 +259,7 @@
InvalidStateError.
"""
if self._state == _CANCELLED:
- raise CancelledError
+ raise self._exception
if self._state != _FINISHED:
raise InvalidStateError('Exception is not set.')
self._log_traceback = False
@@ -333,7 +340,7 @@
return
assert not self.done()
if other.cancelled():
- self.cancel()
+ self.cancel(self._exception)
else:
exception = other.exception()
if exception is not None:
@@ -362,6 +369,8 @@
def _check_cancel_other(f):
if f.cancelled():
+ # concurrent.futures.Future.cancel() doesn't support custom
+ # exceptions so we can't pass f._exception here.
fut.cancel()
new_future.add_done_callback(_check_cancel_other)
diff -r 61fdb7985c13 asyncio/tasks.py
--- a/asyncio/tasks.py Tue May 20 15:56:33 2014 +0200
+++ b/asyncio/tasks.py Mon May 26 21:22:49 2014 -0400
@@ -268,7 +268,7 @@
for line in traceback.format_exception_only(exc.__class__, exc):
print(line, file=file, end='')
- def cancel(self):
+ def cancel(self, exc=None):
"""Request that a task to cancel itself.
This arranges for a CancellationError to be thrown into the
@@ -290,14 +290,21 @@
"""
if self.done():
return False
+ if exc is None:
+ exc = futures.CancelledError
+ elif (not isinstance(exc, futures.CancelledError) and
+ not isinstance(exc, type) and
+ not issubclass(exc, futures.CancelledError)):
+ raise ValueError('exc must subclass CancelledError')
if self._fut_waiter is not None:
- if self._fut_waiter.cancel():
+ if self._fut_waiter.cancel(exc):
# 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._exception = exc
return True
def _step(self, value=None, exc=None):
@@ -305,8 +312,9 @@
'_step(): already done: {!r}, {!r}, {!r}'.format(self, value, exc)
if self._must_cancel:
if not isinstance(exc, futures.CancelledError):
- exc = futures.CancelledError()
+ exc = self._exception
self._must_cancel = False
+ self._exception = None
coro = self._coro
self._fut_waiter = None
@@ -322,7 +330,7 @@
except StopIteration as exc:
self.set_result(exc.value)
except futures.CancelledError as exc:
- super().cancel() # I.e., Future.cancel(self).
+ super().cancel(exc) # I.e., Future.cancel(self, exc).
except Exception as exc:
self.set_exception(exc)
except BaseException as exc:
@@ -336,8 +344,9 @@
result.add_done_callback(self._wakeup)
self._fut_waiter = result
if self._must_cancel:
- if self._fut_waiter.cancel():
+ if self._fut_waiter.cancel(self._exception):
self._must_cancel = False
+ self._exception = None
else:
self._loop.call_soon(
self._step, None,
@@ -596,11 +605,11 @@
super().__init__(loop=loop)
self._children = children
- def cancel(self):
+ def cancel(self, exc=None):
if self.done():
return False
for child in self._children:
- child.cancel()
+ child.cancel(exc)
return True
@@ -643,12 +652,15 @@
def _done_callback(i, fut):
nonlocal nfinished
if outer._state != futures._PENDING:
- if fut._exception is not None:
+ if fut._exception is not None and fut._state != futures._CANCELLED:
# Mark exception retrieved.
fut.exception()
return
if fut._state == futures._CANCELLED:
- res = futures.CancelledError()
+ if isinstance(fut._exception, type):
+ res = fut._exception()
+ else:
+ res = fut._exception
if not return_exceptions:
outer.set_exception(res)
return
@@ -708,7 +720,7 @@
inner.cancelled() or inner.exception()
return
if inner.cancelled():
- outer.cancel()
+ outer.cancel(inner._exception)
else:
exc = inner.exception()
if exc is not None:
diff -r 61fdb7985c13 asyncio/windows_events.py
--- a/asyncio/windows_events.py Tue May 20 15:56:33 2014 +0200
+++ b/asyncio/windows_events.py Mon May 26 21:22:49 2014 -0400
@@ -39,12 +39,12 @@
super().__init__(loop=loop)
self.ov = ov
- def cancel(self):
+ def cancel(self, exc=None):
try:
self.ov.cancel()
except OSError:
pass
- return super().cancel()
+ return super().cancel(exc)
class _WaitHandleFuture(futures.Future):
@@ -54,8 +54,8 @@
super().__init__(loop=loop)
self._wait_handle = wait_handle
- def cancel(self):
- super().cancel()
+ def cancel(self, exc=None):
+ super().cancel(exc)
try:
_overlapped.UnregisterWait(self._wait_handle)
except OSError as e:
