Fix dispatchSource

Braintree.xcodeproj/project.pbxproj

@@ -53,6 +53,8 @@
 		428F976626727333001042E1 /* BTMockOpenURLContext.m in Sources */ = {isa = PBXBuildFile; fileRef = 428F976526727333001042E1 /* BTMockOpenURLContext.m */; };
 		42FC218B25CDE0290047C49A /* BTPayPalRequest_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 42FC218A25CDE0290047C49A /* BTPayPalRequest_Tests.swift */; };
 		42FC237125CE0E110047C49A /* BTPayPalCheckoutRequest_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 42FC237025CE0E110047C49A /* BTPayPalCheckoutRequest_Tests.swift */; };
+		457D7FC82C29CEC300EF6523 /* RepeatingTimer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 457D7FC72C29CEC300EF6523 /* RepeatingTimer.swift */; };
+		457D7FCA2C2A250E00EF6523 /* RepeatingTimer_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 457D7FC92C2A250E00EF6523 /* RepeatingTimer_Tests.swift */; };
 		460C0C220F594AE8EE205E57 /* Pods_Tests_BraintreeCoreTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 9239C9FE850C3587DE61A3A2 /* Pods_Tests_BraintreeCoreTests.framework */; };
 		5708E0A628809AD9007946B9 /* BTJSON.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5708E0A528809AD9007946B9 /* BTJSON.swift */; };
 		5708E0A828809BC6007946B9 /* BTJSONError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5708E0A728809BC6007946B9 /* BTJSONError.swift */; };
@@ -772,6 +774,8 @@
 		42F75E5A24D48138007DC5E7 /* BTThreeDSecureClient_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTThreeDSecureClient_Tests.swift; sourceTree = "<group>"; };
 		42FC218A25CDE0290047C49A /* BTPayPalRequest_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTPayPalRequest_Tests.swift; sourceTree = "<group>"; };
 		42FC237025CE0E110047C49A /* BTPayPalCheckoutRequest_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTPayPalCheckoutRequest_Tests.swift; sourceTree = "<group>"; };
+		457D7FC72C29CEC300EF6523 /* RepeatingTimer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RepeatingTimer.swift; sourceTree = "<group>"; };
+		457D7FC92C2A250E00EF6523 /* RepeatingTimer_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RepeatingTimer_Tests.swift; sourceTree = "<group>"; };
 		463DED22C0F426A474E6D7E2 /* Pods-Tests-BraintreeCoreTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Tests-BraintreeCoreTests.release.xcconfig"; path = "Target Support Files/Pods-Tests-BraintreeCoreTests/Pods-Tests-BraintreeCoreTests.release.xcconfig"; sourceTree = "<group>"; };
 		541AEE40A1F01913E0638CC9 /* Pods-Tests-BraintreeCoreTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Tests-BraintreeCoreTests.debug.xcconfig"; path = "Target Support Files/Pods-Tests-BraintreeCoreTests/Pods-Tests-BraintreeCoreTests.debug.xcconfig"; sourceTree = "<group>"; };
 		5708E0A528809AD9007946B9 /* BTJSON.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTJSON.swift; sourceTree = "<group>"; };
@@ -1489,8 +1493,9 @@
 			isa = PBXGroup;
 			children = (
 				BEE2E4A628FDB64400C03FDD /* BTAnalyticsService_Tests.swift */,
-				8053F05329FAB6B00076F988 /* FPTIBatchData_Tests.swift */,
 				BEDA919F28EDDE64007441D9 /* FakeAnalyticsService.swift */,
+				8053F05329FAB6B00076F988 /* FPTIBatchData_Tests.swift */,
+				457D7FC92C2A250E00EF6523 /* RepeatingTimer_Tests.swift */,
 			);
 			path = Analytics;
 			sourceTree = "<group>";
@@ -1556,6 +1561,7 @@
 				BEE2E4E5290080BD00C03FDD /* BTAnalyticsServiceError.swift */,
 				BEF388C02BE52CD2000965C8 /* BTCoreAnalytics.swift */,
 				800E78C329E0DD5300D1B0FC /* FPTIBatchData.swift */,
+				457D7FC72C29CEC300EF6523 /* RepeatingTimer.swift */,
 			);
 			path = Analytics;
 			sourceTree = "<group>";
@@ -3245,6 +3251,7 @@
 				804DC45D2B2D08FF00F17A15 /* BTConfigurationRequest.swift in Sources */,
 				BED00CB228A57AD400D74AEC /* BTClientTokenError.swift in Sources */,
 				BE24C67328E73E810067B11A /* BTAPIClientHTTPType.swift in Sources */,
+				457D7FC82C29CEC300EF6523 /* RepeatingTimer.swift in Sources */,
 				BE9EC0982899CF040022EC63 /* BTAPIPinnedCertificates.swift in Sources */,
 				BEF1089029019CAE003B2FDA /* BTAnalyticsService.swift in Sources */,
 				BEF388C32BE9340C000965C8 /* BTAPITimingDelegate.swift in Sources */,
