Merge pull request #36359 from kavon/actor-lifetimes

Author: DougGregor

stdlib/public/Concurrency/Actor.cpp

@@ -1094,8 +1094,15 @@ Job *DefaultActorImpl::claimNextJobOrGiveUp(bool actorIsOwned,
 /// the current thread, and keep processing whatever actor we're
 /// running when code returns back to us until we're not processing
 /// any actors anymore.
+///
+/// \param currentActor is expected to be passed in as retained to ensure that
+///                     the actor lives for the duration of job execution.
+///                     Note that this may conflict with the retain/release
+///                     design in the DefaultActorImpl, but it does fix bugs!
 static void processDefaultActor(DefaultActorImpl *currentActor,
                                 RunningJobInfo runner) {
+  DefaultActorImpl *actor = currentActor;
+
   // Register that we're processing a default actor in this frame.
   ExecutorTrackingInfo trackingInfo;
   auto activeTrackingInfo = trackingInfo.enterOrUpdate(
@@ -1142,6 +1149,8 @@ static void processDefaultActor(DefaultActorImpl *currentActor,
   // If we still have an active actor, we should give it up.
   if (currentActor)
     currentActor->giveUpThread(runner);
+
+  swift_release(actor);
 }
 
 void ProcessInlineJob::process(Job *job, ExecutorRef _executor) {
@@ -1153,6 +1162,7 @@ void ProcessInlineJob::process(Job *job, ExecutorRef _executor) {
   auto runner = RunningJobInfo::forInline(targetPriority);
 
   // FIXME: force tail call
+  swift_retain(actor);
   return processDefaultActor(actor, runner);
 }
 
@@ -1168,6 +1178,7 @@ void ProcessOutOfLineJob::process(Job *job, ExecutorRef _executor) {
   delete self;
 
   // FIXME: force tail call
+  swift_retain(actor);
   return processDefaultActor(actor, runner);
 }
 
@@ -1179,6 +1190,7 @@ void ProcessOverrideJob::process(Job *job, ExecutorRef _executor) {
   auto runner = RunningJobInfo::forOverride(self);
 
   // FIXME: force tail call
+  swift_retain(actor);
   return processDefaultActor(actor, runner);
 }
 

test/Concurrency/Runtime/executor_deinit1.swift

@@ -0,0 +1,32 @@
+// RUN: %target-run-simple-swift(-parse-as-library -Xfrontend -enable-experimental-concurrency %import-libdispatch) | %FileCheck %s
+
+// REQUIRES: executable_test
+// REQUIRES: concurrency
+// REQUIRES: libdispatch
+
+// FIXME: this should pass! from rdar://73266050
+// XFAIL: *
+
+// doesn't matter that it's bool identity function or not
+func boolIdentityFn(_ x : Bool) -> Bool { return x }
+
+actor FirstActor {
+    func startTest() { // whether startTest is async or sync doesn't matter
+
+        // do not remove this call or if-statement.
+        if boolIdentityFn(true) {}
+
+    }
+
+    deinit() {
+        // CHECK: called deinit
+        print("called deinit")
+    }
+}
+
+@main struct RunIt {
+    static func main() async {
+        let actor = FirstActor()
+        await actor.startTest() // do not remove this call
+    }
+}

test/Concurrency/Runtime/executor_deinit2.swift

@@ -0,0 +1,32 @@
+// RUN: %target-run-simple-swift(-parse-as-library -Xfrontend -enable-experimental-concurrency %import-libdispatch) | %FileCheck %s
+
+// REQUIRES: executable_test
+// REQUIRES: concurrency
+// REQUIRES: libdispatch
+
+// this needs to match with the check count below.
+let NUM_TASKS : Int = 100
+
+final class Capture : ConcurrentValue {
+    func doSomething() { }
+    deinit {
+        // CHECK-COUNT-100: deinit was called!
+        print("deinit was called!")
+    }
+}
+
+@main
+struct App {
+    static func main() async {
+        var n = 0
+        for _ in 1...NUM_TASKS {
+            let c = Capture()
+            let r = Task.runDetached {
+                c.doSomething()
+            }
+            await r.get()
+            n += 1
+        }
+        print("test complete")
+    }
+}

test/Concurrency/Runtime/executor_deinit3.swift

@@ -0,0 +1,76 @@
+// RUN: %target-run-simple-swift(-parse-as-library -Xfrontend -enable-experimental-concurrency %import-libdispatch) | %FileCheck %s
+
+// REQUIRES: executable_test
+// REQUIRES: concurrency
+// REQUIRES: libdispatch
+
+// for sleep
+#if canImport(Darwin)
+    import Darwin
+#elseif canImport(Glibc)
+    import Glibc
+#endif
+
+class Runner {
+    func run() async {
+        while !Task.isCancelled {
+            sleep(1)
+        }
+    }
+}
+
+actor Container {
+    var generation = 0
+    var runners = [Int : Task.Handle<Void, Never>]()
+
+    func build(_ n: Int) {
+        for _ in 0..<n {
+            let id = generation
+            generation += 1
+            let t = Task.runDetached { [weak self] in
+                let r = Runner()
+                await r.run()
+                await self?.remove(id)
+            }
+            runners[id] = t
+        }
+    }
+
+    func cancelAll() {
+        var count = 0
+        for (_, v) in runners {
+            v.cancel()
+            count += 1
+        }
+        print("Cancelled \(count) runners.")
+    }
+
+    deinit {
+        print("deinit Container with \(runners.count) runners")
+    }
+
+    func remove(_ id: Int) {
+        runners.removeValue(forKey: id)
+    }
+}
+
+// CHECK: starting
+// CHECK: Cancelled 5 runners.
+
+// FIXME: this doesn't work until we have https://github.com/apple/swift/pull/36298
+// COM: deinit Container with {{[0-9]+}} runners
+
+@main struct RunIt {
+    static func startTest() async {
+        let c = Container()
+        await c.build(5)
+        sleep(5)
+        await c.cancelAll()
+    }
+
+static func main() async {
+        print("starting")
+        await RunIt.startTest()
+        sleep(5)
+    }
+}