wait_for and Condition.wait still not playing nicely

[criches]

BPO	39032
Nosy	@asvetlov, @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-12-12.22:11:47.065>
labels = ['3.8', 'type-bug', '3.7', 'expert-asyncio']
title = 'wait_for and Condition.wait still not playing nicely'
updated_at = <Date 2019-12-27.17:20:17.826>
user = 'https://bugs.python.org/criches'

bugs.python.org fields:

activity = <Date 2019-12-27.17:20:17.826>
actor = 'Antonin Rousset'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['asyncio']
creation = <Date 2019-12-12.22:11:47.065>
creator = 'criches'
dependencies = []
files = []
hgrepos = []
issue_num = 39032
keywords = []
message_count = 1.0
messages = ['358307']
nosy_count = 4.0
nosy_names = ['asvetlov', 'yselivanov', 'criches', 'Antonin Rousset']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue39032'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[criches]

This is related to https://bugs.python.org/issue22970, https://bugs.python.org/issue33638, and https://bugs.python.org/issue32751. I've replicated the issue on Python 3.6.9, 3.7.4, and 3.8.0. Looking at the source, I'm fairly sure the bug is still in master right now.

The problem is yet another case of wait_for returning early, before the child has been fully cancelled and terminated. The issue arises if wait_for itself is cancelled. Take the following minimal example:

cond = asyncio.Condition()
async def coro():
    async with cond:
        await asyncio.wait_for(cond.wait(), timeout=999)

If coro is cancelled a few seconds after being run, wait_for will cancel the cond.wait(), then immediately re-raise the CancelledError inside coro, leading to "RuntimeError: Lock is not acquired."

Relevant source code plucked from the 3.8 branch is as follows:

try:
# wait until the future completes or the timeout
try:
await waiter
except exceptions.CancelledError:
fut.remove_done_callback(cb)
fut.cancel()
raise

        if fut.done():
            return fut.result()
        else:
            fut.remove_done_callback(cb)
            # We must ensure that the task is not running
            # after wait_for() returns.
            # See https://bugs.python.org/issue32751
            await _cancel_and_wait(fut, loop=loop)
            raise exceptions.TimeoutError()
    finally:
        timeout_handle.cancel()

Note how if the timeout occurs, the method waits for the future to complete before raising. If CancelledError is thrown, it doesn't.

A simple fix seems to be replacing the "fut.cancel()" with "await _cancel_and_wait(fut, loop=loop)" so the behaviour is the same in both cases, however I'm only superficially familiar with the code, and am unsure if this would cause other problems.