Skip to content

5.6 [Concurrency]: Cancellation of TaskGroup should not trigger cancel handler of parent task #40621

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

rokhinip
Copy link
Contributor

@rokhinip rokhinip commented Dec 18, 2021

Description:
Make sure that cancelling a task group does not invoke cancellation handler of parent task that created the group.

Risk:
Low. This does what the cancelAll API said it would do and makes sure not to invoke client handlers when it shouldn't.

Reviewed by: @ktoso

Testing:
Tests added in PR to verify this.

Original PR:
#40562

Radar-Id: rdar://problem/86346865

@rokhinip rokhinip requested a review from ktoso December 18, 2021 03:09
@rokhinip
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 3b4c14d

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 3b4c14d

handler of parent task that created the group

Change comment in TaskGroup.swift to enforce that only parent task can
call cancelAll on the group

Add tests to verify mutating of task group in child tasks will fail

Radar-Id: rdar://problem/86346865
@rokhinip rokhinip force-pushed the rokhinip/86346865-taskgroup-cancellAll-cancellationHandler-5.6 branch from 3b4c14d to 5e10d36 Compare December 21, 2021 01:54
@rokhinip
Copy link
Contributor Author

@swift-ci please smoke test

@rokhinip
Copy link
Contributor Author

@swift-ci please test

@rokhinip rokhinip merged commit 00f7685 into release/5.6 Jan 10, 2022
@rokhinip rokhinip deleted the rokhinip/86346865-taskgroup-cancellAll-cancellationHandler-5.6 branch January 10, 2022 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants