Skip to content

Commit 0d914ee

Browse files
gwhelanLDgwhelanLD
authored
[sc-145480] Fix race condition on stop during reconnection. (#42)
1 parent eb10a5b commit 0d914ee

File tree

4 files changed

+146
-150
lines changed

4 files changed

+146
-150
lines changed

‎LDSwiftEventSource.xcodeproj/project.pbxproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@
253253
);
254254
runOnlyForDeploymentPostprocessing = 0;
255255
shellPath = /bin/sh;
256-
shellScript = "echo `pwd`\nif which mint >/dev/null; then\n /usr/bin/xcrun --sdk macosx mint run realm/SwiftLint\nelse\n echo \"warning: mint not installed, available from https://github.com/yonaskolb/Mint\"\nfi\n";
256+
shellScript = "# Adds support for Apple Silicon brew directory\nexport PATH=\"$PATH:/opt/homebrew/bin\"\n\nif which mint >/dev/null; then\n /usr/bin/xcrun --sdk macosx mint run realm/SwiftLint\nelse\n echo \"warning: mint not installed, available from https://github.com/yonaskolb/Mint\"\nfi\n";
257257
showEnvVarsInLog = 0;
258258
};
259259
/* End PBXShellScriptBuildPhase section */

‎Source/EventParser.swift

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import Foundation
22

