fix: Atomic increment is now performed within one enqueued operation (#330)

* Version 1.5.10

* fix: Atomic increment is now performed within one enqueued operation

---------

Co-authored-by: Brandon Sneed <[email protected]>

Author: TizianoCoroneo

Examples/other_plugins/IDFACollection.swift

@@ -77,10 +77,15 @@ class IDFACollection: Plugin {
 extension IDFACollection: iOSLifecycle {
     func applicationDidBecomeActive(application: UIApplication?) {
         let status = ATTrackingManager.trackingAuthorizationStatus
-        if status == .notDetermined && !alreadyAsked {
-            // we don't know, so should ask the user.
-            alreadyAsked = true
-            askForPermission()
+
+        _alreadyAsked.withValue { alreadyAsked in
+            if status == .notDetermined && !alreadyAsked {
+                // we don't know, so should ask the user.
+                alreadyAsked = true
+                DispatchQueue.main.async {
+                    askForPermission()
+                }
+            }
         }
     }
 }

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

@@ -76,6 +76,9 @@ internal class iOSVendorSystem: VendorSystem {
         // BKS: It was discovered that on some platforms there can be a delay in retrieval.
         // It has to be fetched on the main thread, so we've spun it off
         // async and cache it when it comes back.
+        // Note that due to how the `@Atomic` wrapper works, this boolean check may pass twice or more
+        // times before the value is updated, fetching the user agent multiple times as the result.
+        // This is not a big deal as the `userAgent` value is not expected to change often.
         if Self.asyncUserAgent == nil {
             DispatchQueue.main.async {
                 Self.asyncUserAgent = WKWebView().value(forKey: "userAgent") as? String
@@ -248,6 +251,9 @@ internal class MacOSVendorSystem: VendorSystem {
         // BKS: It was discovered that on some platforms there can be a delay in retrieval.
         // It has to be fetched on the main thread, so we've spun it off
         // async and cache it when it comes back.
+        // Note that due to how the `@Atomic` wrapper works, this boolean check may pass twice or more
+        // times before the value is updated, fetching the user agent multiple times as the result.
+        // This is not a big deal as the `userAgent` value is not expected to change often.
         if Self.asyncUserAgent == nil {
             DispatchQueue.main.async {
                 Self.asyncUserAgent = WKWebView().value(forKey: "userAgent") as? String

Sources/Segment/Plugins/SegmentDestination.swift

@@ -116,7 +116,9 @@ public class SegmentDestination: DestinationPlugin, Subscriber, FlushCompletion
         guard let storage = self.storage else { return }
         // Send Event to File System
         storage.write(.events, value: event)
-        eventCount += 1
+        self._eventCount.withValue { count in
+            count += 1
+        }
     }
 
     public func flush() {

Sources/Segment/Utilities/Atomic.swift

@@ -29,4 +29,12 @@ public class Atomic<T> {
         get { return queue.sync { return value } }
         set { queue.sync { value = newValue } }
     }
+
+    @discardableResult
+    public func withValue(_ operation: (inout T) -> Void) -> T {
+        queue.sync {
+            operation(&self.value)
+            return self.value
+        }
+    }
 }

Sources/Segment/Utilities/Policies/CountBasedFlushPolicy.swift

@@ -37,7 +37,9 @@ public class CountBasedFlushPolicy: FlushPolicy {
     }
 
    public func updateState(event: RawEvent) {
-        count += 1
+       _count.withValue { value in
+           value += 1
+       }
     }
 
     public func reset() {

Tests/Segment-Tests/Atomic_Tests.swift

@@ -0,0 +1,23 @@
+import XCTest
+@testable import Segment
+
+final class Atomic_Tests: XCTestCase {
+
+    func testAtomicIncrement() {
+
+        @Atomic var counter = 0
+
+        DispatchQueue.concurrentPerform(iterations: 1000) { _ in
+            // counter += 1 would fail, because it is expanded to:
+            // `let oldValue = queue.sync { counter }`
+            // `queue.sync { counter = oldValue + 1 }`
+            // And the threads are free to suspend in between the two calls to `queue.sync`.
+
+            _counter.withValue { value in
+                value += 1
+            }
+        }
+
+        XCTAssertEqual(counter, 1000)
+    }
+}