Skip to content

Commit 050a68f

Browse files
authored
fix: threading issues with locks (#130)
1 parent 350926d commit 050a68f

File tree

7 files changed

+286
-212
lines changed

7 files changed

+286
-212
lines changed

Sources/UnleashProxyClientSwift/Client/UnleashProxyClientSwift.swift

Lines changed: 80 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,26 @@ import SwiftEventBus
33

44
@available(macOS 10.15, *)
55
public class UnleashClientBase {
6-
public var context: Context
7-
var timer: Timer?
6+
private var _context: Context
7+
8+
public var context: Context {
9+
get {
10+
lock.lock()
11+
let value = self._context
12+
lock.unlock()
13+
return value
14+
}
15+
set {
16+
lock.lock()
17+
self._context = newValue
18+
lock.unlock()
19+
}
20+
}
21+
var timer: DispatchSourceTimer?
822
var poller: Poller
923
var metrics: Metrics
1024
var connectionId: UUID
11-
private let queue = DispatchQueue(label: "com.unleash.clientbase", attributes: .concurrent)
25+
private let lock = NSLock()
1226

1327
public init(
1428
unleashUrl: String,
@@ -63,9 +77,9 @@ public class UnleashClientBase {
6377
self.metrics = Metrics(appName: appName, metricsInterval: Double(metricsInterval), clock: { return Date() }, disableMetrics: disableMetrics, poster: urlSessionPoster, url: url, clientKey: clientKey, customHeaders: customHeaders, connectionId: connectionId)
6478
}
6579

66-
self.context = Context(appName: appName, environment: environment, sessionId: String(Int.random(in: 0..<1_000_000_000)))
80+
self._context = Context(appName: appName, environment: environment, sessionId: String(Int.random(in: 0..<1_000_000_000)))
6781
if let providedContext = context {
68-
self.context = self.calculateContext(context: providedContext)
82+
self._context = self.calculateContext(context: providedContext)
6983
}
7084
}
7185

@@ -75,13 +89,13 @@ public class UnleashClientBase {
7589
completionHandler: ((PollerError?) -> Void)? = nil
7690
) -> Void {
7791
Printer.showPrintStatements = printToConsole
78-
self.stopPolling()
79-
poller.start(
80-
bootstrapping: bootstrap.toggles,
81-
context: context,
82-
completionHandler: completionHandler
83-
)
84-
metrics.start()
92+
self.stopPolling()
93+
poller.start(
94+
bootstrapping: bootstrap.toggles,
95+
context: context,
96+
completionHandler: completionHandler
97+
)
98+
metrics.start()
8599
}
86100

87101
private func stopPolling() -> Void {
@@ -90,55 +104,55 @@ public class UnleashClientBase {
90104
}
91105

92106
public func stop() -> Void {
93-
self.stopPolling();
107+
self.stopPolling()
108+
lock.lock()
109+
timer?.cancel()
110+
timer = nil
111+
lock.unlock()
94112
UnleashEvent.allCases.forEach { self.unsubscribe($0) }
95113
}
96114

97115
public func isEnabled(name: String) -> Bool {
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-
}
116+
let toggle = poller.getFeature(name: name)
117+
let enabled = toggle?.enabled ?? false
118+
let contextSnapshot = self.context
114119

115-
return enabled
120+
metrics.count(name: name, enabled: enabled)
121+
122+
if let toggle = toggle, toggle.impressionData {
123+
DispatchQueue.global(qos: .background).async {
124+
SwiftEventBus.post("impression", sender: ImpressionEvent(
125+
toggleName: name,
126+
enabled: enabled,
127+
context: contextSnapshot
128+
))
129+
}
116130
}
131+
132+
return enabled
117133
}
118134

119135
public func getVariant(name: String) -> Variant {
120-
return queue.sync {
121-
let toggle = poller.getFeature(name: name)
122-
let variant = toggle?.variant ?? .defaultDisabled
123-
let enabled = toggle?.enabled ?? false
124-
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-
}
136+
let toggle = poller.getFeature(name: name)
137+
let variant = toggle?.variant ?? .defaultDisabled
138+
let enabled = toggle?.enabled ?? false
139+
let contextSnapshot = self.context
140+
141+
metrics.count(name: name, enabled: enabled)
142+
metrics.countVariant(name: name, variant: variant.name)
139143

140-
return variant
144+
if let toggle = toggle, toggle.impressionData {
145+
DispatchQueue.global(qos: .background).async {
146+
SwiftEventBus.post("impression", sender: ImpressionEvent(
147+
toggleName: name,
148+
enabled: enabled,
149+
variant: variant,
150+
context: contextSnapshot
151+
))
152+
}
141153
}
154+
155+
return variant
142156
}
143157

144158
public func subscribe(name: String, callback: @escaping () -> Void) {
@@ -185,15 +199,16 @@ public class UnleashClientBase {
185199
unsubscribe(name: event.rawValue)
186200
}
187201

188-
public func updateContext(context: [String: String], properties: [String:String]? = nil, completionHandler: ((PollerError?) -> Void)? = nil) {
189-
if Thread.isMainThread {
190-
self.context = self.calculateContext(context: context, properties: properties)
202+
public func updateContext(
203+
context: [String: String],
204+
properties: [String: String]? = nil,
205+
completionHandler: ((PollerError?) -> Void)? = nil
206+
) {
207+
let newContext = self.calculateContext(context: context, properties: properties)
208+
self.context = newContext
209+
210+
DispatchQueue.global(qos: .background).async {
191211
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-
}
197212
}
198213
}
199214

@@ -211,11 +226,13 @@ public class UnleashClientBase {
211226
newProperties[key] = value
212227
}
213228

214-
let sessionId = context["sessionId"] ?? self.context.sessionId;
215-
229+
let currentContext = self.context
230+
231+
let sessionId = context["sessionId"] ?? currentContext.sessionId;
232+
216233
let newContext = Context(
217-
appName: self.context.appName,
218-
environment: self.context.environment,
234+
appName: currentContext.appName,
235+
environment: currentContext.environment,
219236
userId: context["userId"],
220237
sessionId: sessionId,
221238
remoteAddress: context["remoteAddress"],
@@ -243,7 +260,7 @@ public class UnleashClient: UnleashClientBase, ObservableObject {
243260
}
244261
}
245262
}
246-
263+
247264
@MainActor
248265
public func updateContext(
249266
context: [String: String],

Sources/UnleashProxyClientSwift/Metrics/Metrics.swift

Lines changed: 62 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@ public class Metrics {
99
let poster: PosterHandler
1010
let clock: () -> Date
1111
var disableMetrics: Bool
12-
var timer: Timer?
12+
var timer: DispatchSourceTimer?
1313
var bucket: Bucket
1414
let url: URL
1515
let customHeaders: [String: String]
1616
let connectionId: UUID
1717

18+
private let lock = NSLock()
19+
1820
init(appName: String,
1921
metricsInterval: TimeInterval,
2022
clock: @escaping () -> Date,
@@ -37,49 +39,82 @@ public class Metrics {
3739
}
3840

3941
func start() {
40-
if disableMetrics { return }
42+
lock.lock()
43+
let isDisabled = self.disableMetrics
44+
lock.unlock()
45+
46+
if isDisabled { return }
47+
48+
lock.lock()
49+
self.timer?.cancel()
50+
self.timer = nil
51+
let interval = self.metricsInterval
52+
lock.unlock()
4153

42-
self.timer = Timer.scheduledTimer(withTimeInterval: metricsInterval, repeats: true) { _ in
54+
let timer = DispatchSource.makeTimerSource(queue: DispatchQueue.global(qos: .background))
55+
timer.schedule(deadline: .now() + interval, repeating: interval)
56+
timer.setEventHandler { [weak self] in
57+
guard let self = self else { return }
4358
self.sendMetrics()
4459
}
60+
timer.resume()
61+
62+
lock.lock()
63+
self.timer = timer
64+
lock.unlock()
4565
}
4666

4767
func stop() {
48-
self.timer?.invalidate()
68+
lock.lock()
69+
self.timer?.cancel()
70+
self.timer = nil
71+
lock.unlock()
4972
}
50-
51-
private let queue = DispatchQueue(label: "io.getunleash.metrics")
5273

5374
func count(name: String, enabled: Bool) {
54-
if disableMetrics { return }
55-
56-
queue.sync {
57-
var toggle = bucket.toggles[name] ?? ToggleMetrics()
58-
if enabled {
59-
toggle.yes += 1
60-
} else {
61-
toggle.no += 1
62-
}
63-
bucket.toggles[name] = toggle
75+
lock.lock()
76+
let isDisabled = self.disableMetrics
77+
if isDisabled {
78+
lock.unlock()
79+
return
6480
}
81+
82+
var toggle = bucket.toggles[name] ?? ToggleMetrics()
83+
if enabled {
84+
toggle.yes += 1
85+
} else {
86+
toggle.no += 1
87+
}
88+
bucket.toggles[name] = toggle
89+
lock.unlock()
6590
}
6691

6792
func countVariant(name: String, variant: String) {
68-
if disableMetrics { return }
69-
70-
queue.sync {
71-
var toggle = bucket.toggles[name] ?? ToggleMetrics()
72-
toggle.variants[variant, default: 0] += 1
73-
bucket.toggles[name] = toggle
93+
lock.lock()
94+
let isDisabled = self.disableMetrics
95+
if isDisabled {
96+
lock.unlock()
97+
return
7498
}
99+
100+
var toggle = bucket.toggles[name] ?? ToggleMetrics()
101+
toggle.variants[variant, default: 0] += 1
102+
bucket.toggles[name] = toggle
103+
lock.unlock()
75104
}
76105

77106
func sendMetrics() {
107+
let localBucket: Bucket
108+
let clockFunction: () -> Date
109+
110+
lock.lock()
78111
bucket.closeBucket()
79-
guard !bucket.isEmpty() else { return }
112+
localBucket = bucket
113+
clockFunction = self.clock
114+
bucket = Bucket(clock: clockFunction)
115+
lock.unlock()
80116

81-
let localBucket = bucket
82-
bucket = Bucket(clock: clock)
117+
guard !localBucket.isEmpty() else { return }
83118

84119
do {
85120
let payload = MetricsPayload(appName: appName, instanceId: "swift", bucket: localBucket)
@@ -111,8 +146,8 @@ public class Metrics {
111146
request.addValue(appName, forHTTPHeaderField: "unleash-appname")
112147
request.addValue(connectionId.uuidString, forHTTPHeaderField: "unleash-connection-id")
113148
request.setValue("unleash-client-swift:\(LibraryInfo.version)", forHTTPHeaderField: "unleash-sdk")
114-
if !self.customHeaders.isEmpty {
115-
for (key, value) in self.customHeaders {
149+
if !customHeaders.isEmpty {
150+
for (key, value) in customHeaders {
116151
request.setValue(value, forHTTPHeaderField: key)
117152
}
118153
}

Sources/UnleashProxyClientSwift/Poller/DictionaryStorageProvider.swift

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,26 @@ import Foundation
22

33
public class DictionaryStorageProvider: StorageProvider {
44
private var storage: [String: Toggle] = [:]
5-
private let queue = DispatchQueue(label: "com.unleash.storageprovider")
5+
private let lock = NSLock()
66

77
public init() {}
88

99
public func set(values: [String: Toggle]) {
10-
queue.async {
11-
self.storage = values
12-
}
10+
lock.lock()
11+
self.storage = values
12+
lock.unlock()
1313
}
1414

1515
public func value(key: String) -> Toggle? {
16-
queue.sync {
17-
return self.storage[key]
18-
}
16+
lock.lock()
17+
let result = self.storage[key]
18+
lock.unlock()
19+
return result
1920
}
20-
21+
2122
public func clear() {
22-
queue.sync {
23-
self.storage = [:]
24-
}
23+
lock.lock()
24+
self.storage = [:]
25+
lock.unlock()
2526
}
2627
}

0 commit comments

Comments
 (0)