Skip to content

Commit 6550bf5

Browse files
authored
fix: use a dispatchqueue to synchronize setting and getting userConfig (#254)
1 parent 0a8f577 commit 6550bf5

File tree

5 files changed

+239
-21
lines changed

5 files changed

+239
-21
lines changed

DevCycle.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
52A48707278C9BE200DABA34 /* Log.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52A48706278C9BE200DABA34 /* Log.swift */; };
4848
52A96990279F3AAA00D3A602 /* PlatformDetails.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52A9698F279F3AAA00D3A602 /* PlatformDetails.swift */; };
4949
52A96A0F27A0A7CF00D3A602 /* EventQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52A96A0E27A0A7CF00D3A602 /* EventQueue.swift */; };
50+
52D342B72E60A47700B86328 /* DVCConfigTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52D342B62E60A47300B86328 /* DVCConfigTests.swift */; };
5051
52E693EA27567E2600B52375 /* DevCycleService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52E693E927567E2600B52375 /* DevCycleService.swift */; };
5152
52E693ED2756816A00B52375 /* DVCConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52E693EC2756816A00B52375 /* DVCConfig.swift */; };
5253
52E693F62758032500B52375 /* DevCycleServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52E693F52758032500B52375 /* DevCycleServiceTests.swift */; };
@@ -125,6 +126,7 @@
125126
52A48706278C9BE200DABA34 /* Log.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Log.swift; sourceTree = "<group>"; };
126127
52A9698F279F3AAA00D3A602 /* PlatformDetails.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PlatformDetails.swift; sourceTree = "<group>"; };
127128
52A96A0E27A0A7CF00D3A602 /* EventQueue.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventQueue.swift; sourceTree = "<group>"; };
129+
52D342B62E60A47300B86328 /* DVCConfigTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DVCConfigTests.swift; sourceTree = "<group>"; };
128130
52E693E927567E2600B52375 /* DevCycleService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DevCycleService.swift; sourceTree = "<group>"; };
129131
52E693EC2756816A00B52375 /* DVCConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DVCConfig.swift; sourceTree = "<group>"; };
130132
52E693F52758032500B52375 /* DevCycleServiceTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DevCycleServiceTests.swift; sourceTree = "<group>"; };
@@ -203,6 +205,7 @@
203205
5264A78C2768E87C00FEDB43 /* Models */ = {
204206
isa = PBXGroup;
205207
children = (
208+
52D342B62E60A47300B86328 /* DVCConfigTests.swift */,
206209
52E8C1F02DF87C560044783D /* DevCycleEventTests.swift */,
207210
5268DB68275020D900D17A40 /* DevCycleUserTest.swift */,
208211
524F4E61276D20A500CB9069 /* DVCVariableTests.swift */,
@@ -480,6 +483,7 @@
480483
52A1139F27AB235C000B8285 /* EventQueueTests.swift in Sources */,
481484
52133B2428DE0FEB0007691D /* GetTestConfig.swift in Sources */,
482485
5256A99A2798716400E749FF /* ObjCDevCycleClientTests.m in Sources */,
486+
52D342B72E60A47700B86328 /* DVCConfigTests.swift in Sources */,
483487
5256A99B2798716400E749FF /* ObjcDevCycleUserTests.m in Sources */,
484488
524F4E62276D20A600CB9069 /* DVCVariableTests.swift in Sources */,
485489
);

DevCycle/DevCycleClient.swift

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public class DevCycleClient {
158158
Log.error("Error getting config: \(error)", tags: ["setup"])
159159

160160
// If network failed but we have a cached config, don't return error
161-
if self.config?.userConfig != nil {
161+
if self.config?.getUserConfig() != nil {
162162
Log.info("Using cached config due to network error")
163163
finalError = nil
164164
}
@@ -245,7 +245,7 @@ public class DevCycleClient {
245245

246246
private func updateUserConfig(_ config: UserConfig) {
247247
let oldSSEURL = self.config?.userConfig?.sse?.url
248-
self.config?.userConfig = config
248+
self.config?.setUserConfig(config: config)
249249

250250
let newSSEURL = config.sse?.url
251251
if newSSEURL != nil && oldSSEURL != newSSEURL {
@@ -260,7 +260,7 @@ public class DevCycleClient {
260260
return
261261
}
262262

263-
guard let sseURL = self.config?.userConfig?.sse?.url else {
263+
guard let sseURL = self.config?.getUserConfig()?.sse?.url else {
264264
Log.error("No SSE URL in config")
265265
return
266266
}
@@ -275,7 +275,7 @@ public class DevCycleClient {
275275
self.sseConnection = nil
276276
}
277277

278-
if let inactivityDelay = self.config?.userConfig?.sse?.inactivityDelay {
278+
if let inactivityDelay = self.config?.getUserConfig()?.sse?.inactivityDelay {
279279
self.inactivityDelayMS = Double(inactivityDelay)
280280
}
281281
self.sseConnection = SSEConnection(
@@ -297,7 +297,7 @@ public class DevCycleClient {
297297
}
298298
let sseMessage = try SSEMessage(from: messageDictionary)
299299
if sseMessage.data.type == nil || sseMessage.data.type == "refetchConfig" {
300-
if self?.config?.userConfig?.etag == nil
300+
if self?.config?.getUserConfig()?.etag == nil
301301
|| sseMessage.data.etag != self?.config?.userConfig?.etag
302302
{
303303
self?.refetchConfig(
@@ -395,7 +395,7 @@ public class DevCycleClient {
395395
{
396396
variable = variableFromDictionary
397397
} else {
398-
if let config = self.config?.userConfig,
398+
if let config = self.config?.getUserConfig(),
399399
let variableFromApi = config.variables[key]
400400
{
401401
variable = DVCVariable(from: variableFromApi, defaultValue: defaultValue)
@@ -462,12 +462,12 @@ public class DevCycleClient {
462462

463463
// Try to use cached config for the new user
464464
// If we have a cached config, proceed without error
465-
if self.useCachedConfigForUser(user: updateUser), self.config?.userConfig != nil {
465+
if self.useCachedConfigForUser(user: updateUser), self.config?.getUserConfig() != nil {
466466
Log.info(
467467
"Using cached config for identifyUser due to network error: \(error)",
468468
tags: ["identify"])
469469
self.user = user
470-
callback?(nil, self.config?.userConfig?.variables)
470+
callback?(nil, self.config?.getUserConfig()?.variables)
471471
return
472472
} else {
473473
// No cached config available, return error and don't change client state
@@ -482,7 +482,7 @@ public class DevCycleClient {
482482
Log.debug("IdentifyUser config: \(config)", tags: ["identify"])
483483
self.updateUserConfig(config)
484484
self.user = user
485-
callback?(nil, self.config?.userConfig?.variables)
485+
callback?(nil, self.config?.getUserConfig()?.variables)
486486
} else {
487487
Log.error("No config returned for identifyUser", tags: ["identify"])
488488
callback?(ClientError.ConfigFetchFailed, nil)
@@ -564,11 +564,11 @@ public class DevCycleClient {
564564
}
565565

566566
public func allFeatures() -> [String: Feature] {
567-
return self.config?.userConfig?.features ?? [:]
567+
return self.config?.getUserConfig()?.features ?? [:]
568568
}
569569

570570
public func allVariables() -> [String: Variable] {
571-
return self.config?.userConfig?.variables ?? [:]
571+
return self.config?.getUserConfig()?.variables ?? [:]
572572
}
573573

574574
public func track(_ event: DevCycleEvent) {
@@ -738,7 +738,7 @@ public class DevCycleClient {
738738
if options?.disableConfigCache != true,
739739
let cachedConfig = cacheService.getConfig(user: user)
740740
{
741-
self.config?.userConfig = cachedConfig
741+
self.config?.setUserConfig(config: cachedConfig)
742742
Log.debug("Loaded config from cache for user_id \(String(describing: user.userId))")
743743
return true
744744
}

DevCycle/Models/DVCConfig.swift

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,31 @@ import Foundation
99
public class DVCConfig {
1010
var sdkKey: String
1111
var user: DevCycleUser
12-
var userConfig: UserConfig? {
13-
didSet {
14-
if let userConfig = self.userConfig {
12+
private let userConfigQueue = DispatchQueue(label: "com.devcycle.userConfigQueue")
13+
var userConfig: UserConfig?
14+
15+
init(sdkKey: String, user: DevCycleUser) {
16+
self.sdkKey = sdkKey
17+
self.user = user
18+
}
19+
20+
func getUserConfig() -> UserConfig? {
21+
return userConfigQueue.sync {
22+
return self.userConfig
23+
}
24+
}
25+
26+
func setUserConfig(config: UserConfig?) {
27+
var configToNotify: UserConfig?
28+
29+
userConfigQueue.sync {
30+
self.userConfig = config
31+
configToNotify = config
32+
}
33+
34+
// Post notification outside of the lock to avoid potential deadlocks
35+
if let userConfig = configToNotify {
36+
DispatchQueue.main.async {
1537
NotificationCenter.default.post(
1638
name: Notification.Name(NotificationNames.NewUserConfig),
1739
object: self,
@@ -20,9 +42,4 @@ public class DVCConfig {
2042
}
2143
}
2244
}
23-
24-
init(sdkKey: String, user: DevCycleUser) {
25-
self.sdkKey = sdkKey
26-
self.user = user
27-
}
2845
}
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
//
2+
// DVCConfigTests.swift
3+
// DevCycleTests
4+
//
5+
//
6+
7+
import XCTest
8+
@testable import DevCycle
9+
10+
final class DVCConfigTests: XCTestCase {
11+
12+
var mockUser: DevCycleUser!
13+
14+
override func setUp() {
15+
super.setUp()
16+
mockUser = try! DevCycleUser.builder()
17+
.userId("test-user")
18+
.isAnonymous(false)
19+
.build()
20+
}
21+
22+
override func tearDown() {
23+
mockUser = nil
24+
super.tearDown()
25+
}
26+
27+
func getMockUserConfig(config: String = "test_config_eval_reason") throws -> UserConfig {
28+
let data = getConfigData(name: config)
29+
let dictionary = try JSONSerialization.jsonObject(with: data, options: .fragmentsAllowed) as! [String:Any]
30+
return try UserConfig(from: dictionary)
31+
}
32+
33+
func testConcurrentConfigAccess() throws {
34+
let testConfig = DVCConfig(sdkKey: "test-sdk-key", user: mockUser)
35+
36+
let expectation = XCTestExpectation(description: "Concurrent config access completed")
37+
expectation.expectedFulfillmentCount = 100 // 50 reads + 50 writes
38+
39+
let concurrentQueue = DispatchQueue(label: "test.concurrent", attributes: .concurrent)
40+
41+
// Test concurrent reads
42+
for _ in 0..<50 {
43+
concurrentQueue.async {
44+
let _ = testConfig.getUserConfig()
45+
expectation.fulfill()
46+
}
47+
}
48+
49+
let evalConfig = try self.getMockUserConfig(config: "test_config_eval_reason")
50+
51+
// Test concurrent writes
52+
for _ in 0..<50 {
53+
concurrentQueue.async {
54+
testConfig.setUserConfig(config: evalConfig)
55+
expectation.fulfill()
56+
}
57+
}
58+
59+
wait(for: [expectation], timeout: 10.0)
60+
61+
// Verify the final state is consistent
62+
let finalConfig = testConfig.getUserConfig()
63+
XCTAssertNotNil(finalConfig)
64+
}
65+
66+
func testConcurrentConfigAccessWithVariables() throws {
67+
let testConfig = DVCConfig(sdkKey: "test-sdk-key", user: mockUser)
68+
let mockUserConfig = try getMockUserConfig()
69+
testConfig.setUserConfig(config: mockUserConfig)
70+
71+
let expectation = XCTestExpectation(description: "Concurrent variable access completed")
72+
expectation.expectedFulfillmentCount = 200 // 100 reads + 100 writes
73+
74+
let concurrentQueue = DispatchQueue(label: "test.concurrent.variables", attributes: .concurrent)
75+
76+
// Test concurrent reads of variables
77+
for _ in 0..<100 {
78+
concurrentQueue.async {
79+
let variables = testConfig.getUserConfig()?.variables
80+
XCTAssertNotNil(variables)
81+
expectation.fulfill()
82+
}
83+
}
84+
let evalConfig = try self.getMockUserConfig(config: "test_config_eval_reason")
85+
86+
// Test concurrent writes with new configs
87+
for _ in 0..<100 {
88+
concurrentQueue.async {
89+
testConfig.setUserConfig(config: evalConfig)
90+
expectation.fulfill()
91+
}
92+
}
93+
94+
wait(for: [expectation], timeout: 10.0)
95+
96+
// Verify final state
97+
let finalConfig = testConfig.getUserConfig()
98+
XCTAssertNotNil(finalConfig)
99+
XCTAssertNotNil(finalConfig?.variables)
100+
}
101+
102+
func testRapidConfigUpdates() throws {
103+
let testConfig = DVCConfig(sdkKey: "test-sdk-key", user: mockUser)
104+
105+
let expectation = XCTestExpectation(description: "Rapid config updates completed")
106+
expectation.expectedFulfillmentCount = 1000
107+
108+
let concurrentQueue = DispatchQueue(label: "test.rapid", attributes: .concurrent)
109+
let evalConfig = try self.getMockUserConfig(config: "test_config_eval_reason")
110+
111+
// Rapidly update config from multiple threads
112+
for _ in 0..<1000 {
113+
concurrentQueue.async {
114+
testConfig.setUserConfig(config: evalConfig)
115+
expectation.fulfill()
116+
}
117+
}
118+
119+
wait(for: [expectation], timeout: 10.0)
120+
121+
// Verify no crashes occurred
122+
let finalConfig = testConfig.getUserConfig()
123+
XCTAssertNotNil(finalConfig)
124+
}
125+
126+
func testConfigAccessDuringUpdates() throws {
127+
128+
let testConfig = DVCConfig(sdkKey: "test-sdk-key", user: mockUser)
129+
testConfig.setUserConfig(config: try self.getMockUserConfig())
130+
131+
let expectation = XCTestExpectation(description: "Config access during updates completed")
132+
expectation.expectedFulfillmentCount = 500 // 250 updates + 250 reads
133+
134+
let updateQueue = DispatchQueue(label: "test.updates", attributes: .concurrent)
135+
let readQueue = DispatchQueue(label: "test.reads", attributes: .concurrent)
136+
137+
let evalConfig = try self.getMockUserConfig(config: "test_config_eval_reason")
138+
139+
// Continuously update config
140+
for _ in 0..<250 {
141+
updateQueue.async {
142+
testConfig.setUserConfig(config: evalConfig)
143+
expectation.fulfill()
144+
}
145+
}
146+
147+
// Continuously read config while updates are happening
148+
for _ in 0..<250 {
149+
readQueue.async {
150+
let config = testConfig.getUserConfig()
151+
_ = config?.variables
152+
_ = config?.features
153+
XCTAssertNotNil(config)
154+
expectation.fulfill()
155+
}
156+
}
157+
158+
wait(for: [expectation], timeout: 10.0)
159+
160+
// Verify final state
161+
let finalConfig = testConfig.getUserConfig()
162+
XCTAssertNotNil(finalConfig)
163+
}
164+
165+
func testSetConfigNil() throws {
166+
let testNilConfig = DVCConfig(sdkKey: "test-sdk-key", user: mockUser)
167+
testNilConfig.setUserConfig(config: try self.getMockUserConfig())
168+
169+
let expectation = XCTestExpectation(description: "Config access during setting config to nil completed")
170+
expectation.expectedFulfillmentCount = 500 // 250 updates + 250 reads
171+
172+
let updateQueue = DispatchQueue(label: "test.updates", attributes: .concurrent)
173+
let readQueue = DispatchQueue(label: "test.reads", attributes: .concurrent)
174+
175+
let evalConfig = try self.getMockUserConfig(config: "test_config_eval_reason")
176+
177+
// Continuously update config
178+
for i in 0..<250 {
179+
updateQueue.async {
180+
testNilConfig.setUserConfig(config: i % 2 == 0 ? evalConfig : nil)
181+
expectation.fulfill()
182+
}
183+
}
184+
185+
// Continuously read config while updates are happening
186+
for _ in 0..<250 {
187+
readQueue.async {
188+
let config = testNilConfig.getUserConfig()
189+
_ = config?.variables
190+
_ = config?.features
191+
expectation.fulfill()
192+
}
193+
}
194+
195+
wait(for: [expectation], timeout: 10.0)
196+
}
197+
}

DevCycleTests/Models/DevCycleClientTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class DevCycleClientTest: XCTestCase {
165165
client.close(callback: nil)
166166
}
167167

168-
func testFlushEventsWithEvalReasons() throws{
168+
func testFlushEventsWithEvalReasons() throws {
169169
let data = getConfigData(name: "test_config_eval_reason")
170170
let dictionary = try JSONSerialization.jsonObject(with: data, options: .fragmentsAllowed) as! [String:Any]
171171
let evalReasonConfig = try UserConfig(from: dictionary)

0 commit comments

Comments
 (0)