Skip to content

Commit 80f3849

Browse files
committed
[Concurrency] Parent task cancellation must cancel task group itself.
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 #80789 resolves rdar://149177600
1 parent 1f93566 commit 80f3849

File tree

4 files changed

+81
-1
lines changed

4 files changed

+81
-1
lines changed

include/swift/ABI/TaskGroup.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ class alignas(Alignment_TaskGroup) TaskGroup {
4444
/// Checks the cancellation status of the group.
4545
bool isCancelled();
4646

47+
/// Only mark the task group as cancelled, without performing the follow-up
48+
/// work of cancelling all the child tasks.
49+
///
50+
/// Returns true if the group was already cancelled before this call.
51+
bool statusCancel();
52+
4753
// Add a child task to the task group. Always called while holding the
4854
// status record lock of the task group's owning task.
4955
void addChildTask(AsyncTask *task);

stdlib/public/Concurrency/TaskGroup.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1166,6 +1166,10 @@ bool TaskGroup::isCancelled() {
11661166
return asBaseImpl(this)->isCancelled();
11671167
}
11681168

1169+
bool TaskGroup::statusCancel() {
1170+
return asBaseImpl(this)->statusCancel();
1171+
}
1172+
11691173
// =============================================================================
11701174
// ==== offer ------------------------------------------------------------------
11711175

stdlib/public/Concurrency/TaskStatus.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,7 @@ void swift::_swift_taskGroup_detachChild(TaskGroup *group,
781781
/// The caller must guarantee that this is called while holding the owning
782782
/// task's status record lock.
783783
void swift::_swift_taskGroup_cancelAllChildren(TaskGroup *group) {
784+
assert(group->isCancelled() && "Expected task group to be cancelled when cancelling all child tasks.");
784785
// Because only the owning task of the task group can modify the
785786
// child list of a task group status record, and it can only do so
786787
// while holding the owning task's status record lock, we do not need
@@ -825,7 +826,12 @@ static void performCancellationAction(TaskStatusRecord *record) {
825826
// under the synchronous control of the task that owns the group.
826827
case TaskStatusRecordKind::TaskGroup: {
827828
auto groupRecord = cast<TaskGroupTaskStatusRecord>(record);
828-
_swift_taskGroup_cancelAllChildren(groupRecord->getGroup());
829+
auto group = groupRecord->getGroup();
830+
auto wasAlreadyCancelled = group->statusCancel();
831+
if (wasAlreadyCancelled) {
832+
return;
833+
}
834+
_swift_taskGroup_cancelAllChildren(group);
829835
return;
830836
}
831837

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
// RUN: %target-run-simple-swift( -target %target-swift-5.1-abi-triple -parse-as-library) | %FileCheck %s
2+
3+
// REQUIRES: executable_test
4+
// REQUIRES: concurrency
5+
6+
// rdar://76038845
7+
// REQUIRES: concurrency_runtime
8+
// UNSUPPORTED: back_deployment_runtime
9+
10+
import Dispatch
11+
12+
@available(SwiftStdlib 6.0, *)
13+
func test_withTaskGroup_addUnlessCancelled() async throws {
14+
let task = Task {
15+
await withTaskGroup(of: Void.self) { group in
16+
print("Inner: Sleep...")
17+
try? await Task.sleep(nanoseconds: 2_000_000_000)
18+
print("Inner: Task.isCancelled: \(Task.isCancelled)")
19+
20+
let added = group.addTaskUnlessCancelled {
21+
print("Added Task! Child Task.isCancelled: \(Task.isCancelled)")
22+
}
23+
print("Inner: Task added = \(added)") // CHECK: Task added = false
24+
}
25+
}
26+
27+
try? await Task.sleep(nanoseconds: 1_000_000)
28+
print("Outer: Cancel!")
29+
task.cancel()
30+
print("Outer: Cancelled")
31+
32+
await task.value
33+
}
34+
35+
@available(SwiftStdlib 6.0, *)
36+
func test_withDiscardingTaskGroup_addUnlessCancelled() async throws {
37+
let task = Task {
38+
await withDiscardingTaskGroup { group in
39+
print("Inner: Sleep...")
40+
try? await Task.sleep(nanoseconds: 2_000_000_000)
41+
print("Inner: Task.isCancelled: \(Task.isCancelled)")
42+
43+
let added = group.addTaskUnlessCancelled {
44+
print("Added Task! Child Task.isCancelled: \(Task.isCancelled)")
45+
}
46+
print("Inner: Task added = \(added)") // CHECK: Task added = false
47+
}
48+
}
49+
50+
try? await Task.sleep(nanoseconds: 1_000_000)
51+
print("Outer: Cancel!")
52+
task.cancel()
53+
print("Outer: Cancelled")
54+
55+
await task.value
56+
}
57+
58+
@available(SwiftStdlib 6.0, *)
59+
@main struct Main {
60+
static func main() async {
61+
try! await test_withTaskGroup_addUnlessCancelled()
62+
try! await test_withDiscardingTaskGroup_addUnlessCancelled()
63+
}
64+
}

0 commit comments

Comments
 (0)