Convert public API and internals to @mainactor

In 7d6acde we made the decision to use `actor` types to manage the
mutable state of the SDK. Whilst we've been able to build the SDK using
this approach, it has proved cumbersome.

The public API is littered with `async` methods and getters for things
that really should be synchronous, such as adding a subscription or
fetching a room's state. This deviates from the JavaScript API and makes
call sites look complicated.

The use of `async` for fetching room state also means that it's
impossible for a user to, for example, check a room's state and then
reason about what operations they should be able to attempt to perform
on the room (i.e. without the SDK immediately rejecting the attempt),
because the room state can change under their feet. This is one of a
class of threading-related problems that I describe in #260.

Internally, we find ourselves having to write `await` to do basic things
like fetching the room lifecycle manager's state or to find out the
arguments with which a mock method was called. And similarly, one
internal component can't rely on the state that's isolated to a
different internal isolation domain in order to make a decision, because
this state might change under its feet.

To address this, here I have decided to isolate all of the SDK's mutable
state to the main actor. This simplifies the experience of developing
the SDK, and it simplifies the experience of interacting with it from
the main thread, which is where most of our users interact with it from.

(Note that this does not prevent us from using background tasks
internally for moving heavy processing off the main thread; I'm
specifically talking about _state_.)

This also moves us closer to being able to address #49 (keeping our
internal state in sync with that of the underlying ably-cocoa objects).

Note that this change does decrease the parallelisability of our test
suite. But it still runs very fast.

Have removed:

- all `actor` types (both in SDK and mocks)
- nearly all locking (there are a couple of non-MainActor types for
  which we still have it)

There might be some simplifications that can be made for the lifecycle
manager tests now, but I'm not going to look at them now because that
class will change a lot for the single-channel work in #242.

I'll update the ably.com documentation in #254.

Even after this change, Messages.subscribe is still `async` because its
implementation requires it to be. I believe that this is a bug; have
created #257 for it.

Resolves #232.

Author: lawrence-forooghian

CONTRIBUTING.md

@@ -39,6 +39,7 @@ To check formatting and code quality, run `swift run BuildTool lint`. Run with `
   - Describe the SDK’s functionality via protocols (when doing so would still be sufficiently idiomatic to Swift).
   - When defining a `struct` that is emitted by the public API of the library, make sure to define a public memberwise initializer so that users can create one to be emitted by their mocks. (There is no way to make Swift’s autogenerated memberwise initializer public, so you will need to write one yourself. In Xcode, you can do this by clicking at the start of the type declaration and doing Editor → Refactor → Generate Memberwise Initializer.)
 - When writing code that implements behaviour specified by the Chat SDK features spec, add a comment that references the identifier of the relevant spec item.
+- The SDK isolates all of its mutable state to the main actor. Stateful objects should be marked as `@MainActor`.
 
 ### Throwing errors
 
@@ -64,6 +65,34 @@ If you haven't worked with typed throws before, be aware of a few sharp edges:
   }
   ```
 
+### Swift concurrency rough edges
+
+#### `AsyncSequence` operator compiler errors
+
+Consider the following code:
+
+```swift
+@MainActor
+func myThing() async {
+    let streamComponents = AsyncStream<Void>.makeStream()
+    await streamComponents.stream.first { _ in true }
+}
+```
+
+This gives a compiler error "Sending main actor-isolated value of type '(Void) async -> Bool' with later accesses to nonisolated context risks causing data races". This is a minimal reproduction of a similar error that I have come across when trying to use operators on an `AsyncSequence`. I do not understand enough about Swift concurrency to be able to give a good explanation of what's going on here. However, I have noticed that this error goes away if you explicitly mark the operator body as `@Sendable` (my reasoning was "the closure mentions the fact that the closure is main actor-isolated, so what if I make it not be; I think writing `@Sendable` achieves that for reasons I'm not fully sure of").
+
+So the following code compiles, and you'll notice lots of `@Sendable` closures dotted around the codebase for this reason.
+
+```swift
+@MainActor
+func myThing() async {
+    let streamComponents = AsyncStream<Void>.makeStream()
+    await streamComponents.stream.first { @Sendable _ in true }
+}
+```
+
+I hope that as we understand more about Swift concurrency, we'll have a better understanding of what's going on here and whether this is the right way to fix it.
+
 ### Testing guidelines
 
 #### Exposing test-only APIs