@@ -3564,6 +3571,7 @@
 				A9E5C23424FD6FAE00EE691F /* BTAPIClient_Tests.swift in Sources */,
 				8053F05729FAD4790076F988 /* Encodable+Dictionary_Tests.swift in Sources */,
 				BEE2E4A728FDB64400C03FDD /* BTAnalyticsService_Tests.swift in Sources */,
+				457D7FCA2C2A250E00EF6523 /* RepeatingTimer_Tests.swift in Sources */,
 				A95229C424FD8EEE006F7D25 /* BTURLUtils_Tests.swift in Sources */,
 				A908436324FD82C0004134CA /* BTBinData_Tests.swift in Sources */,
 				A908436224FD82A9004134CA /* BTAppContextSwitcher_Tests.swift in Sources */,

CHANGELOG.md

@@ -4,6 +4,7 @@
 * BraintreeCore
   * For analytics, only call `fetchOrReturnRemoteConfig()` when batch uploading, not on each analytic event enqueue
   * For analytics, add additional metrics on networking timing
+  * Fix bug causing random crashes in `BTAnalyticsService`
 
 ## 6.21.0 (2024-06-12)
 * BraintreePayPal

Sources/BraintreeCore/Analytics/BTAnalyticsService.swift

@@ -14,26 +14,35 @@ class BTAnalyticsService: Equatable {
     var shouldBypassTimerQueue = false
 
     // MARK: - Private Properties
-
+    
     private static let events = BTAnalyticsEventsStorage()
-
-    private let apiClient: BTAPIClient
+
     /// Amount of time, in seconds, between batch API requests sent to FPTI
-    private let timerInterval = 15
+    private static let timeInterval = 15
+
+    private static let timer = RepeatingTimer(timeInterval: timeInterval)
 
-    private static var timer: DispatchSourceTimer?
+    private let apiClient: BTAPIClient
 
     // MARK: - Initializer
 
     init(apiClient: BTAPIClient) {
         self.apiClient = apiClient
+        self.http = BTHTTP(authorization: apiClient.authorization, customBaseURL: Self.url)
+
+        Self.timer.eventHandler = { [weak self] in
+            guard let self else { return }
+            Task {
+                await self.sendQueuedAnalyticsEvents()
+            }
+        }
+        Self.timer.resume()
     }
 
     // MARK: - Deinit
 
     deinit {
-        BTAnalyticsService.timer?.cancel()
-        BTAnalyticsService.timer = nil
+        Self.timer.suspend()
     }
 
     // MARK: - Internal Methods
@@ -111,36 +120,8 @@ class BTAnalyticsService: Equatable {
 
         await BTAnalyticsService.events.append(event)
 
-        // TODO: - Refactor to make HTTP non-optional property and instantiate in init()
-        if self.http == nil {
-            self.http = BTHTTP(authorization: self.apiClient.authorization, customBaseURL: BTAnalyticsService.url)
-        }
-
-        // A special value passed in by unit tests to prevent BTHTTP from actually posting
-        if let http = self.http, http.customBaseURL?.absoluteString == "test://do-not-send.url" {
-            return
-        }
-
         if shouldBypassTimerQueue {
             await self.sendQueuedAnalyticsEvents()
-            return
-        }
-
-        if BTAnalyticsService.timer == nil {
-            BTAnalyticsService.timer = DispatchSource.makeTimerSource(queue: self.http?.dispatchQueue)
-            BTAnalyticsService.timer?.schedule(
-                deadline: .now() + .seconds(self.timerInterval),
-                repeating: .seconds(self.timerInterval),
-                leeway: .seconds(1)
-            )
-
-            BTAnalyticsService.timer?.setEventHandler {
-                Task {
-                    await self.sendQueuedAnalyticsEvents()
-                }
-            }
-
-            BTAnalyticsService.timer?.resume()
         }
     }
 
@@ -150,9 +131,9 @@ class BTAnalyticsService: Equatable {
         if await !BTAnalyticsService.events.isEmpty {
             do {
                 let configuration = try await apiClient.fetchConfiguration()
-                let postParameters = await createAnalyticsEvent(config: configuration, sessionID: apiClient.metadata.sessionID, events: BTAnalyticsService.events.allValues)
+                let postParameters = await createAnalyticsEvent(config: configuration, sessionID: apiClient.metadata.sessionID, events: Self.events.allValues)
                 http?.post("v1/tracking/batch/events", parameters: postParameters) { _, _, _ in }
-                await BTAnalyticsService.events.removeAll()
+                await Self.events.removeAll()
             } catch {
                 return
             }

Sources/BraintreeCore/Analytics/RepeatingTimer.swift

@@ -0,0 +1,77 @@
+import Foundation
+
+final class RepeatingTimer {
+
+    // MARK: - Private Properties
+
+    /// Amount of time, in seconds.
+    private let timeInterval: Int
+
+    private lazy var timer: DispatchSourceTimer = {
+        let timerSource = DispatchSource.makeTimerSource(queue: DispatchQueue.main)
+        timerSource.schedule(
+            deadline: .now() + .seconds(timeInterval),
+            repeating: .seconds(timeInterval),
+            leeway: .seconds(1)
+        )
+        timerSource.setEventHandler { [weak self] in
+            self?.eventHandler?()
+        }
+        return timerSource
+    }()
+
+    private var state: State = .suspended
+
+    private enum State {
+        case suspended
+        case resumed
+    }
+
+    // MARK: - Internal Properties
+
+    var eventHandler: (() -> Void)?
+
+    // MARK: - Initializer
+
+    init(timeInterval: Int) {
+        self.timeInterval = timeInterval
+    }
+
+    deinit {
+        timer.setEventHandler { }
+        timer.cancel()
+        // If the timer is suspended, calling cancel without resuming afterwards
+        // triggers a crash. This is documented here https://forums.developer.apple.com/thread/15902
+        resume()
+        eventHandler = nil
+    }
+
+    // MARK: - Internal Methods
+
+    /*
+     GCD timers are sensitive to errors. It is crucial to maintain
+     balance between calls to `dispatch_suspend` and `dispatch_resume`. Failure to do so
+     results in crashes with errors similar to:
+
+       BUG IN CLIENT OF LIBDISPATCH: Over-resume of an object
+
+     Such errors indicate an attempt to resume an already resumed timer. According to the
+     documentation, each call to `dispatch_suspend` must be matched with a corresponding
+     call to `dispatch_resume` to ensure proper event delivery and avoid crashes.
+     For more information: https://developer.apple.com/library/archive/documentation/General/Conceptual/ConcurrencyProgrammingGuide/GCDWorkQueues/GCDWorkQueues.html#//apple_ref/doc/uid/TP40008091-CH103-SW8
+    */
+
+    func resume() {
+        guard state != .resumed else { return }
+
+        state = .resumed
+        timer.resume()
+    }
+
+    func suspend() {
+        guard state != .suspended else { return }
+
+        state = .suspended
+        timer.suspend()
+    }
+}

UnitTests/BraintreeCoreTests/Analytics/BTAnalyticsService_Tests.swift

@@ -34,12 +34,12 @@ final class BTAnalyticsService_Tests: XCTestCase {
 
         XCTAssertEqual(mockAnalyticsHTTP.lastRequestEndpoint, "v1/tracking/batch/events")
 
-        let timestamp = self.parseTimestamp(mockAnalyticsHTTP.lastRequestParameters)!
-        let eventName = self.parseEventName(mockAnalyticsHTTP.lastRequestParameters)
+        let timestamp = parseTimestamp(mockAnalyticsHTTP.lastRequestParameters)!
+        let eventName = parseEventName(mockAnalyticsHTTP.lastRequestParameters)
 
         XCTAssertEqual(eventName!, "any.analytics.event")
-        XCTAssertGreaterThanOrEqual(timestamp, self.currentTime)
-        XCTAssertLessThanOrEqual(timestamp, self.oneSecondLater)
+        XCTAssertGreaterThanOrEqual(timestamp, currentTime)
+        XCTAssertLessThanOrEqual(timestamp, oneSecondLater)
         XCTAssertEqual(mockAnalyticsHTTP.POSTRequestCount, 1)
 
         self.validateMetadataParameters(mockAnalyticsHTTP.lastRequestParameters)

UnitTests/BraintreeCoreTests/Analytics/RepeatingTimer_Tests.swift

@@ -0,0 +1,52 @@
+import XCTest
+@testable import BraintreeCore
+
+class RepeatingTimerTests: XCTestCase {
+
+    func testResume_callsEventHandler() {
+        let sut = RepeatingTimer(timeInterval: 2)
+        let expectation = expectation(description: "Timer should fire")
+        var handlerCalled = false
+
+        sut.eventHandler = {
+            handlerCalled = true
+            expectation.fulfill()
+        }
+
+        sut.resume()
+        waitForExpectations(timeout: 3, handler: nil)
+        XCTAssertTrue(handlerCalled, "Event handler should be called after timer resumes")
+    }
+
+    func testSuspend_preventsEventHandler() {
+        let sut = RepeatingTimer(timeInterval: 2)
+        let expectation = expectation(description: "Timer should not fire after suspension")
+        var handlerCalled = false
+
+        sut.eventHandler = {
+            handlerCalled = true
+        }
+
+        sut.resume()
+        sut.suspend()
+
+        DispatchQueue.main.asyncAfter(deadline: .now() + 3) {
+            expectation.fulfill()
+        }
+
+        waitForExpectations(timeout: 4, handler: nil)
+        XCTAssertFalse(handlerCalled, "Event handler should not be called after timer is suspended")
+    }
+
+    func testDeinit() {
+        var sut: RepeatingTimer? = RepeatingTimer(timeInterval: 1)
+        sut?.resume()
+
+        addTeardownBlock {
+            sut?.suspend()
+        }
+
+        sut = nil
+        XCTAssertNil(sut, "Timer should be deallocated")
+    }
+}