[ECO-5384] Implement memory management pattern similar to ably-cocoa

CONTRIBUTING.md

@@ -45,6 +45,28 @@ To check formatting and code quality, run `swift run BuildTool lint`. Run with `
 - The public API of the SDK should use typed throws, and the thrown errors should be of type `ARTErrorInfo`.
 - `Dictionary.mapValues` does not support typed throws. We have our own extension `ablyLiveObjects_mapValuesWithTypedThrow` which does; use this.
 
+### Memory management
+
+We follow an approach to memory management that is broadly similar to that of ably-cocoa: we keep all of the internal components of the SDK alive as long as the user is holding a strong reference to any object vended by the public API of the SDK. This means that, for example, a user can use LiveObjects functionality by maintaining only a reference to the root `LiveMap`; even if they relinquish their references to, say, the `ARTRealtime` or `ARTRealtimeChannel` instance, they will continue to receive events from the `LiveMap` and they will be able to use its methods.
+
+We achieve this by vending a set of public types which maintain strong references to all of the internal components of the SDK which are needed in order for the public type to function correctly. For example, the public `PublicDefaultLiveMap` type wraps an `InternalDefaultLiveMap`, and holds strong references to the `CoreSDK` object, which in turn holds the following sequence of strong references: `CoreSDK` → `RealtimeClient` → `RealtimeChannel` → `InternalDefaultRealtimeObjects`, thus ensuring that:
+
+1. the `InternalDefaultLiveMap` can always perform actions on these dependencies in response to a user action
+2. these dependencies, which deliver events to the `InternalDefaultLiveMap`, remain alive and thus remain delivering events
+
+The key rules that must be followed in order to avoid a strong reference cycle are that the SDK's _internal_ classes (that is `InternalDefaultLiveMap` etc) _must never hold a strong reference to_:
+
+- any of the corresponding public types (e.g. `PublicDefaultLiveMap`)
+- any of the ably-cocoa components that hold a strong reference to these internal components (that is, the realtime client or channel); thus, they must never hold a strong reference to a `CoreSDK` object
+
+Note that, unlike ably-cocoa, the internal components do not even hold weak references to their dependencies; rather, these dependencies are passed in by the public object when the user performs an action that requires one of these dependencies. (There may turn out to be limitations to this approach, but haven't found them yet.)
+
+Also note that, unlike ably-cocoa, we aim to provide a stable pointer identity for the public objects vended by the SDK, instead of creating a new object each time the user requests one. See the `PublicObjectsStore` class for more details.
+
+The `Public…` classes all follow the same pattern and are not very interesting; the business logic should be in the `Internal…` classes and those should be where we focus our unit testing effort.
+
+`ObjectLifetimesTests.swift` contains tests of the behaviour described in this section.
+
 ### Testing guidelines
 
 #### Attributing tests to a spec point

Package.swift

@@ -56,6 +56,14 @@ let package = Package(
             name: "AblyLiveObjectsTests",
             dependencies: [
                 "AblyLiveObjects",
+                .product(
+                    name: "Ably",
+                    package: "ably-cocoa",
+                ),
+                .product(
+                    name: "AblyPlugin",
+                    package: "ably-cocoa",
+                ),
             ],
             resources: [
                 .copy("ably-common"),

Sources/AblyLiveObjects/Internal/CoreSDK.swift

@@ -12,29 +12,20 @@ internal protocol CoreSDK: AnyObject, Sendable {
 }
 
 internal final class DefaultCoreSDK: CoreSDK {
-    // We hold a weak reference to the channel so that `DefaultLiveObjects` can hold a strong reference to us without causing a strong reference cycle. We'll revisit this in https://github.com/ably/ably-cocoa-liveobjects-plugin/issues/9.
-    private let weakChannel: WeakRef<AblyPlugin.RealtimeChannel>
+    private let channel: AblyPlugin.RealtimeChannel
+    private let client: AblyPlugin.RealtimeClient
     private let pluginAPI: PluginAPIProtocol
 
     internal init(
         channel: AblyPlugin.RealtimeChannel,
+        client: AblyPlugin.RealtimeClient,
         pluginAPI: PluginAPIProtocol
     ) {
-        weakChannel = .init(referenced: channel)
+        self.channel = channel
+        self.client = client
         self.pluginAPI = pluginAPI
     }
 
-    // MARK: - Fetching channel
-
-    private var channel: AblyPlugin.RealtimeChannel {
-        guard let channel = weakChannel.referenced else {
-            // It's currently completely possible that the channel _does_ become deallocated during the usage of the LiveObjects SDK; in https://github.com/ably/ably-cocoa-liveobjects-plugin/issues/9 we'll figure out how to prevent this.
-            preconditionFailure("Expected channel to not become deallocated during usage of LiveObjects SDK")
-        }
-
-        return channel
-    }
-
     // MARK: - CoreSDK conformance
 
     internal func sendObject(objectMessages: [OutboundObjectMessage]) async throws(InternalError) {

Sources/AblyLiveObjects/Internal/DefaultInternalPlugin.swift

@@ -17,41 +17,32 @@ internal final class DefaultInternalPlugin: NSObject, AblyPlugin.LiveObjectsInte
     /// The `pluginDataValue(forKey:channel:)` key that we use to store the value of the `ARTRealtimeChannel.objects` property.
     private static let pluginDataKey = "LiveObjects"
 
-    /// Retrieves the value that should be returned by `ARTRealtimeChannel.objects`.
-    ///
-    /// We expect this value to have been previously set by ``prepare(_:)``.
-    internal static func objectsProperty(for channel: ARTRealtimeChannel, pluginAPI: AblyPlugin.PluginAPIProtocol) -> DefaultRealtimeObjects {
-        let pluginChannel = pluginAPI.channel(forPublicRealtimeChannel: channel)
-        return realtimeObjects(for: pluginChannel, pluginAPI: pluginAPI)
-    }
-
     /// Retrieves the `RealtimeObjects` for this channel.
     ///
     /// We expect this value to have been previously set by ``prepare(_:)``.
-    private static func realtimeObjects(for channel: AblyPlugin.RealtimeChannel, pluginAPI: AblyPlugin.PluginAPIProtocol) -> DefaultRealtimeObjects {
+    internal static func realtimeObjects(for channel: AblyPlugin.RealtimeChannel, pluginAPI: AblyPlugin.PluginAPIProtocol) -> InternalDefaultRealtimeObjects {
         guard let pluginData = pluginAPI.pluginDataValue(forKey: pluginDataKey, channel: channel) else {
             // InternalPlugin.prepare was not called
             fatalError("To access LiveObjects functionality, you must pass the LiveObjects plugin in the client options when creating the ARTRealtime instance: `clientOptions.plugins = [.liveObjects: AblyLiveObjects.Plugin.self]`")
         }
 
         // swiftlint:disable:next force_cast
-        return pluginData as! DefaultRealtimeObjects
+        return pluginData as! InternalDefaultRealtimeObjects
     }
 
     // MARK: - LiveObjectsInternalPluginProtocol
 
     // Populates the channel's `objects` property.
-    internal func prepare(_ channel: AblyPlugin.RealtimeChannel) {
+    internal func prepare(_ channel: AblyPlugin.RealtimeChannel, client _: AblyPlugin.RealtimeClient) {
         let logger = pluginAPI.logger(for: channel)
 
         logger.log("LiveObjects.DefaultInternalPlugin received prepare(_:)", level: .debug)
-        let coreSDK = DefaultCoreSDK(channel: channel, pluginAPI: pluginAPI)
-        let liveObjects = DefaultRealtimeObjects(coreSDK: coreSDK, logger: logger)
+        let liveObjects = InternalDefaultRealtimeObjects(logger: logger)
         pluginAPI.setPluginDataValue(liveObjects, forKey: Self.pluginDataKey, channel: channel)
     }
 
     /// Retrieves the internally-typed `objects` property for the channel.
-    private func realtimeObjects(for channel: AblyPlugin.RealtimeChannel) -> DefaultRealtimeObjects {
+    private func realtimeObjects(for channel: AblyPlugin.RealtimeChannel) -> InternalDefaultRealtimeObjects {
         Self.realtimeObjects(for: channel, pluginAPI: pluginAPI)
     }
 

Sources/AblyLiveObjects/Internal/DefaultLiveCounter.swift → Sources/AblyLiveObjects/Internal/InternalDefaultLiveCounter.swift

@@ -2,8 +2,8 @@ import Ably
 internal import AblyPlugin
 import Foundation
 
-/// Our default implementation of ``LiveCounter``.
-internal final class DefaultLiveCounter: LiveCounter {
+/// This provides the implementation behind ``PublicDefaultLiveCounter``, via internal versions of the ``LiveCounter`` API.
+internal final class InternalDefaultLiveCounter: Sendable {
     // Used for synchronizing access to all of this instance's mutable state. This is a temporary solution just to allow us to implement `Sendable`, and we'll revisit it in https://github.com/ably/ably-cocoa-liveobjects-plugin/issues/3.
     private let mutex = NSLock()
 
@@ -27,28 +27,24 @@ internal final class DefaultLiveCounter: LiveCounter {
         }
     }
 
-    private let coreSDK: CoreSDK
     private let logger: AblyPlugin.Logger
 
     // MARK: - Initialization
 
     internal convenience init(
         testsOnly_data data: Double,
         objectID: String,
-        coreSDK: CoreSDK,
         logger: AblyPlugin.Logger
     ) {
-        self.init(data: data, objectID: objectID, coreSDK: coreSDK, logger: logger)
+        self.init(data: data, objectID: objectID, logger: logger)
     }
 
     private init(
         data: Double,
         objectID: String,
-        coreSDK: CoreSDK,
         logger: AblyPlugin.Logger
     ) {
         mutableState = .init(liveObject: .init(objectID: objectID), data: data)
-        self.coreSDK = coreSDK
         self.logger = logger
     }
 
@@ -58,35 +54,31 @@ internal final class DefaultLiveCounter: LiveCounter {
     ///   - objectID: The value for the "private objectId field" of RTO5c1b1a.
     internal static func createZeroValued(
         objectID: String,
-        coreSDK: CoreSDK,
         logger: AblyPlugin.Logger,
     ) -> Self {
         .init(
             data: 0,
             objectID: objectID,
-            coreSDK: coreSDK,
             logger: logger,
         )
     }
 
-    // MARK: - LiveCounter conformance
+    // MARK: - Internal methods that back LiveCounter conformance
 
-    internal var value: Double {
-        get throws(ARTErrorInfo) {
-            // RTLC5b: If the channel is in the DETACHED or FAILED state, the library should indicate an error with code 90001
-            let currentChannelState = coreSDK.channelState
-            if currentChannelState == .detached || currentChannelState == .failed {
-                throw LiveObjectsError.objectsOperationFailedInvalidChannelState(
-                    operationDescription: "LiveCounter.value",
-                    channelState: currentChannelState,
-                )
-                .toARTErrorInfo()
-            }
+    internal func value(coreSDK: CoreSDK) throws(ARTErrorInfo) -> Double {
+        // RTLC5b: If the channel is in the DETACHED or FAILED state, the library should indicate an error with code 90001
+        let currentChannelState = coreSDK.channelState
+        if currentChannelState == .detached || currentChannelState == .failed {
+            throw LiveObjectsError.objectsOperationFailedInvalidChannelState(
+                operationDescription: "LiveCounter.value",
+                channelState: currentChannelState,
+            )
+            .toARTErrorInfo()
+        }
 
-            return mutex.withLock {
-                // RTLC5c
-                mutableState.data
-            }
+        return mutex.withLock {
+            // RTLC5c
+            mutableState.data
         }
     }