You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
asyncio.TaskGroup may not cancel all tasks on failure of one #94398
While writing documentation for asyncio.TaskGroup I discovered something fishy.
There is a behavior where if a task fails with an exception (except asyncio.CancelledError), the remaining tasks are cancelled. There is also a rule that this is only done once, implemented using self._aborted, which is set by self._abort().
But what should happen if new tasks are created afterself._abort() is called, and one of those tasks fails? Then the remaining new tasks are not cancelled. To repro, we need something that creates two tasks, where the first one fails, and the second catches asyncio.CancelledError and when caught creates two more tasks. The third task would then fail, and the fourth task might wait for ever, never getting cancelled.
Is this a bug (or design flaw)? I think we decided we would support task creation during the wait (even though EdgeDb's TaskGroup disallowed it) so that it's possible to dynamically create new tasks forever -- this seems useful. But I'm not sure we thought deep about whether to allow creating new tasks once we're waiting for all cancelled tasks to finish.
How would we fix it? Disallowing new task creation once self._aborted is set seems excessive, since it would disallow legitimate creation of new tasks during cleanup. We could keep a weak set of tasks that we haven't cancelled yet, and if one of those fails we could cancel all others in that set (and remove them from the set) -- this would essentially create successive "generations" of tasks that live or die together (starting a new generation once any member of the current generation dies). Is this worth it?
CC: @njs@Tinche@agronholm -- I assume this problem doesn't exist in Trio because of its level-triggered cancellation, but maybe one of you still has a useful insight.
My gut reaction is we disallow new tasks in that Task group; it's shutting down and closed for business. If a task inside wants to spawn new tasks during cleanup it can create a new TG then and there, right?
In that test, we could extend coro2() to create several new tasks, one of which fails -- the others will run uninterrupted. (UPDATE: I didn't do that in the actual test since the fix for the issue is to forbid creating new tasks at all.)
Activity
Tinche commentedon Jun 29, 2022
My gut reaction is we disallow new tasks in that Task group; it's shutting down and closed for business. If a task inside wants to spawn new tasks during cleanup it can create a new TG then and there, right?
gvanrossum commentedon Jun 29, 2022
That would be an easy fix (just two lines in
create_task()) and causes no existing tests to fail. And yes, creating a new TG makes sense.agronholm commentedon Jun 29, 2022
A sample that reproduces this problem would indeed be welcome.
gvanrossum commentedon Jun 29, 2022
The new test I added to the PR might help:
https://github.com/python/cpython/pull/94400/files#diff-6f3f42fe6cdf1387de256a705e9f0d4c809ba04a2da04c317c9c79cfc4c5bc69R723
In that test, we could extend
coro2()to create several new tasks, one of which fails -- the others will run uninterrupted. (UPDATE: I didn't do that in the actual test since the fix for the issue is to forbid creating new tasks at all.)GH-94398: TaskGroup: Fail create_task() during shutdown (GH-94400)
pythonGH-94398: TaskGroup: Fail create_task() during shutdown (python…
GH-94398: TaskGroup: Fail create_task() during shutdown (GH-94400) (G…