[6.2][Concurrency] Parent task cancellation must cancel task group itself #80797
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Seems that during refactorings of child cancellations we somehow missed also cancelling the group itself. It seems we did not have good test coverage of the addTaskUnlessCancelled somehow and thus this slipped through. This adds a regression test for addTaskUnlessCancelled and fixes how we handle the cancellation effect in TaskStatus. resolves swiftlang#80789 resolves rdar://149177600
This way we do the right thing always when cancelling the group; and we MAY visit the child tasks if we have to.
ktoso
changed the title
|
@swift-ci please test |
ktoso
force-pushed
the
pick-wip-parent-cancel-must-cancel-group
branch
from
6cda4f9 to
60a1978
Compare
|
@swift-ci please test |
DougGregor
approved these changes
Apr 14, 2025
|
CI has some weird llvm issue: |
|
@swift-ci please test |
ktoso
added
concurrency
|
Flaky test? |
|
@swift-ci please test macOS |
|
@swift-ci please test Linux |
|
@swift-ci please test macOS |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description: Seems that during refactorings of child cancellations we somehow missed
also cancelling the group itself. It seems we did not have good test
coverage of the addTaskUnlessCancelled somehow and thus this slipped
through.
This would allow tasks to continue getting added even though the "parent task" was cancelled, breaking the contract of the
addTaskUnlessCancelledmethod.This adds a regression test for addTaskUnlessCancelled and fixes how we
handle the cancellation effect in TaskStatus.
Scope/Impact: Task groups whose parent task is cancelled specifically. Cancellation is idempotent, which makes this fix very safe.
Risk: Low, very low risk chance since we only flip atomically the cancelled bit which was missing.
Testing: Added regression test, confirmed the solution
Reviewed by: @al45tair
Original PR: #80794
Radar: rdar://149177600
resolves #80789