Hi Guido, First of all, thank you for your suggestion. I still have to wrap my mind around async programming contrary to multithreaded programming. I have rethought my code and could actually clean it up a great deal.
Regarding the documentation: I'd probably put a docstring on Task.cancel that reads similar to this: === Cancel a task by raising a CancellationError inside the wrapped coroutine. Contrary to Future.cancel this does not guarantee that the task is cancelled as the exception might be caught and acted upon, delaying or preventing cancellation of the task. Additonally, this function only marks the task for cancellation but will not directly cancel it (thus Task.cancelled will not return True right after calling Task.cancel). === You could add the "Additionally, ..." part as an rst ..note block, which I think would be prettier. Regards, Florian On Friday, March 28, 2014 10:58:03 PM UTC+1, Guido van Rossum wrote: > > Hi Florian, > > I'm sorry this wasn't documented in the official docs. (It's described in > PEP 3156 in the two sections on Futures and Tasks.) > > The behavior is indeed as intended, and came from a long discussion and > deep thinking about cancellation. When a Task (as opposed to a Future) is > cancelled, the coroutine that the Task wraps will receive a > asyncio.CancelledError, which will be handled by any except/finally clauses > active at that point. But this will only run when the event loop goes > through its next cycle. It is possible (though not recommended) for a > coroutine to completely ignore this exception, and it is also possible for > it to yield, waiting for additional blocking events, before finally > exiting. The cancelled() flag is only set once the coroutine has completed. > > We should really document this -- do you have a suggestion for a sentence > or paragraph that we should add to the docs? > > As a practical matter, you shouldn't write code that depends on checking > whether a task has a pending cancellation or not (since it may not take > effect at all). In your particular app, if you really need to keep track of > which tasks you have already tried to cancel, I recommend a separate > variable containing a set of tasks which you have cancelled for that > purpose. > > Finally, your description of how you are using this sounds like you are > really thinking of your collection of tasks as a collection of callbacks > that you are managing manually. Things might (or might not) become easier > if you let go of this perspective -- e.g. instead of looping over all tasks > and cancelling slow ones, wrap each task in a wait_for() call that enforces > a timeout. > > > > On Fri, Mar 28, 2014 at 2:22 PM, Florian Rüchel > <[email protected]<javascript:> > > wrote: > >> I had an issue with my code where I called Task.cancel on an instance and >> it would stay in state pending (cancelled returning False). I did some >> digging and found in the source that Task overwrites the cancel method and >> does not directly set the new state. Going back to the documentation, I did >> not find anything that documents this behavior. >> >> The problematic use case was a situation like the following: I iterate >> over a list of tasks, check various properties (like how long they are >> running etc.) and based on that decide to either give it more time or >> cancel it or check its results if its done. After having handled all >> running tasks, I do other work and later return to check them again. When I >> checked again, I explicitly checked the case where I already had cancelled >> a task. In this next iteration I want to see if it is done yet or if it >> needs some more time cleaning up. However, to my surprise, task.cancelled() >> returned false, which led me to execute task.cancel() again, which in turn >> raised a CancellationError inside the finally-block doing cleanup. I then >> added something like this: >> >> task.cancel() >> assert task.canelled() >> >> A statement that, on a future, has to succeed (as I see it from the >> source) but on a task must fail. Is there a reason this behavior is not >> documented or did I miss it somewhere? >> >> Also since this seems to be desired behavior, is there a reliable way to >> perform this check on a task? In __repr__ you check against _must_cancel >> for the difference between displaying "PENDING" and "CANCELLING" (this >> actually led me to the point of looking at the Task source). However, this >> is an internal variable and so I probably shouldn't use it. The only other >> thing I can come up with is using the return value and store it somewhere >> else and then check that variable, however that seems rather "dirty", >> storing a value that should belong to the task outside of it. >> > > > > -- > --Guido van Rossum (python.org/~guido) >
