Skip to content

Commit 411cdb7

Browse files
nickolas-pohiletsnickolas-pohilets
committed
Added unit-tests for (not)copying task-local values. Fixed release after deallocation.
1 parent 95079af commit 411cdb7

File tree

3 files changed

+229
-5
lines changed

3 files changed

+229
-5
lines changed

‎stdlib/public/Concurrency/Actor.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,8 +1576,6 @@ static void defaultActorDrain(DefaultActorImpl *actor) {
15761576
#if SWIFT_CONCURRENCY_ENABLE_PRIORITY_ESCALATION
15771577
done:
15781578
#endif
1579-
// Balances with the retain taken in ProcessOutOfLineJob::process
1580-
swift_release(actor);
15811579
}
15821580

15831581
SWIFT_CC(swiftasync)
@@ -1586,9 +1584,6 @@ void ProcessOutOfLineJob::process(Job *job) {
15861584
DefaultActorImpl *actor = self->Actor;
15871585

15881586
delete self;
1589-
1590-
// Balances with the swift_release in defaultActorDrain()
1591-
swift_retain(actor);
15921587
return defaultActorDrain(actor); // 'return' forces tail call
15931588
}
15941589

‎test/Concurrency/Runtime/async_task_locals_isolated_deinit.swift

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// RUN: %target-run-simple-swift( -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-stdlib %import-libdispatch)
2+
3+
// REQUIRES: executable_test
4+
// REQUIRES: concurrency
5+
6+
import Swift
7+
import _Concurrency
8+
import Dispatch
9+
import StdlibUnittest
10+
11+
@_silgen_name("swift_task_isCurrentExecutor")
12+
private func isCurrentExecutor(_ executor: Builtin.Executor) -> Bool
13+
14+
private func isCurrentExecutor(_ executor: UnownedSerialExecutor) -> Bool {
15+
isCurrentExecutor(unsafeBitCast(executor, to: Builtin.Executor.self))
16+
}
17+
18+
extension DispatchGroup {
19+
func enter(_ count: Int) {
20+
for _ in 0..<count {
21+
self.enter()
22+
}
23+
}
24+
}
25+
26+
@available(SwiftStdlib 5.1, *)
27+
struct TL {
28+
@TaskLocal
29+
static var number: Int = 0
30+
}
31+
32+
func checkTaskLocalStack() {
33+
TL.$number.withValue(-999) {
34+
expectEqual(-999, TL.number)
35+
}
36+
}
37+
38+
actor ActorNoOp {
39+
let expectedNumber: Int
40+
let group: DispatchGroup
41+
let probe: Probe
42+
43+
init(expectedNumber: Int, group: DispatchGroup) {
44+
self.expectedNumber = expectedNumber
45+
self.group = group
46+
self.probe = Probe(expectedNumber: expectedNumber, group: group)
47+
self.probe.probeExpectedExecutor = self.unownedExecutor
48+
}
49+
50+
deinit {
51+
expectTrue(isCurrentExecutor(self.unownedExecutor))
52+
expectEqual(expectedNumber, TL.number)
53+
checkTaskLocalStack()
54+
group.leave()
55+
}
56+
}
57+
58+
@globalActor actor AnotherActor: GlobalActor {
59+
static let shared = AnotherActor()
60+
61+
func performTesting(_ work: @Sendable () -> Void) {
62+
work()
63+
}
64+
}
65+
66+
class Probe {
67+
var probeExpectedExecutor: UnownedSerialExecutor
68+
let probeExpectedNumber: Int
69+
let probeGroup: DispatchGroup
70+
71+
init(expectedNumber: Int, group: DispatchGroup) {
72+
self.probeExpectedExecutor = AnotherActor.shared.unownedExecutor
73+
self.probeExpectedNumber = expectedNumber
74+
self.probeGroup = group
75+
group.enter()
76+
}
77+
78+
deinit {
79+
expectTrue(isCurrentExecutor(probeExpectedExecutor))
80+
expectEqual(probeExpectedNumber, TL.number)
81+
checkTaskLocalStack()
82+
probeGroup.leave()
83+
}
84+
}
85+
86+
class ClassNoOp: Probe {
87+
let expectedNumber: Int
88+
let group: DispatchGroup
89+
let probe: Probe
90+
91+
override init(expectedNumber: Int, group: DispatchGroup) {
92+
self.expectedNumber = expectedNumber
93+
self.group = group
94+
self.probe = Probe(expectedNumber: expectedNumber, group: group)
95+
super.init(expectedNumber: expectedNumber, group: group)
96+
}
97+
98+
@AnotherActor
99+
deinit {
100+
expectTrue(isCurrentExecutor(AnotherActor.shared.unownedExecutor))
101+
expectEqual(expectedNumber, TL.number)
102+
checkTaskLocalStack()
103+
group.leave()
104+
}
105+
}
106+
107+
let tests = TestSuite("Isolated Deinit")
108+
109+
if #available(SwiftStdlib 5.1, *) {
110+
tests.test("fast path") {
111+
let group = DispatchGroup()
112+
group.enter(1)
113+
Task {
114+
await TL.$number.withValue(42) {
115+
await AnotherActor.shared.performTesting {
116+
_ = ClassNoOp(expectedNumber: 42, group: group)
117+
}
118+
}
119+
}
120+
group.wait()
121+
}
122+
123+
tests.test("slow path") {
124+
let group = DispatchGroup()
125+
group.enter(2)
126+
Task {
127+
TL.$number.withValue(37) {
128+
_ = ActorNoOp(expectedNumber: 0, group: group)
129+
}
130+
TL.$number.withValue(99) {
131+
_ = ClassNoOp(expectedNumber: 0, group: group)
132+
}
133+
}
134+
group.wait()
135+
}
136+
137+
tests.test("no TLs") {
138+
let group = DispatchGroup()
139+
group.enter(2)
140+
Task {
141+
_ = ActorNoOp(expectedNumber: 0, group: group)
142+
_ = ClassNoOp(expectedNumber: 0, group: group)
143+
}
144+
group.wait()
145+
}
146+
}
147+
148+
runAllTests()
149+

