Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions Examples/other_plugins/IDFACollection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions Sources/Segment/Plugins/Platforms/Vendors/AppleUtils.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion Sources/Segment/Plugins/SegmentDestination.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
8 changes: 8 additions & 0 deletions Sources/Segment/Utilities/Atomic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ public class CountBasedFlushPolicy: FlushPolicy {
}

public func updateState(event: RawEvent) {
count += 1
_count.withValue { value in
value += 1
}
}

public func reset() {
Expand Down
2 changes: 1 addition & 1 deletion Sources/Segment/Version.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@
// Use release.sh's automation.

// BREAKING.FEATURE.FIX
internal let __segment_version = "1.5.9"
internal let __segment_version = "1.5.10"
23 changes: 23 additions & 0 deletions Tests/Segment-Tests/Atomic_Tests.swift
Original file line number Diff line number Diff line change
@@ -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)
}
}