Fix unnecessary retains to prevent zombie Analytics instances

Sources/Segment/Plugins/Logger/SegmentLog.swift

@@ -22,7 +22,7 @@ internal class SegmentLog: UtilityPlugin {
 
     // For internal use only. Note: This will contain the last created instance
     // of analytics when used in a multi-analytics environment.
-    internal static var sharedAnalytics: Analytics? = nil
+    internal static weak var sharedAnalytics: Analytics? = nil
 
     #if DEBUG
     internal static var globalLogger: SegmentLog {

Sources/Segment/Plugins/Platforms/Vendors/AppleUtils.swift

@@ -172,7 +172,7 @@ internal class watchOSVendorSystem: VendorSystem {
     }
 
     override var requiredPlugins: [PlatformPlugin] {
-        return [watchOSLifecycleMonitor()]
+        return [watchOSLifecycleMonitor(), DeviceToken()]
     }
 
     private func deviceModel() -> String {

Sources/Segment/Plugins/Platforms/watchOS/watchOSLifecycleMonitor.swift

@@ -28,7 +28,7 @@ public extension watchOSLifecycle {
 
 
 class watchOSLifecycleMonitor: PlatformPlugin {
-    var type = PluginType.utility
+    let type = PluginType.utility
     var analytics: Analytics?
     var wasBackgrounded: Bool = false
 

Sources/Segment/Plugins/SegmentDestination.swift

@@ -54,8 +54,8 @@ public class SegmentDestination: DestinationPlugin {
         guard let analytics = self.analytics else { return }
         storage = analytics.storage
         httpClient = HTTPClient(analytics: analytics)
-        flushTimer = QueueTimer(interval: analytics.configuration.values.flushInterval) {
-            self.flush()
+        flushTimer = QueueTimer(interval: analytics.configuration.values.flushInterval) { [weak self] in
+            self?.flush()
         }
         // Add DestinationMetadata enrichment plugin
         add(plugin: DestinationMetadataPlugin())

Sources/Segment/Plugins/StartupQueue.swift

@@ -17,7 +17,9 @@ public class StartupQueue: Plugin, Subscriber {
 
     public var analytics: Analytics? = nil {
         didSet {
-            analytics?.store.subscribe(self, handler: runningUpdate)
+            analytics?.store.subscribe(self) { [weak self] (state: System) in
+                self?.runningUpdate(state: state)
+            }
         }
     }
 

Sources/Segment/Startup.swift

@@ -76,10 +76,10 @@ extension Analytics {
         // do the first one
         checkSettings()
         // set up return-from-background to do it again.
-        NotificationCenter.default.addObserver(forName: UIApplication.willEnterForegroundNotification, object: nil, queue: OperationQueue.main) { (notification) in
+        NotificationCenter.default.addObserver(forName: UIApplication.willEnterForegroundNotification, object: nil, queue: OperationQueue.main) { [weak self] (notification) in
             guard let app = notification.object as? UIApplication else { return }
             if app.applicationState == .background {
-                self.checkSettings()
+                self?.checkSettings()
             }
         }
     }
@@ -100,8 +100,8 @@ extension Analytics {
         // now set up a timer to do it every 24 hrs.
         // mac apps change focus a lot more than iOS apps, so this
         // seems more appropriate here.
-        QueueTimer.schedule(interval: .days(1), queue: .main) {
-            self.checkSettings()
+        QueueTimer.schedule(interval: .days(1), queue: .main) { [weak self] in
+            self?.checkSettings()
         }
     }
 }

Sources/Segment/Timeline.swift

@@ -11,7 +11,7 @@ import Sovran
 
 // MARK: - Main Timeline
 
-public class Timeline: Subscriber {
+public class Timeline {
     internal let plugins: [PluginType: Mediator]
 
     public init() {

Sources/Segment/Utilities/HTTPClient.swift

@@ -24,7 +24,8 @@ public class HTTPClient {
     private var apiHost: String
     private var apiKey: String
     private var cdnHost: String
-    private let analytics: Analytics
+
+    private let analytics: Analytics?
 
     init(analytics: Analytics, apiKey: String? = nil, apiHost: String? = nil, cdnHost: String? = nil) {
         self.analytics = analytics
@@ -75,24 +76,24 @@ public class HTTPClient {
 
         let dataTask = session.uploadTask(with: urlRequest, fromFile: batch) { [weak self] (data, response, error) in
             if let error = error {
-                self?.analytics.log(message: "Error uploading request \(error.localizedDescription).")
+                self?.analytics?.log(message: "Error uploading request \(error.localizedDescription).")
                 completion(.failure(error))
             } else if let httpResponse = response as? HTTPURLResponse {
                 switch (httpResponse.statusCode) {
                 case 1..<300:
                     completion(.success(true))
                     return
                 case 300..<400:
-                    self?.analytics.log(message: "Server responded with unexpected HTTP code \(httpResponse.statusCode).")
+                    self?.analytics?.log(message: "Server responded with unexpected HTTP code \(httpResponse.statusCode).")
                     completion(.failure(HTTPClientErrors.statusCode(code: httpResponse.statusCode)))
                 case 429:
-                    self?.analytics.log(message: "Server limited client with response code \(httpResponse.statusCode).")
+                    self?.analytics?.log(message: "Server limited client with response code \(httpResponse.statusCode).")
                     completion(.failure(HTTPClientErrors.statusCode(code: httpResponse.statusCode)))
                 case 400..<500:
-                    self?.analytics.log(message: "Server rejected payload with HTTP code \(httpResponse.statusCode).")
+                    self?.analytics?.log(message: "Server rejected payload with HTTP code \(httpResponse.statusCode).")
                     completion(.failure(HTTPClientErrors.statusCode(code: httpResponse.statusCode)))
                 default: // All 500 codes
-                    self?.analytics.log(message: "Server rejected payload with HTTP code \(httpResponse.statusCode).")
+                    self?.analytics?.log(message: "Server rejected payload with HTTP code \(httpResponse.statusCode).")
                     completion(.failure(HTTPClientErrors.statusCode(code: httpResponse.statusCode)))
                 }
             }
@@ -113,21 +114,21 @@ public class HTTPClient {
 
         let dataTask = session.dataTask(with: urlRequest) { [weak self] (data, response, error) in
             if let error = error {
-                self?.analytics.log(message: "Error fetching settings \(error.localizedDescription).")
+                self?.analytics?.log(message: "Error fetching settings \(error.localizedDescription).")
                 completion(false, nil)
                 return
             }
 
             if let httpResponse = response as? HTTPURLResponse {
                 if httpResponse.statusCode > 300 {
-                    self?.analytics.log(message: "Server responded with unexpected HTTP code \(httpResponse.statusCode).")
+                    self?.analytics?.log(message: "Server responded with unexpected HTTP code \(httpResponse.statusCode).")
                     completion(false, nil)
                     return
                 }
             }
 
             guard let data = data, let responseJSON = try? JSONDecoder().decode(Settings.self, from: data) else {
-                self?.analytics.log(message: "Error deserializing settings.")
+                self?.analytics?.log(message: "Error deserializing settings.")
                 completion(false, nil)
                 return
             }

Sources/Segment/Utilities/Storage.swift

@@ -9,7 +9,6 @@ import Foundation
 import Sovran
 
 internal class Storage: Subscriber {
-    let store: Store
     let writeKey: String
     let userDefaults: UserDefaults?
     static let MAXFILESIZE = 475000 // Server accepts max 500k per batch
@@ -21,11 +20,14 @@ internal class Storage: Subscriber {
     private var fileHandle: FileHandle? = nil
 
     init(store: Store, writeKey: String) {
-        self.store = store
         self.writeKey = writeKey
         self.userDefaults = UserDefaults(suiteName: "com.segment.storage.\(writeKey)")
-        store.subscribe(self, handler: userInfoUpdate)
-        store.subscribe(self, handler: systemUpdate)
+        store.subscribe(self) { [weak self] (state: UserInfo) in
+            self?.userInfoUpdate(state: state)
+        }
+        store.subscribe(self) { [weak self] (state: System) in
+            self?.systemUpdate(state: state)
+        }
     }
 
     func write<T: Codable>(_ key: Storage.Constants, value: T?) {

Tests/Segment-Tests/MemoryLeak_Tests.swift

@@ -0,0 +1,100 @@
+//
+//  MemoryLeak_Tests.swift
+//  
+//
+//  Created by Brandon Sneed on 10/17/22.
+//
+
+import XCTest
+@testable import Segment
+
+final class MemoryLeak_Tests: XCTestCase {
+
+    override func setUpWithError() throws {
+        // Put setup code here. This method is called before the invocation of each test method in the class.
+    }
+
+    override func tearDownWithError() throws {
+        // Put teardown code here. This method is called after the invocation of each test method in the class.
+    }
+
+    func testLeaksVerbose() throws {
+        let analytics = Analytics(configuration: Configuration(writeKey: "1234"))
+
+        waitUntilStarted(analytics: analytics)
+        analytics.track(name: "test")
+
+        RunLoop.main.run(until: Date(timeIntervalSinceNow: 1))
+
+        let segmentDest = analytics.find(pluginType: SegmentDestination.self)!
+        let destMetadata = segmentDest.timeline.find(pluginType: DestinationMetadataPlugin.self)!
+        let startupQueue = analytics.find(pluginType: StartupQueue.self)!
+        let segmentLog = analytics.find(pluginType: SegmentLog.self)!
+
+        let context = analytics.find(pluginType: Context.self)!
+        let deviceToken = analytics.find(pluginType: DeviceToken.self)!
+
+        #if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
+        let iosLifecycle = analytics.find(pluginType: iOSLifecycleEvents.self)!
+        let iosMonitor = analytics.find(pluginType: iOSLifecycleMonitor.self)!
+        #elseif os(watchOS)
+        let watchLifecycle = analytics.find(pluginType: watchOSLifecycleEvents.self)!
+        let watchMonitor = analytics.find(pluginType: watchOSLifecycleMonitor.self)!
+        #elseif os(macOS)
+        let macLifecycle = analytics.find(pluginType: macOSLifecycleEvents.self)!
+        let macMonitor = analytics.find(pluginType: macOSLifecycleMonitor.self)!
+        #endif
+
+        analytics.remove(plugin: startupQueue)
+        analytics.remove(plugin: segmentLog)
+        analytics.remove(plugin: segmentDest)
+        segmentDest.remove(plugin: destMetadata)
+
+        analytics.remove(plugin: context)
+        analytics.remove(plugin: deviceToken)
+        #if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
+        analytics.remove(plugin: iosLifecycle)
+        analytics.remove(plugin: iosMonitor)
+        #elseif os(watchOS)
+        analytics.remove(plugin: watchLifecycle)
+        analytics.remove(plugin: watchMonitor)
+        #elseif os(macOS)
+        analytics.remove(plugin: macLifecycle)
+        analytics.remove(plugin: macMonitor)
+        #endif
+
+        RunLoop.main.run(until: Date(timeIntervalSinceNow: 1))
+
+        checkIfLeaked(segmentLog)
+        checkIfLeaked(segmentDest)
+        checkIfLeaked(destMetadata)
+        checkIfLeaked(startupQueue)
+
+        checkIfLeaked(deviceToken)
+        checkIfLeaked(context)
+        #if os(iOS) || os(tvOS) || targetEnvironment(macCatalyst)
+        checkIfLeaked(iosLifecycle)
+        checkIfLeaked(iosMonitor)
+        #elseif os(watchOS)
+        checkIfLeaked(watchLifecycle)
+        checkIfLeaked(watchMonitor)
+        #elseif os(macOS)
+        checkIfLeaked(macLifecycle)
+        checkIfLeaked(macMonitor)
+        #endif
+
+        checkIfLeaked(analytics)
+    }
+
+    func testLeaksSimple() throws {
+        let analytics = Analytics(configuration: Configuration(writeKey: "1234"))
+
+        waitUntilStarted(analytics: analytics)
+        analytics.track(name: "test")
+
+        RunLoop.main.run(until: Date(timeIntervalSinceNow: 1))
+
+        checkIfLeaked(analytics)
+    }
+
+}

Tests/Segment-Tests/Support/TestUtilities.swift

@@ -6,6 +6,7 @@
 //
 
 import Foundation
+import XCTest
 @testable import Segment
 
 extension UUID{
@@ -133,3 +134,14 @@ func waitUntilStarted(analytics: Analytics?) {
         }
     }
 }
+
+extension XCTestCase {
+    func checkIfLeaked(_ instance: AnyObject, file: StaticString = #filePath, line: UInt = #line) {
+        addTeardownBlock { [weak instance] in
+            if instance != nil {
+                print("Instance \(String(describing: instance)) is not nil")
+            }
+            XCTAssertNil(instance, "Instance should have been deallocated. Potential memory leak!", file: file, line: line)
+        }
+    }
+}