The documentation on task cancellation is unclear

[realazthat]

Here is an example:

@asyncio.coroutine
def something(sleep,marker):

    try:
        print ('something() is sleeping for %s  seconds!' % sleep,flush=True)
        yield from asyncio.sleep(sleep)
    finally:
        print ('cleaning something()!',flush=True)
        marker['cleaned'] = True

@asyncio.coroutine
def test_session2():
    marker = {'cleaned': False}
    yield from asyncio.wait_for(something(5, marker), timeout=10)
    #here something is cleaned up

    try:
        marker = {'cleaned': False}
        yield from asyncio.wait_for(something(10, marker), timeout=5)
    except asyncio.TimeoutError:
        print ('something() has timed out')
        pass
    #here something is not cancelled yet ... it cancels later.
    #intuitively, you'd think that wait_for would throw after something() is actually cancelled
    print ('is something cleaned up?', ('YES!' if marker['cleaned'] else 'NO!'))
    print ('sleeping a bit')
    yield from asyncio.sleep(1)
    print ('is something cleaned up?', ('YES!' if marker['cleaned'] else 'NO!'))

loop = asyncio.get_event_loop()

loop.run_until_complete(test_session2())

And the output:

$ python3 asyncio.wait_for.test.py
something() is sleeping for 5  seconds!
cleaning something()!
something() is sleeping for 10  seconds!
something() has timed out
is something cleaned up? NO!
sleeping a bit
cleaning something()!
is something cleaned up? YES!

So wait_for() throws TimeoutError, but the coroutine is not yet cancelled.

Basically, I think that the task should first be cancelled, and then wait_for() should return.

OR.

It should be more explicitly documented that the task will cancel, but not necessarily before wait_for() raises an error.

[gvanrossum]

I don't think this is violating the specification of wait_for(). Also, your example is way too complicated for anyone to follow. If you really think there is a bug in asyncio please provide a simpler example, and show the exact flow through your code rather than trying to guess what happened based on assertions.

[realazthat]

@gvanrossum I've updated the example to something simpler.

[realazthat]

Also, as a workaround, I've implemented my own wait_for() wrapper around asyncio.wait_for(), which does the same thing except wait until the task is cancelled before raising TimeoutError.

def wait_for(coro,timeout,*,loop=None):
    if not isinstance(coro, asyncio.Future):
        coro = asyncio.Task(coro,loop=loop)


    completed_future = asyncio.Future(loop=loop)

    @asyncio.coroutine
    def wrapper():
        try:
            return (yield from coro)
        finally:
            completed_future.set_result(None)
    try:
        return (yield from asyncio.wait_for(wrapper(),timeout=timeout,loop=loop))
    except asyncio.TimeoutError:
        if not coro.cancelled():
            coro.cancel()
        yield from completed_future
        raise

[gvanrossum]

OK, your workaround helped me understand what's going on. I don't think we can add this behavior to wait_for() -- it is totally legal (if uncommon) for a task to absorb a cancellation (thrown in) and take its time (waiting for something else) before actually returning, or even simply ignoring the cancellation altogether. In any case a task's response to cancellation (as opposed to a plain Future's) is asynchronous. I don't think wait_for() should be required to wait any further when the timeout is reached. So I'm closing this since it's a feature, not a bug.

[vstinner]

If you believe that the documentation is uncomplete about cancellation, please propose a new text, and I will update the doc.

Currently, the doc says "If the wait is cancelled, the future fut is also cancelled."

Maybe we need to enhance the documentation to explain better how tasks handle cancellation. For example, add a section and add a link from wait_for() doc to this new section? Currently, the explanation of task cancellation lives at:
https://docs.python.org/dev/library/asyncio-task.html#asyncio.Task

[realazthat]

Returns result of the Future or coroutine. When a timeout occurs, it cancels the task and raises asyncio.TimeoutError. To avoid the task cancellation, wrap it in shield().

If the wait is cancelled, the future fut is also cancelled.

Change that to:

Returns result of the Future or coroutine. When a timeout occurs, it cancels the task and raises asyncio.TimeoutError. To avoid the task cancellation, wrap it in shield().

