detach child task after filling in result

Author: ktoso

stdlib/public/Concurrency/DiscardingTaskGroup.swift

@@ -81,11 +81,12 @@ public func withDiscardingTaskGroup<GroupResult>(
 
   let _group = Builtin.createTaskGroupWithFlags(flags, GroupResult.self)
   var group = DiscardingTaskGroup(group: _group)
-  defer { Builtin.destroyTaskGroup(_group) }
 
   let result = await body(&group)
 
   try! await group.awaitAllRemainingTasks() // try!-safe, cannot throw since this is a non throwing group
+
+  Builtin.destroyTaskGroup(_group)
   return result
   #else
   fatalError("Swift compiler is incompatible with this SDK version")
@@ -385,7 +386,6 @@ public func withThrowingDiscardingTaskGroup<GroupResult>(
 
   let _group = Builtin.createTaskGroupWithFlags(flags, GroupResult.self)
   var group = ThrowingDiscardingTaskGroup<Error>(group: _group)
-  defer { Builtin.destroyTaskGroup(_group) }
 
   let result: GroupResult
   do {
@@ -395,11 +395,13 @@ public func withThrowingDiscardingTaskGroup<GroupResult>(
 
     try await group.awaitAllRemainingTasks(bodyError: error)
 
+    Builtin.destroyTaskGroup(_group)
     throw error
   }
 
   try await group.awaitAllRemainingTasks(bodyError: nil)
 
+  Builtin.destroyTaskGroup(_group)
   return result
   #else
   fatalError("Swift compiler is incompatible with this SDK version")

stdlib/public/Concurrency/TaskGroup.cpp

@@ -114,7 +114,7 @@ class NaiveTaskGroupQueue {
     queue = std::move(other.queue);
   }
 
-  virtual ~NaiveTaskGroupQueue() {}
+  ~NaiveTaskGroupQueue() {}
 
   bool dequeue(T &output) {
     if (queue.empty()) {
@@ -1035,7 +1035,7 @@ static void _enqueueRawError(DiscardingTaskGroup* _Nonnull group,
 // TaskGroup is locked upon entry and exit
 void AccumulatingTaskGroup::enqueueCompletedTask(AsyncTask *completedTask, bool hadErrorResult) {
   // Retain the task while it is in the queue; it must remain alive until
-  // it is found by poll.  This retain will balanced by the release in poll.
+  // it is found by poll.  This retain will be balanced by the release in poll.
   swift_retain(completedTask);
 
   _enqueueCompletedTask(&readyQueue, completedTask, hadErrorResult);
@@ -1181,9 +1181,11 @@ void DiscardingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *context)
       if (readyQueue.dequeue(readyErrorItem)) {
         switch (readyErrorItem.getStatus()) {
           case ReadyStatus::RawError:
+            SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, complete, resume with raw error:%p", readyErrorItem.getRawError(this));
             resumeWaitingTaskWithError(readyErrorItem.getRawError(this), assumed, alreadyDecrementedStatus);
             break;
           case ReadyStatus::Error:
+            SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, complete, resume with errorItem.task:%p", readyErrorItem.getTask());
             resumeWaitingTask(readyErrorItem.getTask(), assumed, /*hadErrorResult=*/true, alreadyDecrementedStatus);
             break;
           default:
@@ -1197,16 +1199,18 @@ void DiscardingTaskGroup::offer(AsyncTask *completedTask, AsyncContext *context)
     } else if (readyQueue.isEmpty()) {
       // There was no waiting task, or other tasks are still pending, so we cannot
       // it is the first error we encountered, thus we need to store it for future throwing
+      SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, enqueue child task:%p", completedTask);
       enqueueCompletedTask(completedTask, hadErrorResult);
     } else {
+      SWIFT_TASK_GROUP_DEBUG_LOG(this, "offer, complete, discard child task:%p", completedTask);
       _swift_taskGroup_detachChild(asAbstract(this), completedTask);
     }
 
     unlock();
     return;
   }
 
-  assert(!hadErrorResult);
+  assert(!hadErrorResult && "only successfully completed tasks can reach here");
   if (afterComplete.hasWaitingTask() && afterComplete.pendingTasks(this) == 0) {
     SWIFT_TASK_GROUP_DEBUG_LOG(this,
                                "offer, last pending task completed successfully, resume waitingTask with completedTask:%p",
@@ -1295,6 +1299,12 @@ void TaskGroupBase::resumeWaitingTask(
                                    "resume waiting DONE, task = %p, backup = %p, complete with = %p, status = %s",
                                    waitingTask, backup, completedTask, statusString().c_str());
 
+        auto waitingContext =
+            static_cast<TaskFutureWaitAsyncContext *>(
+                waitingTask->ResumeContext);
+
+        fillGroupNextResult(waitingContext, result);
+
         // Remove the child from the task group's running tasks list.
         // The parent task isn't currently running (we're about to wake
         // it up), so we're still synchronous with it.  We can safely
@@ -1305,12 +1315,6 @@ void TaskGroupBase::resumeWaitingTask(
         // we can't be holding its locks ourselves.
         _swift_taskGroup_detachChild(asAbstract(this), completedTask);
 
-        auto waitingContext =
-            static_cast<TaskFutureWaitAsyncContext *>(
-                waitingTask->ResumeContext);
-
-        fillGroupNextResult(waitingContext, result);
-
         _swift_tsan_acquire(static_cast<Job *>(waitingTask));
         // TODO: allow the caller to suggest an executor
         waitingTask->flagAsAndEnqueueOnExecutor(ExecutorRef::generic());
@@ -1660,8 +1664,8 @@ static void swift_taskGroup_waitAllImpl(
   context->errorResult = nullptr;
   context->successResultPointer = resultPointer;
 
-  SWIFT_TASK_GROUP_DEBUG_LOG(group, "waitAllImpl, waiting task = %p, bodyError = %p, status:%s, polled.status = %s, status.NOW=%s",
-                       waitingTask, bodyError, group->statusString().c_str(), to_string(polled.status).c_str(), group->statusString().c_str());
+//  SWIFT_TASK_GROUP_DEBUG_LOG(group, "waitAllImpl, waiting task = %p, bodyError = %p, status:%s, polled.status = %s, status.NOW=%s",
+//                       waitingTask, bodyError, group->statusString().c_str(), to_string(polled.status).c_str(), group->statusString().c_str());
 
   switch (polled.status) {
     case PollStatus::MustWait: {
@@ -1789,16 +1793,13 @@ PollResult TaskGroupBase::waitAll(SwiftError* bodyError, AsyncTask *waitingTask,
       waitingTask->flagAsSuspended();
     }
     // Put the waiting task at the beginning of the wait queue.
-    SWIFT_TASK_GROUP_DEBUG_LOG(this, "WATCH OUT, set waiter onto... waitQueue.head = %p", waitQueue.load(std::memory_order_relaxed));
-
     if (waitQueue.compare_exchange_strong(
         waitHead, waitingTask,
         /*success*/ std::memory_order_release,
         /*failure*/ std::memory_order_acquire)) {
       statusMarkWaitingAssumeRelease();
       SWIFT_TASK_GROUP_DEBUG_LOG(this, "waitAll, marked waiting status = %s", statusString().c_str());
-      unlock();
-      
+
 #if SWIFT_CONCURRENCY_TASK_TO_THREAD_MODEL
       // The logic here is paired with the logic in TaskGroupBase::offer. Once
       // we run the
@@ -1824,6 +1825,8 @@ PollResult TaskGroupBase::waitAll(SwiftError* bodyError, AsyncTask *waitingTask,
       // no ready tasks, so we must wait.
       result.status = PollStatus::MustWait;
       _swift_task_clearCurrent();
+
+      unlock();
       return result;
     } // else, try again
   }