-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Docs][Concurrency] TaskGroup docs: waitForAll (non-)cancellation #63956
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
Conversation
@swift-ci please smoke test |
While in here is potted some wrong documentation... let me fix that. |
2711e37
to
29b9251
Compare
c06b05d
to
7a67318
Compare
/// ``` | ||
/// | ||
/// - Throws: The *first* error that was thrown by a child task during draining all the tasks. | ||
/// This first error is stored until all other tasks have completed, and is re-thrown afterwards. |
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.
This is the only instance of waitForAll which has such special semantics.
Discarding groups don't have this API, nor can they use next()
, and a not throwing one cannot throw so this cannot happen
@swift-ci please smoke test |
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.
Thanks for clarifying this!
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.
Drive-by review after merging...
/// } catch is CancellationError { | ||
/// // we decide that cancellation errors thrown by children, | ||
/// // should not cause cancellation of the entire group. | ||
/// continue; |
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.
👽 ;
/// // should not cause cancellation of the entire group. | ||
/// continue; | ||
/// } catch { | ||
/// // other errors though we print and cancel the group, |
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.
We do not really print, I guess.
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.
Thanks will add
Resolves rdar://105973001
This behavior is expected, however was not documented so a bug fix of the group previously not properly awaiting all tasks now made this behavior surprising. The solution is not to change the behavior, but to make it documented.
Please review @FranzBusch