Skip to content

Commit dd23929

Browse files
fix: Prevent race condition on updateContext (#84)
## This PR 1. Fixes possible race conditions detected when running updateEvaluationContextAndWait in parallel 2. Adds AsyncProviderOperationsQueue for safe and optimized processing of background context resolving 3. Fix bug where `ready` could be emitted when a context is set without any provider set 4. Adds `clearProviderAndWait` to complement the same pattern in setProvider and setEvaluationContext ### Notes Stacktrace for the original crash (from point 1 above): ``` Task 22150 Queue : com.apple.root.default-qos.cooperative (concurrent) #0 0x00000001970125c4 in _swift_release_dealloc () #1 0x0000000197013094 in swift::RefCounts<swift::RefCountBitsT<(swift::RefCountInlinedness)1>>::doDecrementSlow<(swift::PerformDeinit)1> () #2 0x00000001129667f8 in __swift_destroy_boxed_opaque_existential_1 () #3 0x000000011296a844 in outlined destroy of EvaluationContext? () #4 0x0000000112975098 in OpenFeatureAPI.updateContext(evaluationContext:) at /Users/XXX/Library/Developer/Xcode/DerivedData/confidence-sdk-swift-XXX/SourcePackages/checkouts/swift-sdk/Sources/OpenFeature/OpenFeatureAPI.swift:187 #5 0x0000000112973690 in closure #1 in closure #1 in closure #1 in OpenFeatureAPI.setEvaluationContextAndWait(evaluationContext:) at /Users/XXX/Library/Developer/Xcode/DerivedData/confidence-sdk-swift-XXX/SourcePackages/checkouts/swift-sdk/Sources/OpenFeature/OpenFeatureAPI.swift:95 #6 0x0000000112975f40 in partial apply for closure #1 in closure #1 in closure #1 in OpenFeatureAPI.setEvaluationContextAndWait(evaluationContext:) () #7 0x00000001129764dc in thunk for @escaping @isolated(any) @callee_guaranteed @async () -> (@out A) () #8 0x0000000112976648 in partial apply for thunk for @escaping @isolated(any) @callee_guaranteed @async () -> (@out A) () ``` --------- Signed-off-by: Fabrizio Demaria <[email protected]>
1 parent c14b0cd commit dd23929

File tree

6 files changed

+1036
-70
lines changed

6 files changed

+1036
-70
lines changed
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import Foundation
2+
3+
/// Unified serial async task queue with operation-type-aware last-wins semantics.
4+
/// - Non-last-wins operations always execute in order
5+
/// - Consecutive last-wins operations: only the last one executes
6+
/// - Order is always preserved
7+
internal actor AsyncProviderOperationsQueue {
8+
private var currentTask: Task<Void, Never>?
9+
10+
private struct QueuedOperation {
11+
let operation: () async -> Void
12+
let continuation: CheckedContinuation<Void, Never>
13+
let lastWins: Bool
14+
}
15+
16+
private var queue: [QueuedOperation] = []
17+
18+
/// Runs the given operation serially.
19+
/// - If lastWins is false: operation always executes
20+
/// - If lastWins is true: may be skipped if superseded by a later last-wins operation
21+
func run(lastWins: Bool, operation: @Sendable @escaping () async -> Void) async {
22+
await withCheckedContinuation { continuation in
23+
queue.append(QueuedOperation(operation: operation, continuation: continuation, lastWins: lastWins))
24+
25+
if currentTask == nil {
26+
processNext()
27+
}
28+
}
29+
}
30+
31+
private func processNext() {
32+
guard !queue.isEmpty else {
33+
currentTask = nil
34+
return
35+
}
36+
37+
// Find the next batch to execute
38+
// A batch is either:
39+
// 1. A single non-last-wins operation, OR
40+
// 2. Consecutive last-wins operations (we execute only the last one)
41+
42+
let firstOp = queue[0]
43+
44+
if !firstOp.lastWins {
45+
// Non-last-wins operation: execute it immediately
46+
let op = queue.removeFirst()
47+
currentTask = Task { [weak self] in
48+
await op.operation()
49+
op.continuation.resume()
50+
await self?.processNext()
51+
}
52+
} else {
53+
// Last-wins operation: find all consecutive last-wins ops
54+
var lastWinsCount = 0
55+
for op in queue {
56+
if op.lastWins {
57+
lastWinsCount += 1
58+
} else {
59+
break
60+
}
61+
}
62+
63+
// Execute only the last one in the last-wins batch
64+
let toSkip = Array(queue.prefix(lastWinsCount - 1))
65+
let toExecute = queue[lastWinsCount - 1]
66+
queue.removeFirst(lastWinsCount)
67+
68+
currentTask = Task { [weak self] in
69+
await toExecute.operation()
70+
71+
// Resume all continuations (both skipped and executed)
72+
for op in toSkip {
73+
op.continuation.resume()
74+
}
75+
toExecute.continuation.resume()
76+
77+
await self?.processNext()
78+
}
79+
}
80+
}
81+
}

Sources/OpenFeature/OpenFeatureAPI.swift

Lines changed: 120 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@ import Foundation
55
/// Configuration here will be shared across all ``Client``s.
66
public class OpenFeatureAPI {
77
private let eventHandler = EventHandler()
8-
private let queue = DispatchQueue(label: "com.openfeature.providerDescriptor.queue")
8+
// Sync queue to change state atomically
9+
private let stateQueue = DispatchQueue(label: "com.openfeature.state.queue")
10+
// Queue for provider's initialize and onContextSet operations
11+
private let unifiedQueue = AsyncProviderOperationsQueue()
912

1013
private(set) var providerSubject = CurrentValueSubject<FeatureProvider?, Never>(nil)
1114
private(set) var evaluationContext: EvaluationContext?
@@ -15,18 +18,15 @@ public class OpenFeatureAPI {
1518
/// The ``OpenFeatureAPI`` singleton
1619
static public let shared = OpenFeatureAPI()
1720

18-
public init() {
19-
}
21+
public init() {}
2022

2123
/**
2224
Set provider and calls its `initialize` in a background thread.
2325
Readiness can be determined from `getState` or listening for `ready` event.
2426
*/
2527
public func setProvider(provider: FeatureProvider, initialContext: EvaluationContext?) {
26-
queue.async {
27-
Task {
28-
await self.setProviderInternal(provider: provider, initialContext: initialContext)
29-
}
28+
Task {
29+
await self.setProviderInternal(provider: provider, initialContext: initialContext)
3030
}
3131
}
3232

@@ -35,14 +35,7 @@ public class OpenFeatureAPI {
3535
This async function returns when the `initialize` from the provider is completed.
3636
*/
3737
public func setProviderAndWait(provider: FeatureProvider, initialContext: EvaluationContext?) async {
38-
await withCheckedContinuation { continuation in
39-
queue.async {
40-
Task {
41-
await self.setProviderInternal(provider: provider, initialContext: initialContext)
42-
continuation.resume()
43-
}
44-
}
45-
}
38+
await self.setProviderInternal(provider: provider, initialContext: initialContext)
4639
}
4740

4841
/**
@@ -62,13 +55,31 @@ public class OpenFeatureAPI {
6255
}
6356

6457
public func getProvider() -> FeatureProvider? {
65-
return self.providerSubject.value
58+
return stateQueue.sync {
59+
self.providerSubject.value
60+
}
6661
}
6762

6863
public func clearProvider() {
69-
queue.sync {
70-
self.providerSubject.send(nil)
71-
self.providerStatus = .notReady
64+
Task {
65+
await clearProviderInternal()
66+
}
67+
}
68+
69+
/**
70+
Clear provider.
71+
This async function returns when the clear operation is completed.
72+
*/
73+
public func clearProviderAndWait() async {
74+
await clearProviderInternal()
75+
}
76+
77+
private func clearProviderInternal() async {
78+
await unifiedQueue.run(lastWins: false) { [self] in
79+
stateQueue.sync {
80+
self.providerSubject.send(nil)
81+
self.providerStatus = .notReady
82+
}
7283
}
7384
}
7485

@@ -77,10 +88,8 @@ public class OpenFeatureAPI {
7788
Readiness can be determined from `getState` or listening for `contextChanged` event.
7889
*/
7990
public func setEvaluationContext(evaluationContext: EvaluationContext) {
80-
queue.async {
81-
Task {
82-
await self.updateContext(evaluationContext: evaluationContext)
83-
}
91+
Task {
92+
await self.updateContext(evaluationContext: evaluationContext)
8493
}
8594
}
8695

@@ -89,22 +98,19 @@ public class OpenFeatureAPI {
8998
This async function returns when the `onContextSet` from the provider is completed.
9099
*/
91100
public func setEvaluationContextAndWait(evaluationContext: EvaluationContext) async {
92-
await withCheckedContinuation { continuation in
93-
queue.async {
94-
Task {
95-
await self.updateContext(evaluationContext: evaluationContext)
96-
continuation.resume()
97-
}
98-
}
99-
}
101+
await updateContext(evaluationContext: evaluationContext)
100102
}
101103

102104
public func getEvaluationContext() -> EvaluationContext? {
103-
return self.evaluationContext
105+
return stateQueue.sync {
106+
self.evaluationContext
107+
}
104108
}
105109

106110
public func getProviderStatus() -> ProviderStatus {
107-
return self.providerStatus
111+
return stateQueue.sync {
112+
self.providerStatus
113+
}
108114
}
109115

110116
public func getProviderMetadata() -> ProviderMetadata? {
@@ -120,11 +126,21 @@ public class OpenFeatureAPI {
120126
}
121127

122128
public func addHooks(hooks: (any Hook)...) {
123-
self.hooks.append(contentsOf: hooks)
129+
stateQueue.sync {
130+
self.hooks.append(contentsOf: hooks)
131+
}
124132
}
125133

126134
public func clearHooks() {
127-
self.hooks.removeAll()
135+
stateQueue.sync {
136+
self.hooks.removeAll()
137+
}
138+
}
139+
140+
internal func getHooks() -> [any Hook] {
141+
return stateQueue.sync {
142+
self.hooks
143+
}
128144
}
129145

130146
public func observe() -> AnyPublisher<ProviderEvent?, Never> {
@@ -143,7 +159,7 @@ public class OpenFeatureAPI {
143159
}
144160

145161
internal func getState() -> OpenFeatureState {
146-
return queue.sync {
162+
return stateQueue.sync {
147163
OpenFeatureState(
148164
provider: providerSubject.value,
149165
evaluationContext: evaluationContext,
@@ -152,44 +168,81 @@ public class OpenFeatureAPI {
152168
}
153169

154170
private func setProviderInternal(provider: FeatureProvider, initialContext: EvaluationContext? = nil) async {
155-
self.providerStatus = .notReady
156-
self.providerSubject.send(provider)
157-
158-
if let initialContext = initialContext {
159-
self.evaluationContext = initialContext
160-
}
171+
await unifiedQueue.run(lastWins: false) { [self] in
172+
// Set initial state atomically
173+
stateQueue.sync {
174+
self.providerStatus = .notReady
175+
self.providerSubject.send(provider)
176+
if let initialContext = initialContext {
177+
self.evaluationContext = initialContext
178+
}
179+
}
161180

162-
do {
163-
try await provider.initialize(initialContext: initialContext)
164-
self.providerStatus = .ready
165-
self.eventHandler.send(.ready(nil))
166-
} catch {
167-
switch error {
168-
case OpenFeatureError.providerFatalError(let message):
169-
self.providerStatus = .fatal
170-
self.eventHandler.send(.error(ProviderEventDetails(message: message, errorCode: .providerFatal)))
171-
default:
172-
self.providerStatus = .error
173-
self.eventHandler.send(.error(ProviderEventDetails(message: error.localizedDescription)))
181+
// Initialize provider - this entire operation is atomic
182+
do {
183+
try await provider.initialize(initialContext: initialContext)
184+
stateQueue.sync {
185+
self.providerStatus = .ready
186+
}
187+
self.eventHandler.send(.ready(nil))
188+
} catch {
189+
stateQueue.sync {
190+
switch error {
191+
case OpenFeatureError.providerFatalError(_):
192+
self.providerStatus = .fatal
193+
default:
194+
self.providerStatus = .error
195+
}
196+
}
197+
switch error {
198+
case OpenFeatureError.providerFatalError(let message):
199+
self.eventHandler.send(.error(ProviderEventDetails(message: message, errorCode: .providerFatal)))
200+
default:
201+
self.eventHandler.send(.error(ProviderEventDetails(message: error.localizedDescription)))
202+
}
174203
}
175204
}
176205
}
177206

178207
private func updateContext(evaluationContext: EvaluationContext) async {
179-
do {
180-
let oldContext = self.evaluationContext
181-
self.evaluationContext = evaluationContext
182-
self.providerStatus = .reconciling
208+
await unifiedQueue.run(lastWins: true) { [self] in
209+
// Get old context, set new context, and update status atomically
210+
let (oldContext, provider) = stateQueue.sync { () -> (EvaluationContext?, FeatureProvider?) in
211+
let oldContext = self.evaluationContext
212+
self.evaluationContext = evaluationContext
213+
214+
// Only update status if provider is set
215+
if let provider = self.providerSubject.value {
216+
self.providerStatus = .reconciling
217+
return (oldContext, provider)
218+
}
219+
220+
return (oldContext, nil)
221+
}
222+
223+
// Early return if no provider is set - nothing to reconcile
224+
guard let provider = provider else {
225+
return
226+
}
227+
183228
eventHandler.send(.reconciling(nil))
184-
try await self.providerSubject.value?.onContextSet(
185-
oldContext: oldContext,
186-
newContext: evaluationContext
187-
)
188-
self.providerStatus = .ready
189-
eventHandler.send(.contextChanged(nil))
190-
} catch {
191-
self.providerStatus = .error
192-
eventHandler.send(.error(ProviderEventDetails(message: error.localizedDescription)))
229+
230+
// Call provider's onContextSet - this entire operation is atomic
231+
do {
232+
try await provider.onContextSet(
233+
oldContext: oldContext,
234+
newContext: evaluationContext
235+
)
236+
stateQueue.sync {
237+
self.providerStatus = .ready
238+
}
239+
eventHandler.send(.contextChanged(nil))
240+
} catch {
241+
stateQueue.sync {
242+
self.providerStatus = .error
243+
}
244+
eventHandler.send(.error(ProviderEventDetails(message: error.localizedDescription)))
245+
}
193246
}
194247
}
195248

Sources/OpenFeature/OpenFeatureClient.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ extension OpenFeatureClient {
117117
clientMetadata: self.metadata,
118118
providerMetadata: provider.metadata)
119119
hookLock.lock()
120-
let mergedHooks = provider.hooks + options.hooks + hooks + openFeatureApi.hooks
120+
let mergedHooks = provider.hooks + options.hooks + hooks + openFeatureApi.getHooks()
121121
hookLock.unlock()
122122
do {
123123
hookSupport.beforeHooks(flagValueType: T.flagValueType, hookCtx: hookCtx, hooks: mergedHooks, hints: hints)

Tests/OpenFeatureTests/FlagEvaluationTests.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ final class FlagEvaluationTests: XCTestCase {
2626

2727
OpenFeatureAPI.shared.addHooks(hooks: hook1)
2828

29-
XCTAssertEqual(OpenFeatureAPI.shared.hooks.count, 1)
29+
XCTAssertEqual(OpenFeatureAPI.shared.getHooks().count, 1)
3030

3131
OpenFeatureAPI.shared.addHooks(hooks: hook2)
32-
XCTAssertEqual(OpenFeatureAPI.shared.hooks.count, 2)
32+
XCTAssertEqual(OpenFeatureAPI.shared.getHooks().count, 2)
3333
}
3434

3535
func testNamedClient() {

0 commit comments

Comments
 (0)