Skip to content

Commit 350926d

Browse files
kwasniewkwasniew
authored
feat: Thread safe isEnabled and getVariant (#126)
1 parent 0643098 commit 350926d

File tree

4 files changed

+169
-44
lines changed

4 files changed

+169
-44
lines changed

‎.github/workflows/build.yml

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,10 @@ jobs:
1616
steps:
1717
- uses: actions/checkout@v2
1818
- name: Run tests
19-
run: swift test --sanitize=thread
19+
run: swift test --sanitize=thread
20+
thread_safety_test:
21+
runs-on: macos-latest
22+
steps:
23+
- uses: actions/checkout@v2
24+
- name: Run thread safety tests
25+
run: UNLEASH_THREAD_SAFETY_TEST=1 swift test --filter UnleashThreadSafetyTest

‎Sources/UnleashProxyClientSwift/Client/UnleashProxyClientSwift.swift

Lines changed: 60 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ public class UnleashClientBase {
88
var poller: Poller
99
var metrics: Metrics
1010
var connectionId: UUID
11+
private let queue = DispatchQueue(label: "com.unleash.clientbase", attributes: .concurrent)
1112

1213
public init(
1314
unleashUrl: String,
@@ -61,7 +62,7 @@ public class UnleashClientBase {
6162
}
6263
self.metrics = Metrics(appName: appName, metricsInterval: Double(metricsInterval), clock: { return Date() }, disableMetrics: disableMetrics, poster: urlSessionPoster, url: url, clientKey: clientKey, customHeaders: customHeaders, connectionId: connectionId)
6364
}
64-
65+
6566
self.context = Context(appName: appName, environment: environment, sessionId: String(Int.random(in: 0..<1_000_000_000)))
6667
if let providedContext = context {
6768
self.context = self.calculateContext(context: providedContext)
@@ -74,13 +75,13 @@ public class UnleashClientBase {
7475
completionHandler: ((PollerError?) -> Void)? = nil
7576
) -> Void {
7677
Printer.showPrintStatements = printToConsole
77-
self.stopPolling()
78-
poller.start(
79-
bootstrapping: bootstrap.toggles,
80-
context: context,
81-
completionHandler: completionHandler
82-
)
83-
metrics.start()
78+
self.stopPolling()
79+
poller.start(
80+
bootstrapping: bootstrap.toggles,
81+
context: context,
82+
completionHandler: completionHandler
83+
)
84+
metrics.start()
8485
}
8586

8687
private func stopPolling() -> Void {
@@ -94,40 +95,50 @@ public class UnleashClientBase {
9495
}
9596

9697
public func isEnabled(name: String) -> Bool {
97-
let toggle = poller.getFeature(name: name)
98-
let enabled = toggle?.enabled ?? false
99-
100-
metrics.count(name: name, enabled: enabled)
101-
102-
if let toggle = toggle, toggle.impressionData {
103-
SwiftEventBus.post("impression", sender: ImpressionEvent(
104-
toggleName: name,
105-
enabled: enabled,
106-
context: context
107-
))
98+
return queue.sync {
99+
let toggle = poller.getFeature(name: name)
100+
let enabled = toggle?.enabled ?? false
101+
102+
metrics.count(name: name, enabled: enabled)
103+
104+
if let toggle = toggle, toggle.impressionData {
105+
// Dispatch impression event to background queue
106+
DispatchQueue.global().async {
107+
SwiftEventBus.post("impression", sender: ImpressionEvent(
108+
toggleName: name,
109+
enabled: enabled,
110+
context: self.context
111+
))
112+
}
113+
}
114+
115+
return enabled
108116
}
109-
110-
return enabled
111117
}
112118

113119
public func getVariant(name: String) -> Variant {
114-
let toggle = poller.getFeature(name: name)
115-
let variant = toggle?.variant ?? .defaultDisabled
116-
let enabled = toggle?.enabled ?? false
120+
return queue.sync {
121+
let toggle = poller.getFeature(name: name)
122+
let variant = toggle?.variant ?? .defaultDisabled
123+
let enabled = toggle?.enabled ?? false
117124

118-
metrics.count(name: name, enabled: enabled)
119-
metrics.countVariant(name: name, variant: variant.name)
120-
121-
if let toggle = toggle, toggle.impressionData {
122-
SwiftEventBus.post("impression", sender: ImpressionEvent(
123-
toggleName: name,
124-
enabled: enabled,
125-
variant: variant,
126-
context: context
127-
))
125+
metrics.count(name: name, enabled: enabled)
126+
metrics.countVariant(name: name, variant: variant.name)
127+
128+
if let toggle = toggle, toggle.impressionData {
129+
// Dispatch impression event to background queue
130+
DispatchQueue.global().async {
131+
SwiftEventBus.post("impression", sender: ImpressionEvent(
132+
toggleName: name,
133+
enabled: enabled,
134+
variant: variant,
135+
context: self.context
136+
))
137+
}
138+
}
139+
140+
return variant
128141
}
129-
130-
return variant
131142
}
132143

133144
public func subscribe(name: String, callback: @escaping () -> Void) {
@@ -143,7 +154,7 @@ public class UnleashClientBase {
143154
}
144155
}
145156
}
146-
157+
147158
public func subscribe(_ event: UnleashEvent, callback: @escaping () -> Void) {
148159
subscribe(name: event.rawValue, callback: callback)
149160
}
@@ -156,7 +167,7 @@ public class UnleashClientBase {
156167
let handler: (Notification?) -> Void = { notification in
157168
callback(notification?.object)
158169
}
159-
170+
160171
if Thread.isMainThread {
161172
print("Subscribing to \(name) on main thread with object")
162173
SwiftEventBus.onMainThread(self, name: name, handler: handler)
@@ -169,14 +180,21 @@ public class UnleashClientBase {
169180
public func unsubscribe(name: String) {
170181
SwiftEventBus.unregister(self, name: name)
171182
}
172-
183+
173184
public func unsubscribe(_ event: UnleashEvent) {
174185
unsubscribe(name: event.rawValue)
175186
}
176-
187+
177188
public func updateContext(context: [String: String], properties: [String:String]? = nil, completionHandler: ((PollerError?) -> Void)? = nil) {
178-
self.context = self.calculateContext(context: context, properties: properties)
179-
self.start(Printer.showPrintStatements, completionHandler: completionHandler)
189+
if Thread.isMainThread {
190+
self.context = self.calculateContext(context: context, properties: properties)
191+
self.start(Printer.showPrintStatements, completionHandler: completionHandler)
192+
} else {
193+
DispatchQueue.main.async {
194+
self.context = self.calculateContext(context: context, properties: properties)
195+
self.start(Printer.showPrintStatements, completionHandler: completionHandler)
196+
}
197+
}
180198
}
181199

182200
func calculateContext(context: [String: String], properties: [String:String]? = nil) -> Context {

‎Sources/UnleashProxyClientSwift/Version/Version.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
public struct LibraryInfo {
2-
public static let version = "2.2.0" // Update this string with each new release
2+
public static let version = "2.3.0" // Update this string with each new release
33
}

‎Tests/UnleashProxyClientSwiftTests/UnleashThreadSafetyTest.swift

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import XCTest
2+
@testable import UnleashProxyClientSwift
3+
4+
class UnleashThreadSafetyTest: XCTestCase {
5+
6+
private var shouldRunIntensiveTest: Bool {
7+
return ProcessInfo.processInfo.environment["UNLEASH_THREAD_SAFETY_TEST"] == "1"
8+
}
9+
10+
func testThreadSafety() {
11+
// Skip the test unless the environment variable is set
12+
guard shouldRunIntensiveTest else {
13+
print("Skipping UnleashThreadSafetyTest - set UNLEASH_THREAD_SAFETY_TEST=1 to run")
14+
return
15+
}
16+
17+
// This test demonstrates the thread safety issues in UnleashClient
18+
// by simulating concurrent access from multiple threads to the same client instance
19+
20+
// Run the test 10 times in a row to increase chances of detecting race conditions
21+
for run in 1...10 {
22+
// Create a shared client instance
23+
let unleashClient = UnleashClientBase(
24+
unleashUrl: "https://sandbox.getunleash.io/enterprise/api/frontend",
25+
clientKey: "SDKIntegration:development.f0474f4a37e60794ee8fb00a4c112de58befde962af6d5055b383ea3",
26+
appName: "testIntegration"
27+
)
28+
29+
// Start the client on the main thread
30+
unleashClient.start()
31+
32+
// Create a dispatch group to wait for all operations to complete
33+
let group = DispatchGroup()
34+
35+
// Simulate multiple threads checking feature flags simultaneously
36+
for _ in 1...5 {
37+
// Background thread 1: Repeatedly check isEnabled
38+
group.enter()
39+
DispatchQueue.global().async {
40+
for _ in 1...100 {
41+
// This can crash due to race conditions in isEnabled
42+
_ = unleashClient.isEnabled(name: "enabled-feature")
43+
44+
// Small sleep to increase chance of thread interleaving
45+
Thread.sleep(forTimeInterval: 0.01)
46+
}
47+
group.leave()
48+
}
49+
50+
// Background thread 2: Repeatedly check variants
51+
group.enter()
52+
DispatchQueue.global().async {
53+
for _ in 1...100 {
54+
// This can crash due to race conditions in getVariant
55+
_ = unleashClient.getVariant(name: "enabled-feature")
56+
57+
// Small sleep to increase chance of thread interleaving
58+
Thread.sleep(forTimeInterval: 0.01)
59+
}
60+
group.leave()
61+
}
62+
63+
// Background thread 3: Update context occasionally
64+
group.enter()
65+
DispatchQueue.global().async {
66+
for j in 1...10 {
67+
// This can crash due to race conditions when updating context
68+
unleashClient.updateContext(context: [
69+
"userId": "user-\(j)",
70+
"sessionId": "session-\(j)"
71+
])
72+
73+
// Sleep longer between context updates
74+
Thread.sleep(forTimeInterval: 0.1)
75+
}
76+
group.leave()
77+
}
78+
}
79+
80+
// Add a specific test for dataRaceTest flag, similar to the integration test
81+
var enabledCount = 0
82+
for _ in 1...15000 {
83+
let result = unleashClient.isEnabled(name: "dataRaceTest")
84+
if result {
85+
enabledCount += 1
86+
}
87+
}
88+
89+
// Wait for all operations to complete
90+
group.wait()
91+
92+
// Clean up
93+
unleashClient.stop()
94+
95+
// Add a small delay between runs to ensure resources are properly released
96+
if run < 10 {
97+
Thread.sleep(forTimeInterval: 1.0)
98+
}
99+
}
100+
}
101+
}

0 commit comments

Comments
 (0)