Merge pull request #58656 from ktoso/wip-init-crash

[Distributed][Workaround] Initializers crash when DA has state and uses the local testing system

Author: swift-ci

lib/AST/DistributedDecl.cpp

@@ -472,7 +472,6 @@ bool AbstractFunctionDecl::isDistributedActorSystemRemoteCall(bool isVoidReturn)
   if (!invocationParam->isInOut()) {
     return false;
   }
-
   // --- Check parameter: throwing: Err.Type
   auto thrownTypeParam = params->get(3);
   if (thrownTypeParam->getArgumentName() != C.Id_throwing) {

stdlib/public/Distributed/DistributedActorSystem.swift

@@ -12,7 +12,12 @@
 import Swift
 import _Concurrency
 
-/// A distributed actor system
+/// A distributed actor system is what enables all functionality of distributed actors.
+///
+/// Distributed actors must always be associated with a concrete distributed actor system,
+/// and do so by declaring a `typealias ActorSystem = ...`, or by having a module wide
+/// `typealias DefaultDistributedActorSystem` declared which then applies to all distributed
+/// actors that do not declare a specific type alias in their bodies.
 @available(SwiftStdlib 5.7, *)
 public protocol DistributedActorSystem: Sendable {
   /// The identity used by actors that communicate via this transport

stdlib/public/Distributed/LocalTestingDistributedActorSystem.swift

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import Swift
+import _Concurrency
 
 #if canImport(Darwin)
 import Darwin
@@ -26,25 +27,33 @@ import WinSDK
 /// for learning about `distributed actor` isolation, as well as early
 /// prototyping stages of development where a real system is not necessary yet.
 @available(SwiftStdlib 5.7, *)
-public final class LocalTestingDistributedActorSystem: DistributedActorSystem, @unchecked Sendable {
+public struct LocalTestingDistributedActorSystem: DistributedActorSystem, @unchecked Sendable {
   public typealias ActorID = LocalTestingActorAddress
   public typealias ResultHandler = LocalTestingInvocationResultHandler
   public typealias InvocationEncoder = LocalTestingInvocationEncoder
   public typealias InvocationDecoder = LocalTestingInvocationDecoder
-  public typealias SerializationRequirement = Codable
+  public typealias SerializationRequirement = any Codable
+
 
-  private var activeActors: [ActorID: any DistributedActor] = [:]
-  private let activeActorsLock = _Lock()
+  @usableFromInline
+  final class _Storage {
 
-  private var idProvider: ActorIDProvider = ActorIDProvider()
-  private var assignedIDs: Set<ActorID> = []
-  private let assignedIDsLock = _Lock()
+    var activeActors: [ActorID: any DistributedActor] = [:]
+    let activeActorsLock = _Lock()
 
-  public init() {}
+    var assignedIDs: Set<ActorID> = []
+    let assignedIDsLock = _Lock()
+  }
+  let storage: _Storage
+  var idProvider: ActorIDProvider = ActorIDProvider()
+
+  public init() {
+    storage = .init()
+  }
 
   public func resolve<Act>(id: ActorID, as actorType: Act.Type)
-    throws -> Act? where Act: DistributedActor {
-    guard let anyActor = self.activeActorsLock.withLock({ self.activeActors[id] }) else {
+    throws -> Act? where Act: DistributedActor, Act.ID == ActorID {
+    guard let anyActor = storage.activeActorsLock.withLock({ self.storage.activeActors[id] }) else {
       throw LocalTestingDistributedActorSystemError(message: "Unable to locate id '\(id)' locally")
     }
     guard let actor = anyActor as? Act else {
@@ -54,28 +63,25 @@ public final class LocalTestingDistributedActorSystem: DistributedActorSystem, @
   }
 
   public func assignID<Act>(_ actorType: Act.Type) -> ActorID
-    where Act: DistributedActor {
+    where Act: DistributedActor, Act.ID == ActorID {
     let id = self.idProvider.next()
-    self.assignedIDsLock.withLock {
-      self.assignedIDs.insert(id)
+    storage.assignedIDsLock.withLock {
+      self.storage.assignedIDs.insert(id)
     }
     return id
   }
 
   public func actorReady<Act>(_ actor: Act)
-    where Act: DistributedActor,
-    Act.ID == ActorID {
-    guard self.assignedIDsLock.withLock({ self.assignedIDs.contains(actor.id) }) else {
+    where Act: DistributedActor, Act.ID == ActorID {
+    guard storage.assignedIDsLock.withLock({ self.storage.assignedIDs.contains(actor.id) }) else {
       fatalError("Attempted to mark an unknown actor '\(actor.id)' ready")
     }
-    self.activeActorsLock.withLock {
-      self.activeActors[actor.id] = actor
-    }
+    self.storage.activeActors[actor.id] = actor
   }
 
   public func resignID(_ id: ActorID) {
-    self.activeActorsLock.withLock {
-      self.activeActors.removeValue(forKey: id)
+    storage.activeActorsLock.withLock {
+      self.storage.activeActors.removeValue(forKey: id)
     }
   }
 
@@ -93,7 +99,7 @@ public final class LocalTestingDistributedActorSystem: DistributedActorSystem, @
     where Act: DistributedActor,
           Act.ID == ActorID,
           Err: Error,
-          Res: SerializationRequirement {
+          Res: Codable {
     fatalError("Attempted to make remote call to \(target) on actor \(actor) using a local-only actor system")
   }
 
@@ -109,38 +115,40 @@ public final class LocalTestingDistributedActorSystem: DistributedActorSystem, @
     fatalError("Attempted to make remote call to \(target) on actor \(actor) using  a local-only actor system")
   }
 
-  private struct ActorIDProvider {
+  @usableFromInline
+  final class ActorIDProvider {
     private var counter: Int = 0
     private let counterLock = _Lock()
 
+    @usableFromInline
     init() {}
 
-    mutating func next() -> LocalTestingActorAddress {
+    func next() -> LocalTestingActorAddress {
       let id: Int = self.counterLock.withLock {
         self.counter += 1
         return self.counter
       }
-      return LocalTestingActorAddress(parse: "\(id)")
+      return LocalTestingActorAddress(id)
     }
   }
 }
 
 @available(SwiftStdlib 5.7, *)
 public struct LocalTestingActorAddress: Hashable, Sendable, Codable {
-  public let address: String
+  public let uuid: String
 
-  public init(parse address: String) {
-    self.address = address
+  public init(_ uuid: Int) {
+    self.uuid = "\(uuid)"
   }
 
   public init(from decoder: Decoder) throws {
     let container = try decoder.singleValueContainer()
-    self.address = try container.decode(String.self)
+    self.uuid = try container.decode(String.self)
   }
 
   public func encode(to encoder: Encoder) throws {
     var container = encoder.singleValueContainer()
-    try container.encode(self.address)
+    try container.encode(self.uuid)
   }
 }
 
@@ -220,7 +228,7 @@ public struct LocalTestingDistributedActorSystemError: DistributedActorSystemErr
 // === lock ----------------------------------------------------------------
 
 @available(SwiftStdlib 5.7, *)
-fileprivate class _Lock {
+internal class _Lock {
   #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS)
   private let underlying: UnsafeMutablePointer<os_unfair_lock>
   #elseif os(Windows)
@@ -233,10 +241,27 @@ fileprivate class _Lock {
   private let underlying: UnsafeMutablePointer<pthread_mutex_t>
   #endif
 
+  init() {
+    #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS)
+    self.underlying = UnsafeMutablePointer.allocate(capacity: 1)
+    self.underlying.initialize(to: os_unfair_lock())
+    #elseif os(Windows)
+    self.underlying = UnsafeMutablePointer.allocate(capacity: 1)
+    InitializeSRWLock(self.underlying)
+    #elseif os(WASI)
+    // WASI environment has only a single thread
+    #else
+    self.underlying = UnsafeMutablePointer.allocate(capacity: 1)
+    guard pthread_mutex_init(self.underlying, nil) == 0 else {
+      fatalError("pthread_mutex_init failed")
+    }
+    #endif
+  }
+
   deinit {
     #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS)
     // `os_unfair_lock`s do not need to be explicitly destroyed
-    #elseif os(Windows)    
+    #elseif os(Windows)
     // `SRWLOCK`s do not need to be explicitly destroyed
     #elseif os(WASI)
     // WASI environment has only a single thread
@@ -252,22 +277,6 @@ fileprivate class _Lock {
     #endif
   }
 
-  init() {
-    #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS)
-    self.underlying = UnsafeMutablePointer.allocate(capacity: 1)
-    #elseif os(Windows)
-    self.underlying = UnsafeMutablePointer.allocate(capacity: 1)
-    InitializeSRWLock(self.underlying)
-    #elseif os(WASI)
-    // WASI environment has only a single thread
-    #else
-    self.underlying = UnsafeMutablePointer.allocate(capacity: 1)
-    guard pthread_mutex_init(self.underlying, nil) == 0 else {
-      fatalError("pthread_mutex_init failed")
-    }
-    #endif
-  }
-
   @discardableResult
   func withLock<T>(_ body: () -> T) -> T {
     #if os(iOS) || os(macOS) || os(tvOS) || os(watchOS)

test/Distributed/Runtime/distributed_actor_init_local.swift

@@ -90,6 +90,26 @@ distributed actor MaybeAfterAssign {
   }
 }
 
+distributed actor LocalTestingSystemDA {
+  typealias ActorSystem = LocalTestingDistributedActorSystem
+  var x: Int
+  init() {
+    actorSystem = .init()
+    x = 100
+  }
+}
+
+distributed actor LocalTestingDA_Int {
+  typealias ActorSystem = LocalTestingDistributedActorSystem
+  var int: Int
+  init() {
+    actorSystem = .init()
+    int = 12
+    // CRASH
+  }
+}
+
+
 // ==== Fake Transport ---------------------------------------------------------
 
 struct ActorAddress: Sendable, Hashable, Codable {
@@ -262,6 +282,10 @@ func test() async {
   // CHECK:      assign type:MaybeAfterAssign, id:ActorAddress(address: "[[ID10:.*]]")
   // CHECK-NEXT: ready actor:main.MaybeAfterAssign, id:ActorAddress(address: "[[ID10]]")
 
+  let localDA = LocalTestingDA_Int()
+  print("localDA = \(localDA.id)")
+  // CHECK: localDA = LocalTestingActorAddress(uuid: "1")
+
   // the following tests fail to initialize the actor's identity.
   print("-- start of no-assign tests --")
   test.append(MaybeSystem(nil))
@@ -271,7 +295,6 @@ func test() async {
   // CHECK-NOT: assign
   // CHECK: -- end of no-assign tests --
 
-
   // resigns that come out of the deinits:
 
   // CHECK-DAG: resign id:ActorAddress(address: "[[ID1]]")

test/Distributed/distributed_actor_layout.swift

@@ -12,8 +12,6 @@ import FakeDistributedActorSystems
 @available(SwiftStdlib 5.7, *)
 typealias DefaultDistributedActorSystem = FakeActorSystem
 
-class MyClass { }
-
 // Ensure that the actor layout is (metadata pointer, default actor, id, system,
 // <user fields>)
 
@@ -23,12 +21,27 @@ protocol HasActorSystem {
 
 extension MyActor: HasActorSystem { }
 
-// CHECK: %T27distributed_actor_accessors7MyActorC = type <{ %swift.refcounted, %swift.defaultactor, %T27FakeDistributedActorSystems0C7AddressV, %T27FakeDistributedActorSystems0aC6SystemV, %T27distributed_actor_accessors7MyClassC* }>
+// CHECK: %T27distributed_actor_accessors7MyActorC = type <{ %swift.refcounted, %swift.defaultactor, %T27FakeDistributedActorSystems0C7AddressV, %T27FakeDistributedActorSystems0aC6SystemV, %TSS }>
 @available(SwiftStdlib 5.7, *)
 public distributed actor MyActor {
-  var field: MyClass = MyClass()
+  var field: String = ""
 
   init(actorSystem: FakeActorSystem) {
     self.actorSystem = actorSystem
   }
 }
+
+// CHECK: %T27distributed_actor_accessors10MyActorIntC = type <{ %swift.refcounted, %swift.defaultactor }>
+//
+// This does not have the concrete fields in the IR type because the LocalTestingDistributedActorSystem
+// is declared in Distributed, which means that it is compiled with library evolution turned on,
+// which causes the type to be laid out at runtime.
+@available(SwiftStdlib 5.7, *)
+public distributed actor MyActorInt {
+  public typealias ActorSystem = LocalTestingDistributedActorSystem
+  var field: String = ""
+
+  init(actorSystem: LocalTestingDistributedActorSystem) {
+    self.actorSystem = actorSystem
+  }
+}