On Tue, May 27, 2014 at 9:49 PM, Jason Tackaberry <[email protected]> wrote:
> > On 14-05-27 11:03 AM, Guido van Rossum wrote: > > A cancellation, however, is different. It is initiated by the *consumer* > to indicate that it is no longer interested in the result. > > > I think out of everything you said (and thanks for the nuanced > explanation), this could be the crux of the disagreement Your scenario > describes a producer and a consumer, but the cancellation isn't > *necessarily* initiated by the consumer. It could be, as in my case, > initiated by an outside agent. > > For example, imagine a download manager coroutine that starts a number of > download coroutines. An outside supervisor may cancel one of the download > tasks (say by user request). The download manager is the consumer of the > download coroutine, but it's not the one that cancelled the task, so I > argue it's a surprising side effect that the download manager is also > terminated silently (even while the other download coroutines live on, if > they were gather()ed). > But how would the outside supervisor know which task to cancel if it wasn't told by the download manager? You can't just go running around cancelling tasks if you don't know what they represent. > By requiring CancelledError to be explicitly caught to suppress noisiness, > it's at least spelled out in the code what the effects of cancellation > are. Maybe a given yielded coroutine can be cancelled without consequence, > or maybe not. When cancellation is noisy, it forces the programmer to > specifically think about happens when yielded Future is cancelled. > > One ugly case with the current behaviour is to debug why some coroutine 5 > levels up just stops for no reason, where it might be sufficiently far > enough removed from the sub-task that got cancelled that the programmer > might not even connect the dots. > Hm... The cases are still different, although I think I understand the situation you are describing. Calling result() or exception() on a cancelled Future *is* noisy, in the sense that it raises CancelledError in the calling code. However, if the caller doesn't catch this, it will appear as if the caller itself is cancelled. If this bubbles all the way to a top level task, the exception is indeed not logged. Compare this to a Future with an exception (set by set_exception()): calling result() raises that exception, and if the caller doesn't catch it, the exception bubbles up (just as in the previous case) but at the top level the exception *is* logged. 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...) This subtle bug would explain the difficulty you have debugging that specific case. > As an experiment, I added "self._log_traceback = True" to > Future.cancel() and ran the unittests. There were 81 log messages about > cancelled Futures. I think this speaks for itself; I wouldn't want to add > extra code just to suppress that exception to all those perfectly fine > tests. > > > Isn't this the tail wagging the dog? You've developed unit tests for a > particular behaviour. Here the discussion is about changing the behaviour, > and the argument against changing that behaviour is that the test cases > would need to change too. > > It seems like an argument from inertia, or maybe argument by popularity, > rather than addressing the the idea on its own merits (like you did in your > earlier explanation). Also by calling the tests "perfectly fine" you're > begging the question. :) > I don't think so, but I am getting a little tired of your rhetoric here. My theory is that most of these tests are indeed perfectly fine in that CancelledError is never raised or ignored -- a Future is simply cancelled and then ignored. I claim that that *is* perfectly fine. > > > > I've seen your patch now, but I really would like to find a way to > convince you do come up with a different way to achieve what you're trying > to do. Your change would suddenly add a communication channel back from the > consumer to the producer that can transmit any amount of data (through > exception attributes), compared to the single "cancelled" bit that we > currently have. > > > True, and this could be abused. But aren't we all consenting adults here? > :) > > > > Can't you create an object that is shared between consumer and producer > where the consumer can indicate the difference between soft and hard aborts? > > > I could, but it's inelegant, and would violate the boundaries of cleanly > separated layers of the application. > > 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. > > The way this works with custom cancellation exceptions is S would do > C.cancel(SoftCancelledError) or C.cancel(HardCancelledError) depending on > the case. When B yields C, it catches SoftCancelledError and can clean up > and continue on. HardCancelledError isn't caught in B, so it bubbles up to > A which catches it and takes some higher level action. > > With what you proposed, S would need to know how to pass a message to *B*(and > B could be multiple possible objects depending on the case) to > indicate the task that just got cancelled was a *soft* cancel. And it'd > similarly need to know how to pass a message to *A* (which could again be > multiple possible objects) to indicate the task that got cancelled was a > *hard* cancel. > > So the fact of the cancellation is in band, but the *type* of > cancellation is out of band. Doesn't that seem kludgy? > > If I had to pick one battle to fight between the two issues, it'd be this > one. I can live with silent cancellations as it just requires me to pay > attention, whereas the inability to cancel with custom exceptions requires > inelegant workarounds. > > Thanks for the insight so far, > Jason. > I'll think about this more once we have agreement on the first issue. -- --Guido van Rossum (python.org/~guido)