Example/AblyChatExample/ContentView.swift

@@ -12,6 +12,7 @@ private enum Environment: Equatable {
     ///   - clientId: A string that identifies this client.
     case live(key: String, clientId: String)
 
+    @MainActor
     func createChatClient() -> ChatClient {
         switch self {
         case .mock:
@@ -200,14 +201,18 @@ struct ContentView: View {
         }
         .task {
             do {
-                try await attachRoom()
-                try await showMessages()
-                try await showReactions()
-                try await showPresence()
-                try await showOccupancy()
-                try await showTypings()
-                try await showRoomStatus()
-                await printConnectionStatusChange()
+                let room = try await room()
+
+                try await room.attach()
+                try await room.presence.enter(data: ["status": "📱 Online"])
+
+                try await showMessages(room: room)
+                showReactions(room: room)
+                showPresence(room: room)
+                try await showOccupancy(room: room)
+                showTypings(room: room)
+                showRoomStatus(room: room)
+                printConnectionStatusChange()
             } catch {
                 print("Failed to initialize room: \(error)") // TODO: replace with logger (+ message to the user?)
             }
@@ -231,12 +236,8 @@ struct ContentView: View {
         }
     }
 
-    func attachRoom() async throws {
-        try await room().attach()
-    }
-
-    func showMessages() async throws {
-        let messagesSubscription = try await room().messages.subscribe()
+    func showMessages(room: Room) async throws {
+        let messagesSubscription = try await room.messages.subscribe()
         let previousMessages = try await messagesSubscription.getPreviousMessages(params: .init())
 
         for message in previousMessages.items {
@@ -278,8 +279,8 @@ struct ContentView: View {
         }
     }
 
-    func showReactions() async throws {
-        let reactionSubscription = try await room().reactions.subscribe()
+    func showReactions(room: Room) {
+        let reactionSubscription = room.reactions.subscribe()
 
         // Continue listening for reactions on a background task so this function can return
         Task {
@@ -291,12 +292,10 @@ struct ContentView: View {
         }
     }
 
-    func showPresence() async throws {
-        try await room().presence.enter(data: ["status": "📱 Online"])
-
+    func showPresence(room: Room) {
         // Continue listening for new presence events on a background task so this function can return
         Task {
-            for await event in try await room().presence.subscribe(events: [.enter, .leave, .update]) {
+            for await event in room.presence.subscribe(events: [.enter, .leave, .update]) {
                 withAnimation {
                     listItems.insert(
                         .presence(
@@ -311,8 +310,8 @@ struct ContentView: View {
         }
     }
 
-    func showTypings() async throws {
-        let typingSubscription = try await room().typing.subscribe()
+    func showTypings(room: Room) {
+        let typingSubscription = room.typing.subscribe()
         // Continue listening for typing events on a background task so this function can return
         Task {
             for await typing in typingSubscription {
@@ -326,23 +325,23 @@ struct ContentView: View {
         }
     }
 
-    func showOccupancy() async throws {
+    func showOccupancy(room: Room) async throws {
         // Continue listening for occupancy events on a background task so this function can return
-        let currentOccupancy = try await room().occupancy.get()
+        let currentOccupancy = try await room.occupancy.get()
         withAnimation {
             occupancyInfo = "Connections: \(currentOccupancy.presenceMembers) (\(currentOccupancy.connections))"
         }
 
         Task {
-            for await event in try await room().occupancy.subscribe() {
+            for await event in room.occupancy.subscribe() {
                 withAnimation {
                     occupancyInfo = "Connections: \(event.presenceMembers) (\(event.connections))"
                 }
             }
         }
     }
 
-    func printConnectionStatusChange() async {
+    func printConnectionStatusChange() {
         let connectionSubsciption = chatClient.connection.onStatusChange()
 
         // Continue listening for connection status change on a background task so this function can return
@@ -353,10 +352,10 @@ struct ContentView: View {
         }
     }
 
-    func showRoomStatus() async throws {
+    func showRoomStatus(room: Room) {
         // Continue listening for status change events on a background task so this function can return
         Task {
-            for await status in try await room().onStatusChange() {
+            for await status in room.onStatusChange() {
                 withAnimation {
                     if status.current.isAttaching {
                         statusInfo = "\(status.current)...".capitalized

Example/AblyChatExample/Mocks/MockClients.swift

@@ -1,7 +1,7 @@
 import Ably
 import AblyChat
 
-actor MockChatClient: ChatClient {
+class MockChatClient: ChatClient {
     let realtime: RealtimeClient
     nonisolated let clientOptions: ChatClientOptions
     nonisolated let rooms: Rooms
@@ -19,7 +19,7 @@ actor MockChatClient: ChatClient {
     }
 }
 
-actor MockRooms: Rooms {
+class MockRooms: Rooms {
     let clientOptions: ChatClientOptions
     private var rooms = [String: MockRoom]()
 
@@ -41,27 +41,27 @@ actor MockRooms: Rooms {
     }
 }
 
-actor MockRoom: Room {
+class MockRoom: Room {
     private let clientID = "AblyTest"
 
     nonisolated let roomID: String
     nonisolated let options: RoomOptions
+    nonisolated let messages: any Messages
+    nonisolated let presence: any Presence
+    nonisolated let reactions: any RoomReactions
+    nonisolated let typing: any Typing
+    nonisolated let occupancy: any Occupancy
 
     init(roomID: String, options: RoomOptions) {
         self.roomID = roomID
         self.options = options
+        messages = MockMessages(clientID: clientID, roomID: roomID)
+        presence = MockPresence(clientID: clientID, roomID: roomID)
+        reactions = MockRoomReactions(clientID: clientID, roomID: roomID)
+        typing = MockTyping(clientID: clientID, roomID: roomID)
+        occupancy = MockOccupancy(clientID: clientID, roomID: roomID)
     }
 
-    nonisolated lazy var messages: any Messages = MockMessages(clientID: clientID, roomID: roomID)
-
-    nonisolated lazy var presence: any Presence = MockPresence(clientID: clientID, roomID: roomID)
-
-    nonisolated lazy var reactions: any RoomReactions = MockRoomReactions(clientID: clientID, roomID: roomID)
-
-    nonisolated lazy var typing: any Typing = MockTyping(clientID: clientID, roomID: roomID)
-
-    nonisolated lazy var occupancy: any Occupancy = MockOccupancy(clientID: clientID, roomID: roomID)
-
     var status: RoomStatus = .initialized
 
     private let mockSubscriptions = MockSubscriptionStorage<RoomStatusChange>()
@@ -80,12 +80,12 @@ actor MockRoom: Room {
         }, interval: 8)
     }
 
-    func onStatusChange(bufferingPolicy _: BufferingPolicy) async -> Subscription<RoomStatusChange> {
+    func onStatusChange(bufferingPolicy _: BufferingPolicy) -> Subscription<RoomStatusChange> {
         .init(mockAsyncSequence: createSubscription())
     }
 }
 
-actor MockMessages: Messages {
+class MockMessages: Messages {
     let clientID: String
     let roomID: String
     let channel: any RealtimeChannelProtocol
@@ -116,7 +116,7 @@ actor MockMessages: Messages {
         }, interval: 3)
     }
 
-    func subscribe(bufferingPolicy _: BufferingPolicy) async -> MessageSubscription {
+    func subscribe(bufferingPolicy _: BufferingPolicy) -> MessageSubscription {
         MessageSubscription(mockAsyncSequence: createSubscription()) { _ in
             MockMessagesPaginatedResult(clientID: self.clientID, roomID: self.roomID)
         }
@@ -185,7 +185,7 @@ actor MockMessages: Messages {
     }
 }
 
-actor MockRoomReactions: RoomReactions {
+class MockRoomReactions: RoomReactions {
     let clientID: String
     let roomID: String
     let channel: any RealtimeChannelProtocol
@@ -232,7 +232,7 @@ actor MockRoomReactions: RoomReactions {
     }
 }
 
-actor MockTyping: Typing {
+class MockTyping: Typing {
     let clientID: String
     let roomID: String
     let channel: any RealtimeChannelProtocol
@@ -275,7 +275,7 @@ actor MockTyping: Typing {
     }
 }
 
-actor MockPresence: Presence {
+class MockPresence: Presence {
     let clientID: String
     let roomID: String
 
@@ -395,7 +395,7 @@ actor MockPresence: Presence {
     }
 }
 
-actor MockOccupancy: Occupancy {
+class MockOccupancy: Occupancy {
     let clientID: String
     let roomID: String
     let channel: any RealtimeChannelProtocol
@@ -415,7 +415,7 @@ actor MockOccupancy: Occupancy {
         }, interval: 1)
     }
 
-    func subscribe(bufferingPolicy _: BufferingPolicy) async -> Subscription<OccupancyEvent> {
+    func subscribe(bufferingPolicy _: BufferingPolicy) -> Subscription<OccupancyEvent> {
         .init(mockAsyncSequence: createSubscription())
     }
 
@@ -428,7 +428,7 @@ actor MockOccupancy: Occupancy {
     }
 }
 
-actor MockConnection: Connection {
+class MockConnection: Connection {
     let status: AblyChat.ConnectionStatus
     let error: ARTErrorInfo?
 

Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift

@@ -1,36 +1,32 @@
 import Foundation
 
 // This is copied from ably-chat’s internal class `SubscriptionStorage`.
-class MockSubscriptionStorage<Element: Sendable>: @unchecked Sendable {
+@MainActor
+class MockSubscriptionStorage<Element: Sendable> {
     // We hold a weak reference to the subscriptions that we create, so that the subscriptions’ termination handlers get called when the user releases their final reference to the subscription.
     private struct WeaklyHeldSubscription {
         weak var subscription: MockSubscription<Element>?
     }
 
-    /// Access must be synchronised via ``lock``.
     private var subscriptions: [UUID: WeaklyHeldSubscription] = [:]
-    private let lock = NSLock()
 
     // You must not call the `setOnTermination` method of a subscription returned by this function, as it will replace the termination handler set by this function.
     func create(randomElement: @escaping @Sendable () -> Element, interval: Double) -> MockSubscription<Element> {
         let subscription = MockSubscription<Element>(randomElement: randomElement, interval: interval)
         let id = UUID()
-
-        lock.withLock {
-            subscriptions[id] = .init(subscription: subscription)
-        }
+        subscriptions[id] = .init(subscription: subscription)
 
         subscription.setOnTermination { [weak self] in
-            self?.subscriptionDidTerminate(id: id)
+            Task { @MainActor in
+                self?.subscriptionDidTerminate(id: id)
+            }
         }
 
         return subscription
     }
 
     private func subscriptionDidTerminate(id: UUID) {
-        lock.withLock {
-            _ = subscriptions.removeValue(forKey: id)
-        }
+        _ = subscriptions.removeValue(forKey: id)
     }
 
     func emit(_ element: Element) {

README.md

@@ -114,7 +114,7 @@ let room = try await chatClient.rooms.get(
     roomID: "readme-getting-started", options: RoomOptions.allFeaturesEnabled)
 
 // Add a listener to observe changes to the chat rooms status
-let statusSubscription = await room.onStatusChange()
+let statusSubscription = room.onStatusChange()
 Task {
     for await status in statusSubscription {
         print("Room status changed: \(status.current)")