‎test/Concurrency/voucher_propagation.swift

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,51 @@ actor Counter {
7575
func get() -> Int { n }
7676
}
7777

78+
actor ActorWithIsolatedDeinit {
79+
let expectedVoucher: voucher_t?
80+
let group: DispatchGroup
81+
82+
init(expectedVoucher: voucher_t?, group: DispatchGroup) {
83+
self.expectedVoucher = expectedVoucher
84+
self.group = group
85+
}
86+
87+
deinit {
88+
expectTrue(isCurrentExecutor(self.unownedExecutor))
89+
let currentVoucher = voucher_copy()
90+
expectEqual(expectedVoucher, currentVoucher)
91+
os_release(currentVoucher)
92+
group.leave()
93+
}
94+
}
95+
96+
@globalActor actor AnotherActor: GlobalActor {
97+
static let shared = AnotherActor()
98+
99+
func performTesting(_ work: @Sendable () -> Void) {
100+
work()
101+
}
102+
}
103+
104+
class ClassWithIsolatedDeinit {
105+
let expectedVoucher: voucher_t?
106+
let group: DispatchGroup
107+
108+
init(expectedVoucher: voucher_t?, group: DispatchGroup) {
109+
self.expectedVoucher = expectedVoucher
110+
self.group = group
111+
}
112+
113+
@AnotherActor
114+
deinit {
115+
expectTrue(isCurrentExecutor(AnotherActor.shared.unownedExecutor))
116+
let currentVoucher = voucher_copy()
117+
expectEqual(expectedVoucher, currentVoucher)
118+
os_release(currentVoucher)
119+
group.leave()
120+
}
121+
}
122+
78123
// Make a nice string for a pointer, like what %p would produce in printf.
79124
func ptrstr<T>(_ ptr: T) -> String {
80125
"0x" + String(unsafeBitCast(ptr, to: UInt.self), radix: 16)
@@ -110,6 +155,8 @@ let voucher_adopt = lookup("voucher_adopt") as @convention(c) (voucher_t?)
110155
let os_retain = lookup("os_retain") as @convention(c) (voucher_t?) -> voucher_t?
111156
let os_release = lookup("os_release") as @convention(c) (voucher_t?) -> Void
112157

158+
let isCurrentExecutor = lookup("swift_task_isCurrentExecutor") as @convention(thin) (UnownedSerialExecutor) -> Bool
159+
113160
// Run some async code with test vouchers. Wait for the async code to complete,
114161
// then verify that the vouchers aren't leaked.
115162
func withVouchers(call: @Sendable @escaping (voucher_t?, voucher_t?, voucher_t?)
@@ -360,6 +407,39 @@ if #available(SwiftStdlib 5.1, *) {
360407
group.wait()
361408
}
362409
}
410+
411+
tests.test("voucher propagation in isolated deinit [fast path]") {
412+
withVouchers { v1, v2, v3 in
413+
let group = DispatchGroup()
414+
group.enter()
415+
Task {
416+
await AnotherActor.shared.performTesting {
417+
adopt(voucher: v1)
418+
_ = ClassWithIsolatedDeinit(expectedVoucher: v1, group: group)
419+
}
420+
}
421+
group.wait()
422+
}
423+
}
424+
425+
tests.test("voucher propagation in isolated deinit [slow path]") {
426+
withVouchers { v1, v2, v3 in
427+
let group = DispatchGroup()
428+
group.enter()
429+
group.enter()
430+
Task {
431+
do {
432+
adopt(voucher: v2)
433+
_ = ActorWithIsolatedDeinit(expectedVoucher: v2, group: group)
434+
}
435+
do {
436+
adopt(voucher: v3)
437+
_ = ClassWithIsolatedDeinit(expectedVoucher: v3, group: group)
438+
}
439+
}
440+
group.wait()
441+
}
442+
}
363443
}
364444

365445
runAllTests()

0 commit comments

Comments
 (0)