asyncio.wait_for is still confusing

[DavidLewis]

BPO	37736
Nosy	@asvetlov, @cjrh, @1st1

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-08-01.09:25:01.672>
labels = ['3.8', 'type-bug', '3.7', 'expert-asyncio']
title = 'asyncio.wait_for is still confusing'
updated_at = <Date 2021-02-23.15:56:00.528>
user = 'https://bugs.python.org/DavidLewis'

bugs.python.org fields:

activity = <Date 2021-02-23.15:56:00.528>
actor = 'andrewborba'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2019-08-01.09:25:01.672>
creator = 'David Lewis'
dependencies = []
files = []
hgrepos = []
issue_num = 37736
keywords = []
message_count = 3.0
messages = ['348848', '354029', '387578']
nosy_count = 5.0
nosy_names = ['asvetlov', 'cjrh', 'yselivanov', 'David Lewis', 'andrewborba']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37736'
versions = ['Python 3.7', 'Python 3.8']

[DavidLewis]

This issue is a follow up to previous discussions about confusing results with asyncio.wait_for. In the current implementation, it seems unintuitive that a coroutine with a timeout argument may easily wait forever. Perhaps wait_for could use an await_cancellation=True kwarg.

Prior issues:

a) "It's a feature, not a bug" - Guido
python/asyncio#253 (comment)

b) "I don't feel comfortable with asyncio living with this bug till 3.8." - Yury
https://bugs.python.org/issue32751#msg318065

Originally, wait_for would cancel the future and raise TimeoutError immediately. In the case of a Task, it could remain active for some time after the timeout, since its cancellation is potentially asynchronous.

In (a), this behaviour was defended, since waiting on the cancellation would violate the implicit contract of the timeout argument to wait_for(). While the documentation suggests it's a poor idea, it's not illegal for a task to defer or entirely refuse cancellation.

In (b), the task outliving the TimeoutError was considered a bug, and the behaviour changed to its current state. To address the issue raised in (a), the documentation for wait_for now contains the line "The function will wait until the future is actually cancelled, so the total wait time may exceed the timeout."

However, we still have the case where a misbehaving Task can cause wait_for to hang indefinitely. For example, the following program doesn't terminate:

import asyncio, contextlib

async def bad():
    while True:
        with contextlib.suppress(asyncio.CancelledError):
            await asyncio.sleep(1)
            print("running...")

if __name__ == '__main__':
    asyncio.run(asyncio.wait_for(bad(), 1))

More realistically, the task may have cooperative cancellation logic that waits for something else to be tidied up:

try:
await wait_for(some_task(service), 10)
except TimeoutError:
...
finally:
service.stop()

[cjrh]

asyncio.wait_for is still confusing

Perhaps the confusion can be fixed with improvements to the docs? To me, these specific docs seem pretty clear now, but I might not be a good judge of that.

However, we still have the case where a misbehaving Task can cause wait_for to hang indefinitely.

The key word here is "misbehaving". Cooperative concurrency does require cooperation. There are many ways in which coroutines can misbehave, the popular one being calling blocking functions when they shouldn't. I would be very uncomfortable with my coroutine being killable (e.g. by wait_for) by some other means besides CancelledError (which I can intercept and manage cleanup).

The contract is: if my coroutine has a CancelledError raised, I take that to mean that I need to clean up whatever resources need cleanup, in a timely manner and then exit. If my coro refuses to exit, it is my coroutine that is wrong, not wait_for being unable to kill the coroutine.

I definitely agree with Yury that the previous behaviour, the one where wait_for could raise TimeoutError *before* the inner coro has exited, was buggy and needed to be fixed.

[andrewborba]

The key word here is "misbehaving". Cooperative concurrency does require cooperation.

Your stance makes sense in an ideal environment where we control every Task that our application deals with. However, there are some cases in which busted Task cancellation logic is out of our control to fix, and in some cases it may be critical to strictly enforce a timeout regardless of what a Task does. If wait_for is only to be used as a best effort, then we need a nuclear option as well.

[kumaraditya303]

Fixed by #96764

[kumaraditya303]

Catching CancelledError isn't recommended as it breaks asyncio internal timeout handling.