On 14-05-28 07:34 PM, Guido van Rossum wrote: > I have a feeling that what's really going on here is a subtle bug in > Task._step(): when the coroutine raises CancelledError, it *always* > assumes this means the Task's cancellation was effective, and calls > Future.cancel().But it should only do that as a delayed response to a > cancel() call on the Task -- if the Task was never cancelled, it > should treat the CancelledError as any other exception, and call > self.set_exception() instead. (However, the question is, how long to > wait before we assume the task swallowed the cancellation? _step() > can't look inside the code to see whether it is still in a > try/except/finally clause handling the cancellation...)
It feels like we're honing in on my objection. My concern is really about the current behaviour of silent uncaught implicit cancellations than about silent uncaught explicit ones. That is, if t1 yields from t2, and t2 is cancelled, I could understand if that fact isn't logged. But t1 being cancelled as a consequence of t2 is what I believe should be logged if not caught. It took several readings of your comments above but I think I understand the issue now. Let me see if I've got it: Task.cancel() could set a flag that the task was explicitly cancelled, and if in the next step the coroutine raises CancelledError and that flag is set, the cancellation was successful and Future.cancel() is called. But if the coroutine catches CancelledError, it could yield back arbitrarily. Maybe it reraises, or maybe it swallows the cancellation and continues on, and some time later on yields a completely unrelated future that gets cancelled. That second CancelledError is ambiguous: Task._step() doesn't know if it's related to the original task cancellation (which was in fact swallowed). One idea: CancelledError instances could take an 'origin' attribute that indicates the Future that originated the cancellation. Then, when Task._step() gets a CancelledError from the coroutine, it can check the origin attribute to see if it's self. If it is, the cancellation was successful and Future.cancel() can be called. If not, it should call Future.set_exception() instead. The disadvantage is that this CancelledError instance would need to be passed around, or at least the origin attribute preserved through new instances. This probably starts to look a bit like the patch I drafted for the custom cancellation feature. > Specifically, if I have coroutines A -> B -> C, if C is "soft > cancelled" by some outside supervisory task S, then B handles this > case and continues on working. If C is "hard cancelled" then B > should stop and A needs to be aware of this and handle it > appropriately. > [...] > > I'll think about this more once we have agreement on the first issue. I just thought of a related scenario that may be easier to explain/understand. In the A -> B -> C chain, B isn't able to distinguish between C being cancelled and B itself being cancelled. It's easy to imagine that B would want to handle those cases differently.
