Merge pull request #5 from getditto/BP/retain_cycle

Fix ditto instance retain cycle in PresenceViewer
getditto · Jan 2, 2025 · 2b7f93c · 2b7f93c
2 parents 1f1f912 + 9f8da06
commit 2b7f93c
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 46 deletions.
diff --git a/Swift/Sources/DittoPresenceViewer/DittoPresenceView.swift b/Swift/Sources/DittoPresenceViewer/DittoPresenceView.swift
@@ -36,13 +36,13 @@ public typealias PlatformViewController = NSViewController
  let ditto: Ditto
 
  func showPresence() {
-      let presenceView = DittoPresenceView(ditto: ditto)
-      // maybe add it to an existing view
-      self.view.addSubview(presenceView)
+ let presenceView = DittoPresenceView(ditto: ditto)
+ // maybe add it to an existing view
+ self.view.addSubview(presenceView)
 
-      // or add it as a view controller
-      let viewController = DittoPresenceView(ditto: ditto).viewController
-      present(viewController: viewController, animated: true)
+ // or add it as a view controller
+ let viewController = DittoPresenceView(ditto: ditto).viewController
+ present(viewController: viewController, animated: true)
  }
  ```
  */
@@ -55,12 +55,26 @@ public class DittoPresenceView: PlatformView {
      The `DittoPresenceView` will not display any presence information
      until the `ditto` property has been set.
      */
-    public var ditto: Ditto? {
+    public weak var ditto: Ditto? {
+        willSet {
+            if (ditto !== newValue) {
+                self.stopObservingPresence()
+            }
+        }
+
         didSet {
-            observePresence()
+            if (oldValue !== ditto) {
+                self.startObservingPresence()
+            }
         }
     }
 
+    deinit {
+        stopObservingPresence()
+        webView.removeFromSuperview()
+        _vc = nil
+    }
+
     /**
      Returns a `UIViewController` containing this view.
      */
@@ -73,7 +87,17 @@ public class DittoPresenceView: PlatformView {
         // The inversion will likely trip up more experienced native iOS
         // developers, but they're also the group likely to be designing their
         // own `UIViewControllers` which renders this a non-issue in most cases.
-        return _vc
+        //
+        // Also note that we can't hold the view controller strongly here because
+        // this would lead to a retain cycle. Therefore, we'll hold it weakly
+        // and create a new one whenever the previous one is dealloced:
+        if let vc = self._vc {
+            return vc
+        }
+
+        let vc = DittoPresenceViewController(view: self)
+        self._vc = vc
+        return vc
     }
 
     // MARK: Private Properties
@@ -82,22 +106,20 @@ public class DittoPresenceView: PlatformView {
 
     private var webView = VisJSWebView()
 
-    private lazy var _vc: PlatformViewController! = DittoPresenceViewController(view: self)
+    private weak var _vc: DittoPresenceViewController?
 
     // MARK: Initializer
 
     /**
      Initializes a new `DittoPresenceView`.
 
      - Parameter ditto: A reference to the `Ditto` which you would like
-       to visualize presence status for.
+     to visualize presence status for.
      */
     public convenience init(ditto: Ditto) {
         self.init(frame: .zero)
         self.ditto = ditto
-
-        setup()
-        observePresence()
+        startObservingPresence()
     }
 
     public override init(frame: CGRect) {
@@ -110,9 +132,11 @@ public class DittoPresenceView: PlatformView {
         setup()
     }
 
+
     // MARK: Private Functions
 
     private func setup() {
+
 #if canImport(UIKit)
         backgroundColor = .clear
 #endif
@@ -127,9 +151,9 @@ public class DittoPresenceView: PlatformView {
         ])
     }
 
-    private func observePresence() {
+    private func startObservingPresence() {
         if let ditto = ditto {
-            peersObserver = ditto.presence.observe { presenceGraph in
+            self.peersObserver = ditto.presence.observe { presenceGraph in
                 let encoder = JSONEncoder()
                 encoder.dataEncodingStrategy = .custom({ data, enc in
                     var container = enc.unkeyedContainer()
@@ -144,16 +168,19 @@ public class DittoPresenceView: PlatformView {
                 }
             }
         }
-
-//        // Comment out the ditto observer above and toggle following to test presence with
-//        // fake data. Several different mock drivers exist:
-//        // - runFrequentConnectionChanges()
-//        // - runFrequentRSSIChanges()
-//        // - runLargeMesh()
-//        // - runMassiveMesh()
-//        MockData.runLargeMesh() { [weak self] json in
-//            self?.webView.updateNetwork(json: json, completionHandler: nil)
-//        }
+        //        // Comment out the ditto observer above and toggle following to test presence with
+        //        // fake data. Several different mock drivers exist:
+        //        // - runFrequentConnectionChanges()
+        //        // - runFrequentRSSIChanges()
+        //        // - runLargeMesh()
+        //        // - runMassiveMesh()
+        //        MockData.runLargeMesh() { [weak self] json in
+        //            self?.webView.updateNetwork(json: json, completionHandler: nil)
+        //        }
     }
 
+    private func stopObservingPresence() {
+        peersObserver?.stop()
+        peersObserver = nil
+    }
 }
diff --git a/Swift/Sources/DittoPresenceViewer/PresenceView.swift b/Swift/Sources/DittoPresenceViewer/PresenceView.swift
@@ -1,6 +1,6 @@
 //
 //  PresenceView.swift
-//  
+//
 //
 //  Created by Ben Chatelain on 9/23/22.
 //
@@ -20,47 +20,43 @@ import AppKit
 #endif
 
 #if canImport(WebKit)
+@available(macOS 10.15, *)
 public struct PresenceView: View {
-    public var ditto: Ditto
+    private var ditto: Ditto
 
     public init(ditto: Ditto) {
         self.ditto = ditto
     }
 
     public var body: some View {
-        PresenceView(ditto: ditto)
+        DittoPresenceViewRepresentable(ditto: ditto)
     }
 }
-#endif
 
 // MARK: - UIViewRepresentable
 #if os(iOS)
-extension PresenceView: UIViewRepresentable {
-    public typealias Body = Never
-    public typealias UIViewType = UIView
+private struct DittoPresenceViewRepresentable: UIViewRepresentable {
+    let ditto: Ditto
 
-    public func makeUIView(context: Self.Context) -> Self.UIViewType {
-        return DittoPresenceView(ditto: self.ditto)
+    func makeUIView(context: Context) -> UIView {
+        return DittoPresenceView(ditto: ditto)
     }
 
-    public func updateUIView(_: Self.UIViewType, context: Self.Context) {
-        return
+    func updateUIView(_ uiView: UIView, context: Context) {
     }
 }
-#endif
 
 // MARK: - NSViewRepresentable
-#if os(macOS)
-extension PresenceView: NSViewRepresentable {
-    public typealias Body = Never
-    public typealias NSViewType = NSView
+#elseif os(macOS)
+private struct DittoPresenceViewRepresentable: NSViewRepresentable {
+    let ditto: Ditto
 
-    public func makeNSView(context: Self.Context) -> Self.NSViewType {
-        return DittoPresenceView(ditto: self.ditto)
+    func makeNSView(context: Context) -> NSView {
+        return DittoPresenceView(ditto: ditto)
     }
 
-    public func updateNSView(_: Self.NSViewType, context: Self.Context) {
-        return
+    func updateNSView(_ nsView: NSView, context: Context) {
     }
 }
 #endif
+#endif