fix: enhance thread safety of properties (#257)

* fix: enhance thread safety of properties * make timeline diligent
amplitude · Jan 23, 2025 · 8ffcc65 · 8ffcc65
1 parent 98a5f02
commit 8ffcc65
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 47 deletions.
diff --git a/Sources/Amplitude/Amplitude.swift b/Sources/Amplitude/Amplitude.swift
@@ -10,6 +10,7 @@ public class Amplitude {
 
     var state: State = State()
     var contextPlugin: ContextPlugin
+    let timeline = Timeline()
 
     lazy var storage: any Storage = {
         return self.configuration.storageProvider
@@ -19,13 +20,20 @@ public class Amplitude {
         return self.configuration.identifyStorageProvider
     }()
 
-    lazy var timeline: Timeline = {
-        return Timeline()
-    }()
-
-    lazy var sessions: Sessions = {
-        return Sessions(amplitude: self)
-    }()
+    private var sessionsLock = NSLock()
+    private var _sessions: Sessions?
+    var sessions: Sessions {
+        get {
+            sessionsLock.synchronizedLazy(&_sessions) {
+                Sessions(amplitude: self)
+            }
+        }
+        set {
+            sessionsLock.withLock {
+                _sessions = newValue
+            }
+        }
+    }
 
     public lazy var logger: (any Logger)? = {
         return self.configuration.loggerProvider

diff --git a/Sources/Amplitude/Utilities/Atomic.swift b/Sources/Amplitude/Utilities/Atomic.swift
@@ -33,3 +33,16 @@ public struct Atomic<T> {
         value = newValue
     }
 }
+
+extension NSLock {
+    func synchronizedLazy<T>(_ storage: inout T?, initializer: () -> T) -> T {
+        lock()
+        defer { unlock() }
+        if let existing = storage {
+            return existing
+        }
+        let newValue = initializer()
+        storage = newValue
+        return newValue
+    }
+}
diff --git a/Sources/Amplitude/Utilities/Diagonostics.swift b/Sources/Amplitude/Utilities/Diagonostics.swift
@@ -8,22 +8,24 @@
 import Foundation
 
 public class Diagnostics {
-
     private static let MAX_ERROR_LOGS = 10
 
-    private var malformedEvents: [String]?
+    private var malformedEvents: [String] = []
     private var errorLogs = NSMutableOrderedSet(capacity: 10)
 
-    init(){}
+    private let lock = NSLock()
 
     func addMalformedEvent(_ event: String) {
-        if malformedEvents == nil {
-            malformedEvents = [String]()
-        }
-        malformedEvents?.append(event)
+        lock.lock()
+        defer { lock.unlock() }
+
+        malformedEvents.append(event)
     }
 
     func addErrorLog(_ log: String) {
+        lock.lock()
+        defer { lock.unlock() }
+
         errorLogs.add(log)
 
         // trim to MAX_ERROR_LOGS elements
@@ -32,33 +34,31 @@ public class Diagnostics {
         }
     }
 
-    func hasDiagnostics() -> Bool {
-        return (malformedEvents != nil && malformedEvents!.count > 0) || errorLogs.count > 0
-    }
-
     /**
      * Extracts the diagnostics as a JSON string.
      * Warning: This will clear stored diagnostics.
      * @return JSON string of diagnostics or empty if no diagnostics are present.
      */
-     func extractDiagonosticsToString() -> String {
-        if !hasDiagnostics() {
-            return ""
-        }
+    func extractDiagnosticsToString() -> String? {
+        lock.lock()
+        defer { lock.unlock() }
+
+        guard !malformedEvents.isEmpty || errorLogs.count > 0 else { return nil }
+
         var diagnostics = [String: [String]]()
-        if malformedEvents != nil && malformedEvents!.count > 0 {
+        if !malformedEvents.isEmpty {
             diagnostics["malformed_events"] = malformedEvents
         }
-         if errorLogs.count > 0, let errorStrings = errorLogs.array as? [String] {
-             diagnostics["error_logs"] = errorStrings
+        if errorLogs.count > 0, let errorStrings = errorLogs.array as? [String] {
+            diagnostics["error_logs"] = errorStrings
         }
         do {
             let data = try JSONSerialization.data(withJSONObject: diagnostics, options: [])
-            malformedEvents?.removeAll()
+            malformedEvents.removeAll()
             errorLogs.removeAllObjects()
-            return String(data: data, encoding: .utf8) ?? ""
+            return String(data: data, encoding: .utf8)
         } catch {
-            return ""
+            return nil
         }
-     }
+    }
 }
diff --git a/Sources/Amplitude/Utilities/HttpClient.swift b/Sources/Amplitude/Utilities/HttpClient.swift
@@ -121,13 +121,10 @@ class HttpClient {
                 ,"options":{"min_id_length":\(minIdLength)}
                 """
         }
-        if diagnostics.hasDiagnostics() {
-            let diagnosticsInfo = diagnostics.extractDiagonosticsToString()
-            if !diagnosticsInfo.isEmpty {
-                requestPayload += """
+        if let diagnosticsInfo = diagnostics.extractDiagnosticsToString(), !diagnosticsInfo.isEmpty {
+            requestPayload += """
                 ,"request_metadata":{"sdk":\(diagnosticsInfo)}
                 """
-            }
         }
         requestPayload += "}"
         return requestPayload.data(using: .utf8)

diff --git a/Tests/AmplitudeTests/Storages/PersistentStorageTests.swift b/Tests/AmplitudeTests/Storages/PersistentStorageTests.swift
@@ -183,8 +183,7 @@ final class PersistentStorageTests: XCTestCase {
         let data = try? JSONSerialization.data(withJSONObject: malformedArr, options: [])
         let expectedPartial = String(data: data!, encoding: .utf8) ?? ""
         XCTAssertEqual(decodedEvents!.count, 1)
-        XCTAssertTrue(self.diagonostics.hasDiagnostics() == true)
-        XCTAssertEqual(self.diagonostics.extractDiagonosticsToString(), "{\"malformed_events\":\(expectedPartial)}")
+        XCTAssertEqual(self.diagonostics.extractDiagnosticsToString(), "{\"malformed_events\":\(expectedPartial)}")
         persistentStorage.reset()
    }
 

diff --git a/Tests/AmplitudeTests/Utilities/DiagnosticsTests.swift b/Tests/AmplitudeTests/Utilities/DiagnosticsTests.swift
@@ -14,41 +14,40 @@ final class DiagnosticsTests: XCTestCase {
     func testAddMalformedEvent() {
         let diagnostics = Diagnostics()
         diagnostics.addMalformedEvent("event")
-        XCTAssertTrue(diagnostics.hasDiagnostics())
-        XCTAssertEqual(diagnostics.extractDiagonosticsToString(), "{\"malformed_events\":[\"event\"]}")
+        XCTAssertEqual(diagnostics.extractDiagnosticsToString(), "{\"malformed_events\":[\"event\"]}")
     }
 
     func testAddErrorLog() {
         let diagnostics = Diagnostics()
         diagnostics.addErrorLog("log")
-        XCTAssertTrue(diagnostics.hasDiagnostics())
-        XCTAssertEqual(diagnostics.extractDiagonosticsToString(), "{\"error_logs\":[\"log\"]}")
+        XCTAssertEqual(diagnostics.extractDiagnosticsToString(), "{\"error_logs\":[\"log\"]}")
     }
 
     func testHasDiagonostics() {
         let diagnostics = Diagnostics()
-        XCTAssertFalse(diagnostics.hasDiagnostics())
+        XCTAssertNil(diagnostics.extractDiagnosticsToString())
         diagnostics.addMalformedEvent("event")
-        XCTAssertTrue(diagnostics.hasDiagnostics())
+        XCTAssertNotNil(diagnostics.extractDiagnosticsToString())
         diagnostics.addErrorLog("log")
-        XCTAssertTrue(diagnostics.hasDiagnostics())
+        XCTAssertNotNil(diagnostics.extractDiagnosticsToString())
     }
 
     func testExtractDiagnostic() {
         let diagnostics = Diagnostics()
-        XCTAssertEqual(diagnostics.extractDiagonosticsToString(), "")
+        XCTAssertNil(diagnostics.extractDiagnosticsToString())
         diagnostics.addMalformedEvent("event")
         diagnostics.addErrorLog("log")
-        let result = convertToDictionary(text: diagnostics.extractDiagonosticsToString())
+        let result = convertToDictionary(text: diagnostics.extractDiagnosticsToString()!)
         XCTAssertEqual((result?["malformed_events"] as? [String]) ?? [], ["event"])
         XCTAssertEqual((result?["error_logs"] as? [String]) ?? [], ["log"])
+        XCTAssertNil(diagnostics.extractDiagnosticsToString())
     }
 
     func testDedupsErrorLogs() {
         let diagnostics = Diagnostics()
         diagnostics.addErrorLog("dup")
         diagnostics.addErrorLog("dup")
-        let result = convertToDictionary(text: diagnostics.extractDiagonosticsToString())
+        let result = convertToDictionary(text: diagnostics.extractDiagnosticsToString()!)
         XCTAssertEqual(result?["error_logs"] as? [String], ["dup"])
     }
 
@@ -58,7 +57,7 @@ final class DiagnosticsTests: XCTestCase {
         (0..<maxErrorLogs + 1).forEach {
             diagnostics.addErrorLog("\($0)")
         }
-        let result = convertToDictionary(text: diagnostics.extractDiagonosticsToString())
+        let result = convertToDictionary(text: diagnostics.extractDiagnosticsToString()!)
         guard let errorLogs = result?["error_logs"] as? [String] else {
             XCTFail("Unable to extract error logs")
             return