-
-
Notifications
You must be signed in to change notification settings - Fork 32k
gh-101581 Optionally prevent child tasks from being cancelled in asyncio taskgroups #101648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
gh-101581 Optionally prevent child tasks from being cancelled in asyncio taskgroups #101648
Conversation
`asyncio.TaskGroup` cancels all child tasks when one raises. Setting `defer_errors=True` will prevent this
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
…ont_abort_on_exception
Add explanation about `defer_errors=True` bheaviour in `asyncio.TaskGroup` docstring
@gvanrossum @kumaraditya303 (as participants of the parent issue) @1st1, @asvetlov, @graingert (as asyncio experts) |
@@ -198,7 +201,8 @@ def _on_task_done(self, task): | ||
return | ||
|
||
self._errors.append(exc) | ||
if self._is_base_error(exc) and self._base_error is None: | ||
is_base_error = self._is_base_error(exc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @DontPanicO, I'm trying to understand why is_base_error
is needed. Please help me understand why that is preferable to self._is_base_error(exc)
. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @willingc . I stored self._is_base_error(exc)
result in a variable since it's used twice: once on line 205 and once on line 238
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I've left a reply in the discourse thread, I'm concerned about potential memory leaks when a taskgroup with defer_errors=True
is long-lived (e.g., throughout the entire lifecycle of the event loop, or the "server" usage scenario) because the current implementation stores all unhandled exceptions in self._errors
for an indefinite length of time.
I'd like to have some discussion on the design of handling the exceptions for long-lived cases.
Ideas:
- Rename
defer_errors=True
toignore_errors=True
and let it drop all unhandled errors immediately to keep running.- Cons: yet another silent swallowing of exceptions...
- Replace
defer_errors=True
withdelegate_errors=handler_func
, wherehandler_func
shares the compatible interface withloop.call_exception_handler()
. If it isNone
(default), behave like the original taskgroup.- Cons: what whould be the difference to
task.add_done_callback()
by the caller?
- Cons: what whould be the difference to
- Split the behavior of "store-and-raising up exceptions as exception group" and "skipping cancellation of child tasks" as two separate options:
asyncio.TaskGroup(raise_errors: bool, cancel_together: bool)
orasyncio.TaskGroup(raise_errors: bool, run_to_completion: bool)
raise_errors=True
: stores unhandled exceptions and raises up as exception group upon exit, otherwise silently drop them.cancel_together=False
/run_to_completion=True
: does not cancel other children when it sees the first unhandled exception.
- Cons: too complicated
The current aiotools.Supervisor
works like the first one. I'm using add_done_callback()
to catch unhandled exceptions to implement safe_as_completed()
and alikes.
This is why I wanted to have a separate API to avoid confusion because the semantic is different at the first place (even though it may be technically feasible to implement as an option to TaskGroup):
- TaskGroup: all child tasks share the same destiny (always cancelled together, exception group)
- May have strong references to the child tasks
- Intended to be short-lived
- Supervisor: each child task has its own destiny (run to completion, individual exception handling)
- Should have weak references to the child tasks
- Intended to be long-lived
- Child tasks are cancelled together only when explicitly requested via
shutdown()
In fact, Supervisor (or TaskScope) is more versatile: it can be used as an underlying lower-level primitive to write higher-level coroutine utilities—that's why I've suggested a naming like "TaskScope". Even TaskGroup could be written from TaskScope with custom task callbacks or a custom exception handler that immediately calls shutdown()
upon the first exception and store-and-raise up exceptions as an exception group. #105011 demonstrates this.
+1 to the @achimnol's idea of TaskScope. We do need to figure out some details but his PR is pretty close. |
asyncio.TaskGroup
#101581