Skip to content

Commit 6aecf11

Browse files
committed
Clarify comments on versioning and race conditions
Improved documentation in EvaluationStorage and its tests to clarify the behavior of the version counter and update flag, especially regarding race conditions and expected outcomes during concurrent operations.
1 parent 7d2fbaf commit 6aecf11

File tree

2 files changed

+8
-7
lines changed

2 files changed

+8
-7
lines changed

Bucketeer/Sources/Internal/Evaluation/EvaluationStorage.swift

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,11 +51,10 @@ protocol EvaluationStorage {
5151
/// whole across app restarts.
5252
///
5353
/// The ephemeral nature of the version counter is intentional. We employ an Optimistic Locking pattern where the integer value does not need to persist across app restarts:
54-
/// On Restart: The persisted isUpdated flag is authoritative. If true, we trigger an immediate sync, regardless of the version.
55-
/// During Runtime: The version acts as a transaction ID to safely handle race conditions where user attributes change while a background fetch is in progress.
56-
/// Correctness: The update flag is only cleared via a Compare-and-Swap check (currentVersion == capturedVersion).
57-
/// If the user modifies attributes during a fetch, the version increments, the check fails, and the flag remains true to schedule a subsequent sync.
58-
54+
/// - On restart: The persisted `isUpdated` flag is authoritative. If `true`, an immediate sync is triggered, regardless of the version.
55+
/// - During runtime: The `version` acts as a transaction ID to safely handle race conditions where user attributes change while a background fetch is in progress.
56+
/// - Correctness: The update flag is only cleared via a compare-and-swap check (`currentVersion == capturedVersion`).
57+
/// If the user modifies attributes during a fetch, the `version` increments, the check fails, and the flag remains `true` to schedule a subsequent sync.
5958
struct UserAttributesState {
6059
let version: Int
6160
let isUpdated: Bool

BucketeerTests/EvaluationStorageTests.swift

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,10 @@ final class EvaluationStorageTests: XCTestCase {
324324
// Wait for all operations to complete
325325
let result = group.wait(timeout: .now() + 10.0)
326326
XCTAssertEqual(result, .success, "Test timed out")
327-
// In 99.9% of runs, storage.userAttributesUpdated resolves to false, but we cannot guarantee this behavior every time due to async nature.
328-
// The critical check is that the version counter matches the number of updates, proving no race conditions when incrementing.
327+
// Due to the inherent race between concurrent "update" and "clear" operations, the final value of
328+
// `userAttributesUpdated` can legitimately be either true or false. This test therefore only asserts
329+
// that the version counter matches the number of update calls, proving there are no race conditions
330+
// when incrementing the version.
329331
let userAttributesState = storage.userAttributesState
330332
XCTAssertEqual(userAttributesState.version, iterations, "Version should exactly match the number of update calls, proving no race conditions")
331333
}

0 commit comments

Comments
 (0)