Skip to content

Commit 94423da

Browse files
committed
[Concurrency] Refactor _swift_taskGroup_cancelAllChildren -> _cancel
This way we do the right thing always when cancelling the group; and we MAY visit the child tasks if we have to.
1 parent 80f3849 commit 94423da

File tree

3 files changed

+16
-20
lines changed

3 files changed

+16
-20
lines changed

stdlib/public/Concurrency/TaskGroup.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2120,8 +2120,8 @@ bool TaskGroupBase::cancelAll(AsyncTask *owningTask) {
21202120

21212121
// Cancel all the child tasks. TaskGroup is not a Sendable type,
21222122
// so cancelAll() can only be called from the owning task. This
2123-
// satisfies the precondition on cancelAllChildren_unlocked().
2124-
_swift_taskGroup_cancelAllChildren_unlocked(asAbstract(this), owningTask);
2123+
// satisfies the precondition on cancel_unlocked().
2124+
_swift_taskGroup_cancel_unlocked(asAbstract(this), owningTask);
21252125

21262126
return true;
21272127
}
@@ -2130,8 +2130,8 @@ SWIFT_CC(swift)
21302130
static void swift_task_cancel_group_child_tasksImpl(TaskGroup *group) {
21312131
// TaskGroup is not a Sendable type, and so this operation (which is not
21322132
// currently exposed in the API) can only be called from the owning
2133-
// task. This satisfies the precondition on cancelAllChildren_unlocked().
2134-
_swift_taskGroup_cancelAllChildren_unlocked(group, swift_task_getCurrent());
2133+
// task. This satisfies the precondition on cancel_unlocked().
2134+
_swift_taskGroup_cancel_unlocked(group, swift_task_getCurrent());
21352135
}
21362136

21372137
// =============================================================================

stdlib/public/Concurrency/TaskPrivate.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,16 @@ AsyncTask *_swift_task_clearCurrent();
8989
/// Set the active task reference for the current thread.
9090
AsyncTask *_swift_task_setCurrent(AsyncTask *newTask);
9191

92-
/// Cancel all the child tasks that belong to `group`.
92+
/// Cancel the task group and all the child tasks that belong to `group`.
9393
///
9494
/// The caller must guarantee that this is called while holding the owning
9595
/// task's status record lock.
96-
void _swift_taskGroup_cancelAllChildren(TaskGroup *group);
96+
void _swift_taskGroup_cancel(TaskGroup *group);
9797

98-
/// Cancel all the child tasks that belong to `group`.
98+
/// Cancel the task group and all the child tasks that belong to `group`.
9999
///
100100
/// The caller must guarantee that this is called from the owning task.
101-
void _swift_taskGroup_cancelAllChildren_unlocked(TaskGroup *group,
101+
void _swift_taskGroup_cancel_unlocked(TaskGroup *group,
102102
AsyncTask *owningTask);
103103

104104
/// Remove the given task from the given task group.

stdlib/public/Concurrency/TaskStatus.cpp

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -776,12 +776,13 @@ void swift::_swift_taskGroup_detachChild(TaskGroup *group,
776776
});
777777
}
778778

779-
/// Cancel all the child tasks that belong to `group`.
779+
/// Cancel the task group and all the child tasks that belong to `group`.
780780
///
781781
/// The caller must guarantee that this is called while holding the owning
782782
/// task's status record lock.
783-
void swift::_swift_taskGroup_cancelAllChildren(TaskGroup *group) {
784-
assert(group->isCancelled() && "Expected task group to be cancelled when cancelling all child tasks.");
783+
void swift::_swift_taskGroup_cancel(TaskGroup *group) {
784+
(void) group->statusCancel();
785+
785786
// Because only the owning task of the task group can modify the
786787
// child list of a task group status record, and it can only do so
787788
// while holding the owning task's status record lock, we do not need
@@ -790,10 +791,10 @@ void swift::_swift_taskGroup_cancelAllChildren(TaskGroup *group) {
790791
swift_task_cancel(childTask);
791792
}
792793

793-
/// Cancel all the child tasks that belong to `group`.
794+
/// Cancel the task group and all the child tasks that belong to `group`.
794795
///
795796
/// The caller must guarantee that this is called from the owning task.
796-
void swift::_swift_taskGroup_cancelAllChildren_unlocked(TaskGroup *group,
797+
void swift::_swift_taskGroup_cancel_unlocked(TaskGroup *group,
797798
AsyncTask *owningTask) {
798799
// Early out. If there are no children, there's nothing to do. We can safely
799800
// check this without locking, since this can only be concurrently mutated
@@ -802,7 +803,7 @@ void swift::_swift_taskGroup_cancelAllChildren_unlocked(TaskGroup *group,
802803
return;
803804

804805
withStatusRecordLock(owningTask, [&group](ActiveTaskStatus status) {
805-
_swift_taskGroup_cancelAllChildren(group);
806+
_swift_taskGroup_cancel(group);
806807
});
807808
}
808809

@@ -826,12 +827,7 @@ static void performCancellationAction(TaskStatusRecord *record) {
826827
// under the synchronous control of the task that owns the group.
827828
case TaskStatusRecordKind::TaskGroup: {
828829
auto groupRecord = cast<TaskGroupTaskStatusRecord>(record);
829-
auto group = groupRecord->getGroup();
830-
auto wasAlreadyCancelled = group->statusCancel();
831-
if (wasAlreadyCancelled) {
832-
return;
833-
}
834-
_swift_taskGroup_cancelAllChildren(group);
830+
_swift_taskGroup_cancel(groupRecord->getGroup());
835831
return;
836832
}
837833

0 commit comments

Comments
 (0)