Note that wait_for() does not wait until the task is cancelled.

If the wait is cancelled, the future fut is also cancelled.

[vstinner]

Note that wait_for() does not wait until the task is cancelled.

This sentence is unclear, or wrong.

If a task is cancelled, wait_for() immediatly returns (raise CancelledError).

If wait_for() is used on a task and a task doesn't complete before the timeout, the task is cancelled. The problem is that the cancellation is only "scheduled": cancelled() returns False. It's counter intuitive.

A shorter example is: "task.cancel(); assert not task.cancelled()".

Ok, I now have a better understanding of the documentation issue.

[realazthat]

@Haypo

OK I'll rephrase:

Note that in the event of a timeout, wait_for() does not wait until the task is cancelled; it raises the asyncio.TimeoutError immediately, as this is the normal Task.cancel() behavior.

[achimnol]

I'd like to add another point for improved docs.
If a task is cancelled but the task has remaining await statements, it must be awaited again.
This may seem to be trivial and obvious for asyncio designers, but people may get lost because it's easy to think "cancel" as complete cancellation of a task instead of injecting asyncio.CancelledError into it. (Of course, cancellation is different from other types of exceptions as it marks the future status differently.)
I found this in aiohttp's documentation (see cleanup_background_tasks function in the example).

A minimal example to demonstrate its necessity:

import asyncio

async def something():
    try:
        print('do')
        await asyncio.sleep(1)
    except asyncio.CancelledError:
        print('cancelled')
    finally:
        print('finally')
        await asyncio.sleep(1)
        print('done')

async def cancel_it(task):
    await asyncio.sleep(0.5)
    task.cancel()
    await task  # if not awaited, above finally block would not finish.

if __name__ == '__main__':
    loop = asyncio.get_event_loop()
    t = loop.create_task(something())
    try:
        loop.run_until_complete(cancel_it(t))
    except KeyboardInterrupt:
        loop.stop()
    finally:
        loop.close()

I believe "await-after-cancel" must be explained in asyncio docs.

[gjcarneiro]

You are catching asyncio.CancelledError inside the task, and not re-raising. IIRC that means that means to ignore cancellation. That is the reason you need to await the task in this example, it is because the task has not been cancelled and you have to wait for it to complete normally.

…

On 10 January 2017 at 08:55, Joongi Kim ***@***.***> wrote: I'd like to add another point for improved docs. If a task is cancelled but the task has remaining await statements, it *must* be awaited again. I found this in aiohttp's documentation <http://aiohttp.readthedocs.io/en/stable/web.html#background-tasks> (see cleanup_background_tasks function in the example). A minimal example to demonstrate its necessity: import asyncio async def something(): try: print('do') await asyncio.sleep(1) except asyncio.CancelledError: print('cancelled') finally: print('finally') await asyncio.sleep(1) print('done') async def cancel_it(task): await asyncio.sleep(0.5) task.cancel() await task # if not awaited, above finally block would not finish. if __name__ == '__main__': loop = asyncio.get_event_loop() t = loop.create_task(something()) try: loop.run_until_complete(cancel_it(t)) except KeyboardInterrupt: loop.stop() finally: loop.close() I believe "await-after-cancel" must be explained in asyncio docs. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#253 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACGGaGiOwPjLCv1rz4pdWJb0WqHXzx8Nks5rQ0d4gaJpZM4FUzTo> .

-- Gustavo J. A. M. Carneiro Gambit Research "The universe is always one step beyond logic." -- Frank Herbert

[achimnol]

Yes, in my example I'm using CancelledError as a signal for graceful shutdown.
In many realistic applications, I think this will be the normal case rather than just re-raising CancelledError.

[achimnol]

The point that confused me before some experience was:

• Omitting await for cancelled tasks works fine as long as the tasks have no more awaits internally.
• It is easy to think that it's not necessary since the code works; for example, most close() methods are not coroutines though recent discussions show that they should be coroutines.

I think the docs need to carefully guide the asyncio beginners not to fall into misunderstandings (like me...), by describing when to use cancellation, what it stands for whether a task absorbs CancelledError or re-raises, etc. The learning experience should be consistent no matter which asyncio-based library you start with.