classification
Title: asyncio.gather should not use special Future
Type: behavior Stage: patch review
Components: asyncio Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Martin.Teichmann, asvetlov, twisteroid ambassador, yselivanov
Priority: normal Keywords: patch

Created on 2018-05-02 18:39 by Martin.Teichmann, last changed 2018-05-10 18:11 by ned.deily.

Pull Requests
URL Status Linked Edit
PR 6694 open Martin.Teichmann, 2018-05-02 18:45
Messages (3)
msg316085 - (view) Author: Martin Teichmann (Martin.Teichmann) * Date: 2018-05-02 18:39
asyncio.gather() returns a _GatheringFuture, which inherits from asyncio.Future. This is weird in current asyncio, as futures are supposed to be created with loop.create_future(). So I tried to reimplement gather() without this weird special future. I succeeded, yet I stumbled over weird inconsistencies with cancellation. There are three cases:

- coroutines have no special notion of cancellation, they treat CancelledError as any other exception

- futures have a clear distinction between exceptions and cancellation: future.set_exception(CancelledError()) is different from future.cancel(), as only for the latter future.cancelled() is True. This is used in the _GatheringFuture: it is cancelled() only if it got cancelled via future.cancel(), if its children gets cancelled it may set_exception(CancelledError()), but it will not be cancelled itself.

- Tasks consider raising a CancelledError always as a cancellation, whether it actually got cancelled or the wrapped coroutine raised CancelledError for whatever other reason. There is one exception: if the coroutine manages to return immediately after being cancelled, it raises a CancelledError, but task.cancelled() is false. So if a coroutine ends in

    current_task().cancel()
    return

the current task raises a CancelledError, but task.cancelled() is false.

I consider the last exception actually a bug, but it allows me to make my inheritance-free gather() look to the outside exactly like it used to be.
msg316281 - (view) Author: twisteroid ambassador (twisteroid ambassador) * Date: 2018-05-08 08:25
I would like to comment on the last observation about current_task().cancel(). I also ran into this corner case recently. 

When a task is cancelled from outside, by virtue of there *being something outside doing the cancelling*, the task being cancelled is not currently running, and that usually means the task is waiting at an `await` statement, in which case a CancelledError will be raised at this `await` statement the next time this task runs. The other possibility is that the task has been created but has not had a chance to run yet, and in this case the task is marked cancelled, and code inside the task will not run.

When one cancels a task from the inside by calling cancel() on the task object, the task will still run as normal until it reaches the next `await` statement, where a CancelledError will be raised. If there is no `await` between calling cancel() and the task returning, however, the CancelledError is never raised inside the task, and the task will end up in the state of done() == True, cancelled() == False, exception() == CancelledError. Anyone awaiting for the task will get a CancelledError without a meaningful stack trace, like this:

Traceback (most recent call last):
  File "cancel_self.py", line 89, in run_one
    loop.run_until_complete(coro)
  File "C:\Program Files\Python36\lib\asyncio\base_events.py", line 467, in run_until_complete
    return future.result()
concurrent.futures._base.CancelledError

This is the case described in the original comment.

I would also consider this a bug or at least undesired behavior. Since CancelledError is never raised inside the task, code in the coroutine cannot catch it, and after the task returns the return value is lost. For a coroutine that acquires and returns some resource (say asyncio.open_connection()), this means that neither the task itself nor the code awaiting the task can release the resource, leading to leakage.

I guess one should be careful not to cancel the current task from the inside.
msg316371 - (view) Author: Martin Teichmann (Martin.Teichmann) * Date: 2018-05-10 17:28
I looked a bit into the details, and found that bpo-30048 created the described weird behavior. There they fixed the problem that a cancel is ignored if a coroutine manages to cancel its own task and return immediately. As shown in the discussion there, this is actually something happening in real code, and is a valid use case.

They fixed that by setting a CancelledError as an exception raised by the task, but did not cancel that task (they could have, I tested it, it would pass all tests).

But this is just a side show of the fact that we have now four different beasts that can be awaited, and behave differently: coroutines, Futures, Tasks, and _GatheringFutures. I think we should consolidate that.
History
Date User Action Args
2018-05-10 18:11:39ned.deilysetnosy: + yselivanov, asvetlov
components: + asyncio
2018-05-10 17:28:43Martin.Teichmannsetmessages: + msg316371
2018-05-08 08:25:09twisteroid ambassadorsetnosy: + twisteroid ambassador
messages: + msg316281
2018-05-02 18:45:52Martin.Teichmannsetkeywords: + patch
stage: patch review
pull_requests: + pull_request6388
2018-05-02 18:39:18Martin.Teichmanncreate