Fix unnecessary retains to prevent zombie Analytics instances (#164)

bsneed · web-flow · commit c2c43989ec70 · 2022-10-17T17:07:08.000-07:00
* Fixed memory leaks

* Added leak testing

* Removed unnecessary subscriber conformance from Timeline.

* Some cleanup

* Analytics property on plugins now needs to be marked `weak`.

* Fixed leak test for linux.
diff --git a/Examples/destination_plugins/AdjustDestination.swift b/Examples/destination_plugins/AdjustDestination.swift
@@ -41,7 +41,7 @@ class AdjustDestination: NSObject, DestinationPlugin, RemoteNotifications {
     let timeline = Timeline()
     let type = PluginType.destination
     let key = "Adjust"
-    var analytics: Analytics? = nil
+    weak var analytics: Analytics? = nil
     
     private var settings: AdjustSettings? = nil
     
diff --git a/Examples/destination_plugins/ComscoreDestination.swift b/Examples/destination_plugins/ComscoreDestination.swift
@@ -44,7 +44,7 @@ class ComscoreDestination: DestinationPlugin {
     let timeline = Timeline()
     let type = PluginType.destination
     let key = "comScore"
-    var analytics: Analytics? = nil
+    weak var analytics: Analytics? = nil
     
     private var comscoreSettings: ComscoreSettings?
     private var comscoreEnrichment: ComscoreEnrichment?
diff --git a/Examples/destination_plugins/ExampleDestination.swift b/Examples/destination_plugins/ExampleDestination.swift
@@ -46,7 +46,7 @@ public class ExampleDestination: DestinationPlugin {
     public let type = PluginType.destination
     // TODO: Fill this out with your settings key that matches your destination in the Segment App
     public let key = "Example"
-    public var analytics: Analytics? = nil
+    public weak var analytics: Analytics? = nil
     
     private var exampleSettings: ExampleSettings?
         
diff --git a/Examples/destination_plugins/FlurryDestination.swift b/Examples/destination_plugins/FlurryDestination.swift
@@ -46,7 +46,7 @@ class FlurryDestination: DestinationPlugin {
     let timeline = Timeline()
     let type = PluginType.destination
     let key = "Flurry"
-    var analytics: Analytics? = nil
+    weak var analytics: Analytics? = nil
     
     var screenTracksEvents = false
     
diff --git a/Examples/destination_plugins/IntercomDestination.swift b/Examples/destination_plugins/IntercomDestination.swift
@@ -43,7 +43,7 @@ class IntercomDestination: DestinationPlugin {
     let timeline = Timeline()
     let type = PluginType.destination
     let key = "Intercom"
-    var analytics: Analytics? = nil
+    weak var analytics: Analytics? = nil
     
     private var intercomSettings: IntercomSettings?
     private var configurationLabels = [String: Any]()
diff --git a/Examples/other_plugins/CellularCarrier.swift b/Examples/other_plugins/CellularCarrier.swift
@@ -54,7 +54,7 @@ import CoreTelephony
 class CellularCarrier: Plugin {
     var type: PluginType = .enrichment
     
-    var analytics: Analytics?
+    weak var analytics: Analytics?
     
     func execute<T: RawEvent>(event: T?) -> T? {
         guard var workingEvent = event else { return event }
diff --git a/Examples/other_plugins/ConsentTracking.swift b/Examples/other_plugins/ConsentTracking.swift
@@ -46,7 +46,7 @@ import UIKit
  */
 class ConsentTracking: Plugin {
     let type = PluginType.before
-    var analytics: Analytics? = nil
+    weak var analytics: Analytics? = nil
     
     var queuedEvents = [RawEvent]()
     
diff --git a/Examples/other_plugins/ConsoleLogger.swift b/Examples/other_plugins/ConsoleLogger.swift
@@ -42,7 +42,7 @@ import Segment
 class ConsoleLogger: Plugin {
     let type = PluginType.after
     let name: String
-    var analytics: Analytics? = nil
+    weak var analytics: Analytics? = nil
     
     var identifier: String? = nil
     
diff --git a/Examples/other_plugins/IDFACollection.swift b/Examples/other_plugins/IDFACollection.swift
@@ -46,7 +46,7 @@ import AppTrackingTransparency
  */
 class IDFACollection: Plugin {
     let type = PluginType.enrichment
-    var analytics: Analytics? = nil
+    weak var analytics: Analytics? = nil
     @Atomic private var alreadyAsked = false
     
     func execute<T: RawEvent>(event: T?) -> T? {
diff --git a/Examples/other_plugins/NotificationTracking.swift b/Examples/other_plugins/NotificationTracking.swift
@@ -40,7 +40,7 @@ import Segment
 
 class NotificationTracking: Plugin {
     var type: PluginType = .utility
-    var analytics: Analytics?
+    weak var analytics: Analytics?
     
     func trackNotification(_ properties: [String: Any], fromLaunch launch: Bool) {
         if launch {
diff --git a/Examples/other_plugins/UIKitScreenTracking.swift b/Examples/other_plugins/UIKitScreenTracking.swift
@@ -51,7 +51,7 @@ class UIKitScreenTracking: UtilityPlugin {
     static let controllerKey = "controller"
     
     let type = PluginType.utility
-    var analytics: Analytics? = nil
+    weak var analytics: Analytics? = nil
     
     init() {
         setupUIKitHooks()
diff --git a/Sources/Segment/Plugins/Context.swift b/Sources/Segment/Plugins/Context.swift
@@ -9,7 +9,7 @@ import Foundation
 
 public class Context: PlatformPlugin {
     public let type: PluginType = .before
-    public var analytics: Analytics?
+    public weak var analytics: Analytics?
     
     internal var staticContext = staticContextData()
     internal static var device = VendorSystem.current
diff --git a/Sources/Segment/Plugins/DestinationMetadataPlugin.swift b/Sources/Segment/Plugins/DestinationMetadataPlugin.swift
@@ -13,7 +13,7 @@ import Foundation
  */
 public class DestinationMetadataPlugin: Plugin {
     public let type: PluginType = PluginType.enrichment
-    public var analytics: Analytics?
+    public weak var analytics: Analytics?
     private var analyticsSettings: Settings? = nil
     
     public func update(settings: Settings, type: UpdateType) {
diff --git a/Sources/Segment/Plugins/DeviceToken.swift b/Sources/Segment/Plugins/DeviceToken.swift
@@ -9,7 +9,7 @@ import Foundation
 
 public class DeviceToken: PlatformPlugin {
     public let type = PluginType.before
-    public var analytics: Analytics?
+    public weak var analytics: Analytics?
     
     public var token: String? = nil
 
diff --git a/Sources/Segment/Plugins/Logger/SegmentLog.swift b/Sources/Segment/Plugins/Logger/SegmentLog.swift
@@ -11,7 +11,7 @@ import Foundation
 
 internal class SegmentLog: UtilityPlugin {
     public var filterKind = LogFilterKind.debug
-    var analytics: Analytics?
+    weak var analytics: Analytics?
     
     let type = PluginType.utility
     
@@ -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 {
diff --git a/Sources/Segment/Plugins/Platforms/Linux/LinuxLifecycleMonitor.swift b/Sources/Segment/Plugins/Platforms/Linux/LinuxLifecycleMonitor.swift
@@ -10,6 +10,6 @@ import Foundation
 #if os(Linux)
 class LinuxLifecycleMonitor: PlatformPlugin {
     let type = PluginType.utility
-    var analytics: Analytics?
+    weak var analytics: Analytics?
 }
 #endif
diff --git a/Sources/Segment/Plugins/Platforms/Mac/macOSLifecycleEvents.swift b/Sources/Segment/Plugins/Platforms/Mac/macOSLifecycleEvents.swift
@@ -16,7 +16,7 @@ class macOSLifecycleEvents: PlatformPlugin, macOSLifecycle {
     static var buildKey = "SEGBuildKeyV2"
     
     let type = PluginType.before
-    var analytics: Analytics?
+    weak var analytics: Analytics?
     
     /// Since application:didFinishLaunchingWithOptions is not automatically called with Scenes / SwiftUI,
     /// this gets around by using a flag in user defaults to check for big events like application updating,
diff --git a/Sources/Segment/Plugins/Platforms/Mac/macOSLifecycleMonitor.swift b/Sources/Segment/Plugins/Platforms/Mac/macOSLifecycleMonitor.swift
@@ -46,7 +46,7 @@ class macOSLifecycleMonitor: PlatformPlugin {
     static var specificName = "Segment_macOSLifecycleMonitor"
     let type = PluginType.utility
     let name = specificName
-    var analytics: Analytics?
+    weak var analytics: Analytics?
     
     private var application: NSApplication
     private var appNotifications: [NSNotification.Name] =
diff --git a/Sources/Segment/Plugins/Platforms/Vendors/AppleUtils.swift b/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 {
diff --git a/Sources/Segment/Plugins/Platforms/iOS/iOSLifecycleEvents.swift b/Sources/Segment/Plugins/Platforms/iOS/iOSLifecycleEvents.swift
@@ -16,7 +16,7 @@ class iOSLifecycleEvents: PlatformPlugin, iOSLifecycle {
     static var buildKey = "SEGBuildKeyV2"
     
     let type = PluginType.before
-    var analytics: Analytics?
+    weak var analytics: Analytics?
     
     /// Since application:didFinishLaunchingWithOptions is not automatically called with Scenes / SwiftUI,
     /// this gets around by using a flag in user defaults to check for big events like application updating,
diff --git a/Sources/Segment/Plugins/Platforms/iOS/iOSLifecycleMonitor.swift b/Sources/Segment/Plugins/Platforms/iOS/iOSLifecycleMonitor.swift
@@ -38,7 +38,7 @@ public extension iOSLifecycle {
 
 class iOSLifecycleMonitor: PlatformPlugin {
     let type = PluginType.utility
-    var analytics: Analytics?
+    weak var analytics: Analytics?
     
     private var application: UIApplication? = nil
     private var appNotifications: [NSNotification.Name] = [UIApplication.didEnterBackgroundNotification,
diff --git a/Sources/Segment/Plugins/Platforms/watchOS/watchOSLifecycleEvents.swift b/Sources/Segment/Plugins/Platforms/watchOS/watchOSLifecycleEvents.swift
@@ -15,7 +15,7 @@ class watchOSLifecycleEvents: PlatformPlugin, watchOSLifecycle {
     static var buildKey = "SEGBuildKeyV2"
     
     let type = PluginType.before
-    var analytics: Analytics?
+    weak var analytics: Analytics?
     
     func applicationDidFinishLaunching(watchExtension: WKExtension) {
         if analytics?.configuration.values.trackApplicationLifecycleEvents == false {
diff --git a/Sources/Segment/Plugins/Platforms/watchOS/watchOSLifecycleMonitor.swift b/Sources/Segment/Plugins/Platforms/watchOS/watchOSLifecycleMonitor.swift
@@ -28,8 +28,8 @@ public extension watchOSLifecycle {
 
 
 class watchOSLifecycleMonitor: PlatformPlugin {
-    var type = PluginType.utility
-    var analytics: Analytics?
+    let type = PluginType.utility
+    weak var analytics: Analytics?
     var wasBackgrounded: Bool = false
     
     private var watchExtension = WKExtension.shared()
diff --git a/Sources/Segment/Plugins/SegmentDestination.swift b/Sources/Segment/Plugins/SegmentDestination.swift
@@ -25,7 +25,7 @@ public class SegmentDestination: DestinationPlugin {
     public let type = PluginType.destination
     public let key: String = Constants.integrationName.rawValue
     public let timeline = Timeline()
-    public var analytics: Analytics? {
+    public weak var analytics: Analytics? {
         didSet {
             initialSetup()
         }
@@ -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())
diff --git a/Sources/Segment/Plugins/StartupQueue.swift b/Sources/Segment/Plugins/StartupQueue.swift
@@ -15,9 +15,11 @@ public class StartupQueue: Plugin, Subscriber {
     
     public let type: PluginType = .before
     
-    public var analytics: Analytics? = nil {
+    public weak var analytics: Analytics? = nil {
         didSet {
-            analytics?.store.subscribe(self, handler: runningUpdate)
+            analytics?.store.subscribe(self) { [weak self] (state: System) in
+                self?.runningUpdate(state: state)
+            }
         }
     }
     
diff --git a/Sources/Segment/Startup.swift b/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()
         }
     }
 }
diff --git a/Sources/Segment/Timeline.swift b/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() {
diff --git a/Sources/Segment/Utilities/HTTPClient.swift b/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 weak var 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
             }
diff --git a/Sources/Segment/Utilities/Storage.swift b/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?) {
diff --git a/Tests/Segment-Tests/MemoryLeak_Tests.swift b/Tests/Segment-Tests/MemoryLeak_Tests.swift
diff --git a/Tests/Segment-Tests/Support/TestUtilities.swift b/Tests/Segment-Tests/Support/TestUtilities.swift
