Skip to content

Commit a174d3f

Browse files
jsalaberjsalaber
authored
fix: fix variable getting called from multiple threads causing a crash (#152)
* fix: fix variable getting called from multiple threads causing a crash * chore: added test that variable called in main queue sync block doesnt crash
1 parent bbc0737 commit a174d3f

File tree

3 files changed

+83
-25
lines changed

3 files changed

+83
-25
lines changed

‎DevCycle.xcodeproj/project.pbxproj‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
52133B2228DE00BC0007691D /* ProcessConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52133B2128DE00BC0007691D /* ProcessConfig.swift */; };
1414
52133B2428DE0FEB0007691D /* GetTestConfig.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52133B2328DE0FEB0007691D /* GetTestConfig.swift */; };
1515
5226DF06290C588900630745 /* NotificationNames.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5226DF05290C588900630745 /* NotificationNames.swift */; };
16+
523A31D029CB411A008F3347 /* ThreadingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 523A31CF29CB411A008F3347 /* ThreadingTests.swift */; };
1617
52404CEA27F3A9FB00290A31 /* isEqual.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52404CE927F3A9FB00290A31 /* isEqual.swift */; };
1718
52404CED27F4A92100290A31 /* IsEqualTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52404CEC27F4A92100290A31 /* IsEqualTests.swift */; };
1819
524D58242770F78B00D7CC56 /* ObjCDVCVariable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 524D58232770F78B00D7CC56 /* ObjCDVCVariable.swift */; };
@@ -85,6 +86,7 @@
8586
52133B2128DE00BC0007691D /* ProcessConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProcessConfig.swift; sourceTree = "<group>"; };
8687
52133B2328DE0FEB0007691D /* GetTestConfig.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GetTestConfig.swift; sourceTree = "<group>"; };
8788
5226DF05290C588900630745 /* NotificationNames.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationNames.swift; sourceTree = "<group>"; };
89+
523A31CF29CB411A008F3347 /* ThreadingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = ThreadingTests.swift; path = DevCycleTests/Models/ThreadingTests.swift; sourceTree = SOURCE_ROOT; };
8890
52404CE927F3A9FB00290A31 /* isEqual.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = isEqual.swift; sourceTree = "<group>"; };
8991
52404CEC27F4A92100290A31 /* IsEqualTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IsEqualTests.swift; sourceTree = "<group>"; };
9092
524D58232770F78B00D7CC56 /* ObjCDVCVariable.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ObjCDVCVariable.swift; sourceTree = "<group>"; };
@@ -150,6 +152,7 @@
150152
52133B1E28DDFAF90007691D /* Networking */ = {
151153
isa = PBXGroup;
152154
children = (
155+
523A31CF29CB411A008F3347 /* ThreadingTests.swift */,
153156
52E693F52758032500B52375 /* DevCycleServiceTests.swift */,
154157
);
155158
path = Networking;
@@ -456,6 +459,7 @@
456459
52E693F62758032500B52375 /* DevCycleServiceTests.swift in Sources */,
457460
52404CED27F4A92100290A31 /* IsEqualTests.swift in Sources */,
458461
5268DB6B275020F800D17A40 /* DVCClientTest.swift in Sources */,
462+
523A31D029CB411A008F3347 /* ThreadingTests.swift in Sources */,
459463
529F0C90277374150075AAB4 /* ObjcDVCVariableTests.m in Sources */,
460464
5268DB69275020D900D17A40 /* DVCUserTest.swift in Sources */,
461465
52A1139F27AB235C000B8285 /* EventQueueTests.swift in Sources */,

‎DevCycle/DVCClient.swift‎

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ public class DVCClient {
5353
private var variableInstanceDictonary = [String: NSMapTable<AnyObject, AnyObject>]()
5454
private var isConfigCached: Bool = false
5555

56+
private var variableQueue = DispatchQueue(label: "com.devcycle.VariableQueue")
57+
5658
/**
5759
Method to initialize the Client object after building
5860
*/
@@ -266,34 +268,35 @@ public class DVCClient {
266268
)
267269
}
268270

269-
var variable: DVCVariable<T>
270-
if (self.variableInstanceDictonary[key] == nil) {
271-
self.variableInstanceDictonary[key] = NSMapTable<AnyObject, AnyObject>(valueOptions: .weakMemory)
272-
}
273-
274-
if let variableFromDictonary = self.variableInstanceDictonary[key]?.object(forKey: defaultValue as AnyObject) as? DVCVariable<T> {
275-
variable = variableFromDictonary
276-
} else {
277-
if let config = self.config?.userConfig,
278-
let variableFromApi = config.variables[key] {
279-
variable = DVCVariable(from: variableFromApi, defaultValue: defaultValue)
271+
return variableQueue.sync {
272+
var variable: DVCVariable<T>
273+
if (self.variableInstanceDictonary[key] == nil) {
274+
self.variableInstanceDictonary[key] = NSMapTable<AnyObject, AnyObject>(valueOptions: .weakMemory)
275+
}
276+
if let variableFromDictionary = self.variableInstanceDictonary[key]?.object(forKey: defaultValue as AnyObject) as? DVCVariable<T> {
277+
variable = variableFromDictionary
280278
} else {
281-
variable = DVCVariable(
282-
key: key,
283-
type: String(describing: T.self),
284-
value: nil,
285-
defaultValue: defaultValue,
286-
evalReason: nil
287-
)
279+
if let config = self.config?.userConfig,
280+
let variableFromApi = config.variables[key] {
281+
variable = DVCVariable(from: variableFromApi, defaultValue: defaultValue)
282+
} else {
283+
variable = DVCVariable(
284+
key: key,
285+
type: String(describing: T.self),
286+
value: nil,
287+
defaultValue: defaultValue,
288+
evalReason: nil
289+
)
290+
}
291+
self.variableInstanceDictonary[key]?.setObject(variable, forKey: defaultValue as AnyObject)
288292
}
289-
self.variableInstanceDictonary[key]?.setObject(variable, forKey: defaultValue as AnyObject)
290-
}
291-
292-
if (!self.closed) {
293-
self.eventQueue.updateAggregateEvents(variableKey: variable.key, variableIsDefaulted: variable.isDefaulted)
293+
294+
if (!self.closed) {
295+
self.eventQueue.updateAggregateEvents(variableKey: variable.key, variableIsDefaulted: variable.isDefaulted)
296+
}
297+
298+
return variable
294299
}
295-
296-
return variable
297300
}
298301

299302
public func identifyUser(user: DVCUser, callback: IdentifyCompletedHandler? = nil) throws {

‎DevCycleTests/Models/ThreadingTests.swift‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
//
2+
// ThreadingTests.swift
3+
// DevCycleTests
4+
//
5+
// Copyright © 2023 Taplytics. All rights reserved.
6+
//
7+
8+
import XCTest
9+
@testable import DevCycle
10+
11+
final class ThreadingTests: XCTestCase {
12+
private var service: MockService!
13+
private var user: DVCUser!
14+
private var builder: DVCClient.ClientBuilder!
15+
private var userConfig: UserConfig!
16+
17+
override func setUpWithError() throws {
18+
self.service = MockService()
19+
self.user = try! DVCUser.builder()
20+
.userId("my_user")
21+
.build()
22+
self.builder = DVCClient.builder().service(service)
23+
24+
let data = getConfigData(name: "test_config")
25+
let dictionary = try! JSONSerialization.jsonObject(with: data, options: .fragmentsAllowed) as! [String:Any]
26+
self.userConfig = try! UserConfig(from: dictionary)
27+
}
28+
29+
func testVariableWorksInASyncBlockOnMainThread() throws {
30+
let client = try! self.builder.user(self.user).sdkKey("my_sdk_key").build(onInitialized: nil)
31+
let expectation = expectation(description: "Expect calling variable in a sync block on the main thread doesn't crash")
32+
DispatchQueue.global(qos: .userInitiated).async {
33+
DispatchQueue.main.sync {
34+
client.variable(key: "test-key", defaultValue: false)
35+
expectation.fulfill()
36+
}
37+
}
38+
wait(for: [expectation], timeout: 1.0)
39+
}
40+
41+
}
42+
43+
private class MockService: DevCycleServiceProtocol {
44+
func getConfig(user: DVCUser, enableEdgeDB: Bool, extraParams: RequestParams?, completion: @escaping ConfigCompletionHandler) {}
45+
46+
func publishEvents(events: [DVCEvent], user: DVCUser, completion: @escaping PublishEventsCompletionHandler) {}
47+
48+
func saveEntity(user: DVCUser, completion: @escaping SaveEntityCompletionHandler) {}
49+
50+
func makeRequest(request: URLRequest, completion: @escaping DevCycle.CompletionHandler) {}
51+
}

0 commit comments

Comments
 (0)