[TaskGroup] Fix unlock order, add missing detaches and add more assertions

stdlib/public/Concurrency/TaskPrivate.h

@@ -781,8 +781,10 @@ struct AsyncTask::PrivateStorage {
 
   // Destroy the opaque storage of the task
   void destroy() {
+#ifndef NDEBUG
     auto oldStatus = _status().load(std::memory_order_relaxed);
     assert(oldStatus.isComplete());
+#endif
 
     this->~PrivateStorage();
   }

test/Concurrency/Runtime/async_taskgroup_asynciterator_semantics.swift

@@ -1,12 +1,11 @@
 // RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s --dump-input=always
+
 // REQUIRES: executable_test
 // REQUIRES: concurrency
 // REQUIRES: concurrency_runtime
 // UNSUPPORTED: back_deployment_runtime
 // UNSUPPORTED: OS=linux-gnu
 
-// REQUIRES: rdar86028226
-
 struct Boom: Error {}
 
 func boom() async throws -> Int {
@@ -17,7 +16,7 @@ func boom() async throws -> Int {
 func test_taskGroup_next() async {
   let sum = await withThrowingTaskGroup(of: Int.self, returning: Int.self) { group in
     for n in 1...10 {
-      group.spawn {
+      group.addTask {
         return n.isMultiple(of: 3) ? try await boom() : n
       }
     }
@@ -50,7 +49,7 @@ func test_taskGroup_next() async {
 func test_taskGroup_for_in() async {
   let sum = await withThrowingTaskGroup(of: Int.self, returning: Int.self) { group in
     for n in 1...10 {
-      group.spawn {
+      group.addTask {
         return n.isMultiple(of: 3) ? try await boom() : n
       }
     }
@@ -81,7 +80,7 @@ func test_taskGroup_for_in() async {
 func test_taskGroup_asyncIterator() async {
   let sum = await withThrowingTaskGroup(of: Int.self, returning: Int.self) { group in
     for n in 1...10 {
-      group.spawn {
+      group.addTask {
         return n.isMultiple(of: 3) ? try await boom() : n
       }
     }
@@ -119,7 +118,7 @@ func test_taskGroup_asyncIterator() async {
 func test_taskGroup_contains() async {
   let sum = await withTaskGroup(of: Int.self, returning: Int.self) { group in
     for n in 1...4 {
-      group.spawn {
+      group.addTask {
         return n
       }
     }
@@ -128,7 +127,7 @@ func test_taskGroup_contains() async {
     print("three = \(three)") // CHECK: three = true
 
     for n in 5...7 {
-      group.spawn {
+      group.addTask {
         return n
       }
     }

test/Concurrency/Runtime/async_taskgroup_discarding_dontLeak.swift

@@ -1,9 +1,6 @@
 // RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s --dump-input=always
 // TODO: move to target-run-simple-leaks-swift once CI is using at least Xcode 14.3
 
-// rdar://109998145 - Temporarily disable this test
-// REQUIRES: rdar109998145
-
 // REQUIRES: concurrency
 // REQUIRES: executable_test
 // REQUIRES: concurrency_runtime
@@ -16,6 +13,37 @@
 
 import _Concurrency
 
+actor SimpleCountDownLatch {
+  let from: Int
+  var count: Int
+
+  var continuation: CheckedContinuation<Void, Never>?
+
+  init(from: Int) {
+    self.from = from
+    self.count = from
+  }
+
+  func hit() {
+    defer { count -= 1 }
+    if count == 0 {
+      fatalError("Counted down more times than expected! (From: \(from))")
+    } else if count == 1 {
+      continuation?.resume()
+    }
+  }
+
+  func wait() async {
+    guard self.count > 0 else {
+      return // we're done
+    }
+
+    return await withCheckedContinuation { cc in
+      self.continuation = cc
+    }
+  }
+}
+
 final class PrintDeinit {
   let id: String
   init(id: String) {
@@ -60,96 +88,145 @@ final class SomeClass: @unchecked Sendable {
 
 // NOTE: Not as StdlibUnittest/TestSuite since these types of tests are unreasonably slow to load/debug.
 
-@main struct Main {
-  static func main() async {
-    _ = try? await withThrowingDiscardingTaskGroup() { group in
-      group.addTask {
-        throw Boom(id: "race-boom-class")
-      }
+func testTwo() async {
+  let latch = SimpleCountDownLatch(from: 2)
+
+  _ = try? await withThrowingDiscardingTaskGroup() { group in
+    group.addTask {
+      await latch.hit()
+      throw Boom(id: "race-boom")
+    }
+    group.addTask {
+      await latch.hit()
+      SomeClass(id: "race-boom-class") // will be discarded
+    }
+
+    return 12
+  }
+
+  // since values may deinit in any order, we just assert their count basically
+  // CHECK-DAG: deinit, id: race-boom
+  // CHECK-DAG: deinit, id: race-boom
+  await latch.wait()
+  try? await Task.sleep(for: .milliseconds(300))
+
+  print("done") // CHECK: done
+}
+
+func manyOk() async {
+  let latch = SimpleCountDownLatch(from: 6)
+
+  _ = try? await withThrowingDiscardingTaskGroup() { group in
+    for i in 0..<6 {
       group.addTask {
-        SomeClass(id: "race-boom-class") // will be discarded
+        await latch.hit()
+        _ = SomeClass(id: "many-ok") // will be discarded
       }
-      // since values may deinit in any order, we just assert their count basically
-      // CHECK-DAG: deinit, id: race-boom-class
-      // CHECK-DAG: deinit, id: race-boom-class
-
-      return 12
     }
 
-    // many ok
-    _ = try? await withThrowingDiscardingTaskGroup() { group in
+    return 12
+  }
+  // since values may deinit in any order, we just assert their count basically
+  // CHECK-DAG: deinit, id: many-ok
+  // CHECK-DAG: deinit, id: many-ok
+  // CHECK-DAG: deinit, id: many-ok
+  // CHECK-DAG: deinit, id: many-ok
+  // CHECK-DAG: deinit, id: many-ok
+  // CHECK-DAG: deinit, id: many-ok
+
+  await latch.wait()
+  try? await Task.sleep(for: .milliseconds(300))
+
+  print("done") // CHECK: done
+}
+
+func manyThrows() async {
+  let latch = SimpleCountDownLatch(from: 6)
+
+  do {
+    let value: Void = try await withThrowingDiscardingTaskGroup() { group in
       for i in 0..<6 {
         group.addTask {
-          SomeClass(id: "many-ok") // will be discarded
+          await latch.hit()
+          throw BoomClass(id: "many-error") // will be rethrown
         }
-        // since values may deinit in any order, we just assert their count basically
-        // CHECK-DAG: deinit, id: many-ok
-        // CHECK-DAG: deinit, id: many-ok
-        // CHECK-DAG: deinit, id: many-ok
-        // CHECK-DAG: deinit, id: many-ok
-        // CHECK-DAG: deinit, id: many-ok
-        // CHECK-DAG: deinit, id: many-ok
       }
 
-      return 12
+      // since values may deinit in any order, we just assert their count basically
+      // CHECK-DAG: deinit, id: many-error
+      // CHECK-DAG: deinit, id: many-error
+      // CHECK-DAG: deinit, id: many-error
+      // CHECK-DAG: deinit, id: many-error
+      // CHECK-DAG: deinit, id: many-error
+      // CHECK-DAG: deinit, id: many-error
+
+      12 // must be ignored
     }
+    preconditionFailure("Should throw")
+  } catch {
+    precondition("\(error)" == "main.BoomClass", "error was: \(error)")
+  }
 
-    // many throws
-    do {
-      let value = try await withThrowingDiscardingTaskGroup() { group in
-        for i in 0..<6 {
-          group.addTask {
-            throw BoomClass(id: "many-error") // will be rethrown
-          }
-        }
+  await latch.wait()
+  try? await Task.sleep(for: .milliseconds(300))
 
-        // since values may deinit in any order, we just assert their count basically
-        // CHECK-DAG: deinit, id: many-error
-        // CHECK-DAG: deinit, id: many-error
-        // CHECK-DAG: deinit, id: many-error
-        // CHECK-DAG: deinit, id: many-error
-        // CHECK-DAG: deinit, id: many-error
-        // CHECK-DAG: deinit, id: many-error
+  print("done") // CHECK: done
+}
 
-        12 // must be ignored
-      }
-      preconditionFailure("Should throw")
-    } catch {
-      precondition("\(error)" == "main.BoomClass", "error was: \(error)")
+func manyValuesThrows() async {
+  let latch = SimpleCountDownLatch(from: 6)
+
+  // many errors, many values
+  _ = try? await withThrowingDiscardingTaskGroup() { group in
+    group.addTask {
+      await latch.hit()
+      _ = SomeClass(id: "mixed-ok") // will be discarded
+    }
+    group.addTask {
+      await latch.hit()
+      _ = SomeClass(id: "mixed-ok") // will be discarded
+    }
+    group.addTask {
+      await latch.hit()
+      _ = SomeClass(id: "mixed-ok") // will be discarded
+    }
+    group.addTask {
+      await latch.hit()
+      throw Boom(id: "mixed-error")
+    }
+    group.addTask {
+      await latch.hit()
+      throw Boom(id: "mixed-error")
+    }
+    group.addTask {
+      await latch.hit()
+      throw Boom(id: "mixed-error")
     }
 
-    // many errors, many values
-    _ = try? await withThrowingDiscardingTaskGroup() { group in
-      group.addTask {
-        SomeClass(id: "mixed-ok") // will be discarded
-      }
-      group.addTask {
-        SomeClass(id: "mixed-ok") // will be discarded
-      }
-      group.addTask {
-        SomeClass(id: "mixed-ok") // will be discarded
-      }
-      group.addTask {
-        throw Boom(id: "mixed-error")
-      }
-      group.addTask {
-        throw Boom(id: "mixed-error")
-      }
-      group.addTask {
-        throw Boom(id: "mixed-error")
-      }
+    return 12
+  }
 
-      // since values may deinit in any order, we just assert their count basically
-      // three ok's
-      // CHECK-DAG: deinit, id: mixed
-      // CHECK-DAG: deinit, id: mixed
-      // CHECK-DAG: deinit, id: mixed
-      // three errors
-      // CHECK-DAG: deinit, id: mixed
-      // CHECK-DAG: deinit, id: mixed
-      // CHECK-DAG: deinit, id: mixed
-
-      return 12
-    }
+  // since values may deinit in any order, we just assert their count basically
+  // three ok's
+  // CHECK-DAG: deinit, id: mixed
+  // CHECK-DAG: deinit, id: mixed
+  // CHECK-DAG: deinit, id: mixed
+  // three errors
+  // CHECK-DAG: deinit, id: mixed
+  // CHECK-DAG: deinit, id: mixed
+  // CHECK-DAG: deinit, id: mixed
+
+  await latch.wait()
+  try? await Task.sleep(for: .milliseconds(300))
+
+  print("done") // CHECK: done
+}
+
+@main struct Main {
+  static func main() async {
+    await testTwo()
+    await manyOk()
+    await manyThrows()
+    await manyValuesThrows()
   }
 }

test/Concurrency/Runtime/async_taskgroup_next_not_invoked_cancelAll.swift

@@ -14,14 +14,14 @@ import Dispatch
 func test_skipCallingNext_butInvokeCancelAll() async {
   let numbers = [1, 1]
 
-  let result = try! await withTaskGroup(of: Int.self) { (group) async -> Int in
+  let result = await withTaskGroup(of: Int.self) { (group) async -> Int in
     for n in numbers {
-      print("group.spawn { \(n) }")
-      group.spawn { [group] () async -> Int in
+      print("group.addTask { \(n) }")
+      group.addTask { [group] () async -> Int in
         await Task.sleep(1_000_000_000)
-        print("  inside group.spawn { \(n) }")
-        print("  inside group.spawn { \(n) } (group cancelled: \(group.isCancelled))")
-        print("  inside group.spawn { \(n) } (group child task cancelled: \(Task.isCancelled))")
+        print("  inside group.addTask { \(n) }")
+        print("  inside group.addTask { \(n) } (group cancelled: \(group.isCancelled))")
+        print("  inside group.addTask { \(n) } (group child task cancelled: \(Task.isCancelled))")
         return n
       }
     }
@@ -34,13 +34,13 @@ func test_skipCallingNext_butInvokeCancelAll() async {
     return 0
   }
 
-  // CHECK: group.spawn { 1 }
+  // CHECK: group.addTask { 1 }
   //
   // CHECK: return immediately 0 (group cancelled: true)
   // CHECK: return immediately 0 (task cancelled: false)
   //
-  // CHECK: inside group.spawn { 1 } (group cancelled: true)
-  // CHECK: inside group.spawn { 1 } (group child task cancelled: true)
+  // CHECK: inside group.addTask { 1 } (group cancelled: true)
+  // CHECK: inside group.addTask { 1 } (group child task cancelled: true)
 
   // CHECK: result: 0
   print("result: \(result)")

test/Concurrency/Runtime/async_taskgroup_next_on_pending.swift

@@ -4,11 +4,8 @@
 // REQUIRES: concurrency
 // REQUIRES: libdispatch
 
-// rdar://76038845
 // REQUIRES: concurrency_runtime
 
-// REQUIRES: rdar75096485
-
 import Dispatch
 
 func completeSlowly(n: Int) async -> Int {

test/Sanitizers/tsan/async_taskgroup_next.swift

@@ -1,8 +1,5 @@
 // RUN: %target-run-simple-swift( %import-libdispatch -parse-as-library -sanitize=thread)
 
-// Segfaulted in CI on TSan bot. rdar://78264164
-// REQUIRES: rdar78264164
-
 // REQUIRES: executable_test
 // REQUIRES: concurrency
 // REQUIRES: libdispatch