Disabled isolated deinit for SWIFT_CONCURRENCY_ACTORS_AS_LOCKS in the runtime

Author: nickolas-pohilets

stdlib/public/Concurrency/Actor.cpp

@@ -2257,6 +2257,24 @@ static void swift_task_deinitOnExecutorImpl(void *object,
                                             DeinitWorkFunction *work,
                                             SerialExecutorRef newExecutor,
                                             size_t rawFlags) {
+#if SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
+  // To properly support isolated deinit in this mode, we
+  // need to be able to postpone deallocation of the lock
+  // until actor is unlocked.
+  //
+  // Note that such zombie state probably should be supported regardless of the isolated deinit.
+  // Theoretically it is possible for last release to happen in the middle of a job isolated to the actor.
+  // But until isolated consuming parameters are fixed, this seems to be impossible to reproduce.
+  // See also https://github.com/swiftlang/swift/issues/76083
+  //
+  // Alternatively we could lock and unlock before executing deinit body.
+  // This would be sufficient to wait until all jobs isolated to the actor have finished.
+  // Any code attempting to take actor's lock while deinit is running is incorrect anyway.
+  // So it does not matter much if deinit body is executed with lock held or not.
+  //
+  // But this workaround applies only to the isolated deinit and does not solve the generic case.
+  swift_Concurrency_fatalError(0, "Isolated deinit is not yet supported in actor as locks model");
+#else
   // If the current executor is compatible with running the new executor,
   // we can just immediately continue running with the resume function
   // we were passed in.
@@ -2271,12 +2289,7 @@ static void swift_task_deinitOnExecutorImpl(void *object,
     return work(object); // 'return' forces tail call
   }
 
-#if SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
-  // In this mode taking actor lock is the only possible implementation
-#else
-  // Otherwise, it is an optimisation applied when deinitializing default actors
   if (newExecutor.isDefaultActor() && object == newExecutor.getIdentity()) {
-#endif
     // Try to take the lock. This should always succeed, unless someone is
     // running the actor using unsafe unowned reference.
     if (asImpl(newExecutor.getDefaultActor())->tryLock(false)) {
@@ -2316,12 +2329,7 @@ static void swift_task_deinitOnExecutorImpl(void *object,
       // Give up the current actor.
       asImpl(newExecutor.getDefaultActor())->unlock(true);
       return;
-    } else {
-#if SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
-      assert(false && "Should not enqueue onto default actor in actor as locks model");
-#endif
     }
-#if !SWIFT_CONCURRENCY_ACTORS_AS_LOCKS
   }
 
   auto currentTask = swift_task_getCurrent();

test/Concurrency/Runtime/actor_recursive_deinit.swift

@@ -5,6 +5,7 @@
 
 // REQUIRES: concurrency_runtime
 // UNSUPPORTED: back_deployment_runtime
+// UNSUPPORTED: freestanding
 
 import Swift
 import _Concurrency
@@ -34,12 +35,12 @@ func isMainThread() -> Bool {
 @_silgen_name("swift_task_isCurrentExecutor")
 private func isCurrentExecutor(_ executor: Builtin.Executor) -> Bool
 
-func getExecutor(_ a: AnyActor) -> Builtin.Executor {
-  let pack = (a, UnsafeRawPointer?.none)
+func getExecutor(_ a: any Actor) -> Builtin.Executor {
+  let pack: (AnyObject, UnsafeRawPointer?) = (a, UnsafeRawPointer?.none)
   return unsafeBitCast(pack, to: Builtin.Executor.self)
 }
 
-func isCurrent(_ a: AnyActor) -> Bool {
+func isCurrent(_ a: any Actor) -> Bool {
   return isCurrentExecutor(getExecutor(a))
 }
 
@@ -53,7 +54,7 @@ actor Foo {
     }
 
     isolated deinit {
-        print("DEINIT: \(name) isolated:\(isCurrent(self)) mainThread:\(isMainThread())")
+      print("DEINIT: \(name) isolated:\(isCurrent(self)) mainThread:\(isMainThread())")
     }
 }
 

test/Concurrency/Runtime/async_task_locals_isolated_deinit_freestanding.swift

@@ -0,0 +1,89 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift -plugin-path %swift-plugin-dir -Xfrontend -disable-availability-checking -parse-stdlib %s -Xfrontend -concurrency-model=task-to-thread -g -o %t/a.out
+// RUN: %target-codesign %t/a.out
+// RUN: %env-SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE=swift6 %target-run %t/a.out
+
+// REQUIRES: freestanding
+// REQUIRES: executable_test
+// REQUIRES: concurrency
+// REQUIRES: concurrency_runtime
+
+// XFAIL: freestanding
+
+@_spi(_TaskToThreadModel) import _Concurrency
+import StdlibUnittest
+import Darwin
+import Swift
+
+@_silgen_name("swift_task_isCurrentExecutor")
+private func isCurrentExecutor(_ executor: Builtin.Executor) -> Bool
+
+private func isCurrentExecutor(_ executor: UnownedSerialExecutor) -> Bool {
+    isCurrentExecutor(unsafeBitCast(executor, to: Builtin.Executor.self))
+}
+
+@available(SwiftStdlib 5.1, *)
+struct TL {
+  @TaskLocal
+  static var number: Int = 0
+}
+
+func checkTaskLocalStack() {
+  TL.$number.withValue(-999) {
+    expectEqual(-999, TL.number)
+  }
+}
+
+actor ActorNoOp {
+  let expectedNumber: Int
+  let probe: Probe
+
+  init(expectedNumber: Int) {
+    self.expectedNumber = expectedNumber
+    self.probe = Probe(expectedNumber: expectedNumber)
+    self.probe.probeExpectedExecutor = self.unownedExecutor
+  }
+
+  isolated deinit {
+    expectTrue(isCurrentExecutor(self.unownedExecutor))
+    expectEqual(expectedNumber, TL.number)
+    checkTaskLocalStack()
+  }
+}
+
+class Probe {
+  var probeExpectedExecutor: UnownedSerialExecutor!
+  let probeExpectedNumber: Int
+  
+  init(expectedNumber: Int) {
+    self.probeExpectedExecutor = nil
+    self.probeExpectedNumber = expectedNumber
+  }
+
+  deinit {
+    expectTrue(isCurrentExecutor(probeExpectedExecutor))
+    expectEqual(probeExpectedNumber, TL.number)
+    checkTaskLocalStack()
+  }
+}
+
+let tests = TestSuite("Isolated Deinit")
+
+if #available(SwiftStdlib 5.1, *) {
+  tests.test("actor sync not isolated") {
+    Task.runInline {
+      TL.$number.withValue(99) {
+        // Freestanding runtime always executes deinit inline
+        _ = ActorNoOp(expectedNumber: 99)
+      }
+    }
+  }
+
+  tests.test("no TLs") {
+    Task.runInline {
+      _ = ActorNoOp(expectedNumber: 0)
+    }
+  }
+}
+
+runAllTests()