3-
typealias ConnectionHandler = (setReconnectionTime: (TimeInterval) -> Void, setLastEventId: (String) -> Void)
4-
53
class EventParser {
64
private struct Constants {
75
static let dataLabel: Substring = "data"
@@ -11,15 +9,17 @@ class EventParser {
119
}
1210

1311
private let handler: EventHandler
14-
private let connectionHandler: ConnectionHandler
1512

1613
private var data: String = ""
17-
private var lastEventId: String?
1814
private var eventType: String = ""
15+
private var lastEventIdBuffer: String?
16+
private var lastEventId: String?
17+
private var currentRetry: TimeInterval
1918

20-
init(handler: EventHandler, connectionHandler: ConnectionHandler) {
19+
init(handler: EventHandler, initialEventId: String?, initialRetry: TimeInterval) {
2120
self.handler = handler
22-
self.connectionHandler = connectionHandler
21+
self.lastEventId = initialEventId
22+
self.currentRetry = initialRetry
2323
}
2424

2525
func parse(line: String) {
@@ -35,9 +35,13 @@ class EventParser {
3535
}
3636
}
3737

38-
func reset() {
38+
func getLastEventId() -> String? { lastEventId }
39+
40+
func reset() -> TimeInterval {
3941
data = ""
4042
eventType = ""
43+
lastEventIdBuffer = nil
44+
return currentRetry
4145
}
4246

4347
private func dropLeadingSpace(str: Substring) -> Substring {
@@ -56,23 +60,22 @@ class EventParser {
5660
// See https://github.com/whatwg/html/issues/689 for reasoning on not setting lastEventId if the value
5761
// contains a null code point.
5862
if !value.contains("\u{0000}") {
59-
lastEventId = String(value)
63+
lastEventIdBuffer = String(value)
6064
}
6165
case Constants.eventLabel:
6266
eventType = String(value)
6367
case Constants.retryLabel:
6468
if value.allSatisfy(("0"..."9").contains), let reconnectionTime = Int64(value) {
65-
connectionHandler.setReconnectionTime(Double(reconnectionTime) * 0.001)
69+
currentRetry = Double(reconnectionTime) * 0.001
6670
}
6771
default:
6872
break
6973
}
7074
}
7175

7276
private func dispatchEvent() {
73-
if let lastEventId = lastEventId {
74-
connectionHandler.setLastEventId(lastEventId)
75-
}
77+
lastEventId = lastEventIdBuffer ?? lastEventId
78+
lastEventIdBuffer = nil
7679
guard !data.isEmpty
7780
else {
7881
eventType = ""

‎Source/LDSwiftEventSource.swift

Lines changed: 73 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,30 @@ public class EventSource {
108108
}
109109
}
110110

111+
class ReconnectionTimer {
112+
private let maxDelay: TimeInterval
113+
private let resetInterval: TimeInterval
114+
115+
var backoffCount: Int = 0
116+
var connectedTime: Date?
117+
118+
init(maxDelay: TimeInterval, resetInterval: TimeInterval) {
119+
self.maxDelay = maxDelay
120+
self.resetInterval = resetInterval
121+
}
122+
123+
func reconnectDelay(baseDelay: TimeInterval) -> TimeInterval {
124+
backoffCount += 1
125+
if let connectedTime = connectedTime, Date().timeIntervalSince(connectedTime) >= resetInterval {
126+
backoffCount = 0
127+
}
128+
self.connectedTime = nil
129+
let maxSleep = min(maxDelay, baseDelay * pow(2.0, Double(backoffCount)))
130+
return maxSleep / 2 + Double.random(in: 0...(maxSleep / 2))
131+
}
132+
}
133+
134+
// MARK: EventSourceDelegate
111135
class EventSourceDelegate: NSObject, URLSessionDataDelegate {
112136
private let delegateQueue: DispatchQueue = DispatchQueue(label: "ESDelegateQueue")
113137
private let logger = Logs()
@@ -120,22 +144,19 @@ class EventSourceDelegate: NSObject, URLSessionDataDelegate {
120144
}
121145
}
122146

123-
private var lastEventId: String?
124-
private var reconnectTime: TimeInterval
125-
private var connectedTime: Date?
126-
127-
private var reconnectionAttempts: Int = 0
128-
private var errorHandlerAction: ConnectionErrorAction = .proceed
129147
private let utf8LineParser: UTF8LineParser = UTF8LineParser()
130-
// swiftlint:disable:next implicitly_unwrapped_optional
131-
private var eventParser: EventParser!
148+
private let eventParser: EventParser
149+
private let reconnectionTimer: ReconnectionTimer
132150
private var urlSession: URLSession?
133151
private var sessionTask: URLSessionDataTask?
134152

135153
init(config: EventSource.Config) {
136154
self.config = config
137-
self.lastEventId = config.lastEventId
138-
self.reconnectTime = config.reconnectTime
155+
self.eventParser = EventParser(handler: config.handler,
156+
initialEventId: config.lastEventId,
157+
initialRetry: config.reconnectTime)
158+
self.reconnectionTimer = ReconnectionTimer(maxDelay: config.maxReconnectTime,
159+
resetInterval: config.backoffResetThreshold)
139160
}
140161

141162
func start() {
@@ -153,19 +174,24 @@ class EventSourceDelegate: NSObject, URLSessionDataDelegate {
153174
}
154175

155176
func stop() {
156-
let previousState = readyState
157-
readyState = .shutdown
158-
sessionTask?.cancel()
159-
if previousState == .open {
160-
config.handler.onClosed()
177+
delegateQueue.async {
178+
let previousState = self.readyState
179+
self.readyState = .shutdown
180+
self.sessionTask?.cancel()
181+
if previousState == .open {
182+
self.config.handler.onClosed()
183+
}
184+
self.urlSession?.invalidateAndCancel()
185+
self.urlSession = nil
161186
}
162-
urlSession?.invalidateAndCancel()
163187
}
164188

165-
func getLastEventId() -> String? { lastEventId }
189+
func getLastEventId() -> String? { eventParser.getLastEventId() }
166190

167191
func createSession() -> URLSession {
168-
URLSession(configuration: config.urlSessionConfiguration, delegate: self, delegateQueue: nil)
192+
let opQueue = OperationQueue()
193+
opQueue.underlyingQueue = self.delegateQueue
194+
return URLSession(configuration: config.urlSessionConfiguration, delegate: self, delegateQueue: opQueue)
169195
}
170196

171197
func createRequest() -> URLRequest {
@@ -174,7 +200,7 @@ class EventSourceDelegate: NSObject, URLSessionDataDelegate {
174200
timeoutInterval: self.config.idleTimeout)
175201
urlRequest.httpMethod = self.config.method
176202
urlRequest.httpBody = self.config.body
177-
urlRequest.setValue(self.lastEventId, forHTTPHeaderField: "Last-Event-Id")
203+
urlRequest.setValue(eventParser.getLastEventId(), forHTTPHeaderField: "Last-Event-Id")
178204
urlRequest.allHTTPHeaderFields = self.config.headerTransform(
179205
urlRequest.allHTTPHeaderFields?.merging(self.config.headers) { $1 } ?? self.config.headers
180206
)
@@ -183,11 +209,6 @@ class EventSourceDelegate: NSObject, URLSessionDataDelegate {
183209

184210
private func connect() {
185211
logger.log(.info, "Starting EventSource client")
186-
let connectionHandler: ConnectionHandler = (
187-
setReconnectionTime: { [weak self] reconnectionTime in self?.reconnectTime = reconnectionTime },
188-
setLastEventId: { [weak self] eventId in self?.lastEventId = eventId }
189-
)
190-
self.eventParser = EventParser(handler: self.config.handler, connectionHandler: connectionHandler)
191212
let task = urlSession?.dataTask(with: createRequest())
192213
task?.resume()
193214
sessionTask = task
@@ -201,67 +222,44 @@ class EventSourceDelegate: NSObject, URLSessionDataDelegate {
201222
return action
202223
}
203224

204-
private func afterComplete() {
205-
guard readyState != .shutdown
206-
else { return }
207-
208-
var nextState: ReadyState = .closed
209-
let currentState: ReadyState = readyState
210-
if errorHandlerAction == .shutdown {
211-
logger.log(.info, "Connection has been explicitly shut down by error handler")
212-
nextState = .shutdown
213-
}
214-
readyState = nextState
215-
216-
if currentState == .open {
217-
config.handler.onClosed()
218-
}
219-
220-
if nextState != .shutdown {
221-
reconnect()
222-
}
223-
}
224-
225-
private func reconnect() {
226-
reconnectionAttempts += 1
227-
228-
if let connectedTime = connectedTime, Date().timeIntervalSince(connectedTime) >= config.backoffResetThreshold {
229-
reconnectionAttempts = 0
230-
}
231-
self.connectedTime = nil
232-
233-
let maxSleep = min(config.maxReconnectTime, reconnectTime * pow(2.0, Double(reconnectionAttempts)))
234-
let sleep = maxSleep / 2 + Double.random(in: 0...(maxSleep / 2))
235-
236-
logger.log(.info, "Waiting %.3f seconds before reconnecting...", sleep)
237-
delegateQueue.asyncAfter(deadline: .now() + sleep) { [weak self] in
238-
self?.connect()
239-
}
240-
}
241-
242225
// MARK: URLSession Delegates
243226

244227
// Tells the delegate that the task finished transferring data.
245228
public func urlSession(_ session: URLSession,
246229
task: URLSessionTask,
247230
didCompleteWithError error: Error?) {
248231
utf8LineParser.closeAndReset()
249-
eventParser.reset()
232+
let currentRetry = eventParser.reset()
233+
234+
guard readyState != .shutdown
235+
else { return }
250236

251237
if let error = error {
252-
// Ignore cancelled error
253-
if (error as NSError).code == NSURLErrorCancelled {
254-
} else if readyState != .shutdown && errorHandlerAction != .shutdown {
238+
if (error as NSError).code != NSURLErrorCancelled {
255239
logger.log(.info, "Connection error: %@", error.localizedDescription)
256-
errorHandlerAction = dispatchError(error: error)
257-
} else {
258-
errorHandlerAction = .shutdown
240+
if dispatchError(error: error) == .shutdown {
241+
logger.log(.info, "Connection has been explicitly shut down by error handler")
242+
if readyState == .open {
243+
config.handler.onClosed()
244+
}
245+
readyState = .shutdown
246+
return
247+
}
259248
}
260249
} else {
261250
logger.log(.info, "Connection unexpectedly closed.")
262251
}
263252

264-
afterComplete()
253+
if readyState == .open {
254+
config.handler.onClosed()
255+
}
256+
257+
readyState = .closed
258+
let sleep = reconnectionTimer.reconnectDelay(baseDelay: currentRetry)
259+
logger.log(.info, "Waiting %.3f seconds before reconnecting...", sleep)
260+
delegateQueue.asyncAfter(deadline: .now() + sleep) { [weak self] in
261+
self?.connect()
262+
}
265263
}
266264

267265
// Tells the delegate that the data task received the initial reply (headers) from the server.
@@ -280,13 +278,16 @@ class EventSourceDelegate: NSObject, URLSessionDataDelegate {
280278
// swiftlint:disable:next force_cast
281279
let httpResponse = response as! HTTPURLResponse
282280
if (200..<300).contains(httpResponse.statusCode) {
283-
connectedTime = Date()
281+
reconnectionTimer.connectedTime = Date()
284282
readyState = .open
285283
config.handler.onOpened()
286284
completionHandler(.allow)
287285
} else {
288286
logger.log(.info, "Unsuccessful response: %d", httpResponse.statusCode)
289-
errorHandlerAction = dispatchError(error: UnsuccessfulResponseError(responseCode: httpResponse.statusCode))
287+
if dispatchError(error: UnsuccessfulResponseError(responseCode: httpResponse.statusCode)) == .shutdown {
288+
logger.log(.info, "Connection has been explicitly shut down by error handler")
289+
readyState = .shutdown
290+
}
290291
completionHandler(.cancel)
291292
}
292293
}

0 commit comments

Comments
 (0)