Skip to content

Commit 9668f04

Browse files
authored
Merge pull request #67590 from ktoso/wip-multi-error-group-single-leak
[TaskGroup] Fix unlock order, add missing detaches and add more assertions
2 parents c3fb137 + 1195955 commit 9668f04

7 files changed

+412
-232
lines changed

stdlib/public/Concurrency/TaskGroup.cpp

+240-132
Large diffs are not rendered by default.

stdlib/public/Concurrency/TaskPrivate.h

+2
Original file line numberDiff line numberDiff line change
@@ -781,8 +781,10 @@ struct AsyncTask::PrivateStorage {
781781

782782
// Destroy the opaque storage of the task
783783
void destroy() {
784+
#ifndef NDEBUG
784785
auto oldStatus = _status().load(std::memory_order_relaxed);
785786
assert(oldStatus.isComplete());
787+
#endif
786788

787789
this->~PrivateStorage();
788790
}

test/Concurrency/Runtime/async_taskgroup_asynciterator_semantics.swift

+6-7
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s --dump-input=always
2+
23
// REQUIRES: executable_test
34
// REQUIRES: concurrency
45
// REQUIRES: concurrency_runtime
56
// UNSUPPORTED: back_deployment_runtime
67
// UNSUPPORTED: OS=linux-gnu
78

8-
// REQUIRES: rdar86028226
9-
109
struct Boom: Error {}
1110

1211
func boom() async throws -> Int {
@@ -17,7 +16,7 @@ func boom() async throws -> Int {
1716
func test_taskGroup_next() async {
1817
let sum = await withThrowingTaskGroup(of: Int.self, returning: Int.self) { group in
1918
for n in 1...10 {
20-
group.spawn {
19+
group.addTask {
2120
return n.isMultiple(of: 3) ? try await boom() : n
2221
}
2322
}
@@ -50,7 +49,7 @@ func test_taskGroup_next() async {
5049
func test_taskGroup_for_in() async {
5150
let sum = await withThrowingTaskGroup(of: Int.self, returning: Int.self) { group in
5251
for n in 1...10 {
53-
group.spawn {
52+
group.addTask {
5453
return n.isMultiple(of: 3) ? try await boom() : n
5554
}
5655
}
@@ -81,7 +80,7 @@ func test_taskGroup_for_in() async {
8180
func test_taskGroup_asyncIterator() async {
8281
let sum = await withThrowingTaskGroup(of: Int.self, returning: Int.self) { group in
8382
for n in 1...10 {
84-
group.spawn {
83+
group.addTask {
8584
return n.isMultiple(of: 3) ? try await boom() : n
8685
}
8786
}
@@ -119,7 +118,7 @@ func test_taskGroup_asyncIterator() async {
119118
func test_taskGroup_contains() async {
120119
let sum = await withTaskGroup(of: Int.self, returning: Int.self) { group in
121120
for n in 1...4 {
122-
group.spawn {
121+
group.addTask {
123122
return n
124123
}
125124
}
@@ -128,7 +127,7 @@ func test_taskGroup_contains() async {
128127
print("three = \(three)") // CHECK: three = true
129128

130129
for n in 5...7 {
131-
group.spawn {
130+
group.addTask {
132131
return n
133132
}
134133
}
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
// RUN: %target-run-simple-swift( -Xfrontend -disable-availability-checking -parse-as-library) | %FileCheck %s --dump-input=always
22
// TODO: move to target-run-simple-leaks-swift once CI is using at least Xcode 14.3
33

4-
// rdar://109998145 - Temporarily disable this test
5-
// REQUIRES: rdar109998145
6-
74
// REQUIRES: concurrency
85
// REQUIRES: executable_test
96
// REQUIRES: concurrency_runtime
@@ -16,6 +13,37 @@
1613

1714
import _Concurrency
1815

16+
actor SimpleCountDownLatch {
17+
let from: Int
18+
var count: Int
19+
20+
var continuation: CheckedContinuation<Void, Never>?
21+
22+
init(from: Int) {
23+
self.from = from
24+
self.count = from
25+
}
26+
27+
func hit() {
28+
defer { count -= 1 }
29+
if count == 0 {
30+
fatalError("Counted down more times than expected! (From: \(from))")
31+
} else if count == 1 {
32+
continuation?.resume()
33+
}
34+
}
35+
36+
func wait() async {
37+
guard self.count > 0 else {
38+
return // we're done
39+
}
40+
41+
return await withCheckedContinuation { cc in
42+
self.continuation = cc
43+
}
44+
}
45+
}
46+
1947
final class PrintDeinit {
2048
let id: String
2149
init(id: String) {
@@ -60,96 +88,145 @@ final class SomeClass: @unchecked Sendable {
6088

6189
// NOTE: Not as StdlibUnittest/TestSuite since these types of tests are unreasonably slow to load/debug.
6290

63-
@main struct Main {
64-
static func main() async {
65-
_ = try? await withThrowingDiscardingTaskGroup() { group in
66-
group.addTask {
67-
throw Boom(id: "race-boom-class")
68-
}
91+
func testTwo() async {
92+
let latch = SimpleCountDownLatch(from: 2)
93+
94+
_ = try? await withThrowingDiscardingTaskGroup() { group in
95+
group.addTask {
96+
await latch.hit()
97+
throw Boom(id: "race-boom")
98+
}
99+
group.addTask {
100+
await latch.hit()
101+
SomeClass(id: "race-boom-class") // will be discarded
102+
}
103+
104+
return 12
105+
}
106+
107+
// since values may deinit in any order, we just assert their count basically
108+
// CHECK-DAG: deinit, id: race-boom
109+
// CHECK-DAG: deinit, id: race-boom
110+
await latch.wait()
111+
try? await Task.sleep(for: .milliseconds(300))
112+
113+
print("done") // CHECK: done
114+
}
115+
116+
func manyOk() async {
117+
let latch = SimpleCountDownLatch(from: 6)
118+
119+
_ = try? await withThrowingDiscardingTaskGroup() { group in
120+
for i in 0..<6 {
69121
group.addTask {
70-
SomeClass(id: "race-boom-class") // will be discarded
122+
await latch.hit()
123+
_ = SomeClass(id: "many-ok") // will be discarded
71124
}
72-
// since values may deinit in any order, we just assert their count basically
73-
// CHECK-DAG: deinit, id: race-boom-class
74-
// CHECK-DAG: deinit, id: race-boom-class
75-
76-
return 12
77125
}
78126

79-
// many ok
80-
_ = try? await withThrowingDiscardingTaskGroup() { group in
127+
return 12
128+
}
129+
// since values may deinit in any order, we just assert their count basically
130+
// CHECK-DAG: deinit, id: many-ok
131+
// CHECK-DAG: deinit, id: many-ok
132+
// CHECK-DAG: deinit, id: many-ok
133+
// CHECK-DAG: deinit, id: many-ok
134+
// CHECK-DAG: deinit, id: many-ok
135+
// CHECK-DAG: deinit, id: many-ok
136+
137+
await latch.wait()
138+
try? await Task.sleep(for: .milliseconds(300))
139+
140+
print("done") // CHECK: done
141+
}
142+
143+
func manyThrows() async {
144+
let latch = SimpleCountDownLatch(from: 6)
145+
146+
do {
147+
let value: Void = try await withThrowingDiscardingTaskGroup() { group in
81148
for i in 0..<6 {
82149
group.addTask {
83-
SomeClass(id: "many-ok") // will be discarded
150+
await latch.hit()
151+
throw BoomClass(id: "many-error") // will be rethrown
84152
}
85-
// since values may deinit in any order, we just assert their count basically
86-
// CHECK-DAG: deinit, id: many-ok
87-
// CHECK-DAG: deinit, id: many-ok
88-
// CHECK-DAG: deinit, id: many-ok
89-
// CHECK-DAG: deinit, id: many-ok
90-
// CHECK-DAG: deinit, id: many-ok
91-
// CHECK-DAG: deinit, id: many-ok
92153
}
93154

94-
return 12
155+
// since values may deinit in any order, we just assert their count basically
156+
// CHECK-DAG: deinit, id: many-error
157+
// CHECK-DAG: deinit, id: many-error
158+
// CHECK-DAG: deinit, id: many-error
159+
// CHECK-DAG: deinit, id: many-error
160+
// CHECK-DAG: deinit, id: many-error
161+
// CHECK-DAG: deinit, id: many-error
162+
163+
12 // must be ignored
95164
}
165+
preconditionFailure("Should throw")
166+
} catch {
167+
precondition("\(error)" == "main.BoomClass", "error was: \(error)")
168+
}
96169

97-
// many throws
98-
do {
99-
let value = try await withThrowingDiscardingTaskGroup() { group in
100-
for i in 0..<6 {
101-
group.addTask {
102-
throw BoomClass(id: "many-error") // will be rethrown
103-
}
104-
}
170+
await latch.wait()
171+
try? await Task.sleep(for: .milliseconds(300))
105172

106-
// since values may deinit in any order, we just assert their count basically
107-
// CHECK-DAG: deinit, id: many-error
108-
// CHECK-DAG: deinit, id: many-error
109-
// CHECK-DAG: deinit, id: many-error
110-
// CHECK-DAG: deinit, id: many-error
111-
// CHECK-DAG: deinit, id: many-error
112-
// CHECK-DAG: deinit, id: many-error
173+
print("done") // CHECK: done
174+
}
113175

114-
12 // must be ignored
115-
}
116-
preconditionFailure("Should throw")
117-
} catch {
118-
precondition("\(error)" == "main.BoomClass", "error was: \(error)")
176+
func manyValuesThrows() async {
177+
let latch = SimpleCountDownLatch(from: 6)
178+
179+
// many errors, many values
180+
_ = try? await withThrowingDiscardingTaskGroup() { group in
181+
group.addTask {
182+
await latch.hit()
183+
_ = SomeClass(id: "mixed-ok") // will be discarded
184+
}
185+
group.addTask {
186+
await latch.hit()
187+
_ = SomeClass(id: "mixed-ok") // will be discarded
188+
}
189+
group.addTask {
190+
await latch.hit()
191+
_ = SomeClass(id: "mixed-ok") // will be discarded
192+
}
193+
group.addTask {
194+
await latch.hit()
195+
throw Boom(id: "mixed-error")
196+
}
197+
group.addTask {
198+
await latch.hit()
199+
throw Boom(id: "mixed-error")
200+
}
201+
group.addTask {
202+
await latch.hit()
203+
throw Boom(id: "mixed-error")
119204
}
120205

121-
// many errors, many values
122-
_ = try? await withThrowingDiscardingTaskGroup() { group in
123-
group.addTask {
124-
SomeClass(id: "mixed-ok") // will be discarded
125-
}
126-
group.addTask {
127-
SomeClass(id: "mixed-ok") // will be discarded
128-
}
129-
group.addTask {
130-
SomeClass(id: "mixed-ok") // will be discarded
131-
}
132-
group.addTask {
133-
throw Boom(id: "mixed-error")
134-
}
135-
group.addTask {
136-
throw Boom(id: "mixed-error")
137-
}
138-
group.addTask {
139-
throw Boom(id: "mixed-error")
140-
}
206+
return 12
207+
}
141208

142-
// since values may deinit in any order, we just assert their count basically
143-
// three ok's
144-
// CHECK-DAG: deinit, id: mixed
145-
// CHECK-DAG: deinit, id: mixed
146-
// CHECK-DAG: deinit, id: mixed
147-
// three errors
148-
// CHECK-DAG: deinit, id: mixed
149-
// CHECK-DAG: deinit, id: mixed
150-
// CHECK-DAG: deinit, id: mixed
151-
152-
return 12
153-
}
209+
// since values may deinit in any order, we just assert their count basically
210+
// three ok's
211+
// CHECK-DAG: deinit, id: mixed
212+
// CHECK-DAG: deinit, id: mixed
213+
// CHECK-DAG: deinit, id: mixed
214+
// three errors
215+
// CHECK-DAG: deinit, id: mixed
216+
// CHECK-DAG: deinit, id: mixed
217+
// CHECK-DAG: deinit, id: mixed
218+
219+
await latch.wait()
220+
try? await Task.sleep(for: .milliseconds(300))
221+
222+
print("done") // CHECK: done
223+
}
224+
225+
@main struct Main {
226+
static func main() async {
227+
await testTwo()
228+
await manyOk()
229+
await manyThrows()
230+
await manyValuesThrows()
154231
}
155232
}

test/Concurrency/Runtime/async_taskgroup_next_not_invoked_cancelAll.swift

+9-9
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,14 @@ import Dispatch
1414
func test_skipCallingNext_butInvokeCancelAll() async {
1515
let numbers = [1, 1]
1616

17-
let result = try! await withTaskGroup(of: Int.self) { (group) async -> Int in
17+
let result = await withTaskGroup(of: Int.self) { (group) async -> Int in
1818
for n in numbers {
19-
print("group.spawn { \(n) }")
20-
group.spawn { [group] () async -> Int in
19+
print("group.addTask { \(n) }")
20+
group.addTask { [group] () async -> Int in
2121
await Task.sleep(1_000_000_000)
22-
print(" inside group.spawn { \(n) }")
23-
print(" inside group.spawn { \(n) } (group cancelled: \(group.isCancelled))")
24-
print(" inside group.spawn { \(n) } (group child task cancelled: \(Task.isCancelled))")
22+
print(" inside group.addTask { \(n) }")
23+
print(" inside group.addTask { \(n) } (group cancelled: \(group.isCancelled))")
24+
print(" inside group.addTask { \(n) } (group child task cancelled: \(Task.isCancelled))")
2525
return n
2626
}
2727
}
@@ -34,13 +34,13 @@ func test_skipCallingNext_butInvokeCancelAll() async {
3434
return 0
3535
}
3636

37-
// CHECK: group.spawn { 1 }
37+
// CHECK: group.addTask { 1 }
3838
//
3939
// CHECK: return immediately 0 (group cancelled: true)
4040
// CHECK: return immediately 0 (task cancelled: false)
4141
//
42-
// CHECK: inside group.spawn { 1 } (group cancelled: true)
43-
// CHECK: inside group.spawn { 1 } (group child task cancelled: true)
42+
// CHECK: inside group.addTask { 1 } (group cancelled: true)
43+
// CHECK: inside group.addTask { 1 } (group child task cancelled: true)
4444

4545
// CHECK: result: 0
4646
print("result: \(result)")

test/Concurrency/Runtime/async_taskgroup_next_on_pending.swift

-3
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,8 @@
44
// REQUIRES: concurrency
55
// REQUIRES: libdispatch
66

7-
// rdar://76038845
87
// REQUIRES: concurrency_runtime
98

10-
// REQUIRES: rdar75096485
11-
129
import Dispatch
1310

1411
func completeSlowly(n: Int) async -> Int {

test/Sanitizers/tsan/async_taskgroup_next.swift

-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
// RUN: %target-run-simple-swift( %import-libdispatch -parse-as-library -sanitize=thread)
22

3-
// Segfaulted in CI on TSan bot. rdar://78264164
4-
// REQUIRES: rdar78264164
5-
63
// REQUIRES: executable_test
74
// REQUIRES: concurrency
85
// REQUIRES: libdispatch

0 commit comments

Comments
 (0)