Skip to content

Commit cba3aef

Browse files
committed
refactor: Fix DataTask cancel() method design issue
- Add URLSession as instance variable to DataTask class - Remove urlSession parameter from cancel() method - Update all related methods to use instance variable - Improve memory management by setting urlSession to nil after cancel - Fix issue where cancel() couldn't access the correct URLSession instance This resolves the design issue where urlSession was created inside events() function but cancel() method required it as a parameter, making it impossible to properly cancel the session from external calls.
1 parent 7b2f4f5 commit cba3aef

File tree

1 file changed

+29
-19
lines changed

1 file changed

+29
-19
lines changed

Sources/EventSource/EventSource.swift

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,17 @@ public extension EventSource {
138138
}
139139
}
140140

141+
private let _urlSession: Mutex<URLSession?> = Mutex(nil)
142+
143+
private var urlSession: URLSession? {
144+
get {
145+
_urlSession.withLock { $0 }
146+
}
147+
set {
148+
_urlSession.withLock { $0 = newValue }
149+
}
150+
}
151+
141152
private var urlSessionConfiguration: URLSessionConfiguration {
142153
let configuration = URLSessionConfiguration.default
143154
configuration.httpAdditionalHeaders = [
@@ -176,6 +187,9 @@ public extension EventSource {
176187
delegate: sessionDelegate,
177188
delegateQueue: nil
178189
)
190+
191+
self.urlSession = urlSession
192+
179193
let urlSessionDataTask = urlSession.dataTask(with: urlRequest)
180194

181195
let sessionDelegateTask = Task { [weak self] in
@@ -184,30 +198,29 @@ public extension EventSource {
184198

185199
switch event {
186200
case let .didCompleteWithError(error):
187-
handleSessionError(error, stream: continuation, urlSession: urlSession)
201+
handleSessionError(error, stream: continuation)
188202
case let .didReceiveResponse(response, completionHandler):
189203
handleSessionResponse(
190204
response,
191205
stream: continuation,
192-
urlSession: urlSession,
193206
completionHandler: completionHandler
194207
)
195208
case let .didReceiveData(data):
196-
parseMessages(from: data, stream: continuation, urlSession: urlSession)
209+
parseMessages(from: data, stream: continuation)
197210
}
198211
}
199212
}
200213

201214
#if compiler(>=6.0)
202215
continuation.onTermination = { @Sendable [weak self] _ in
203216
sessionDelegateTask.cancel()
204-
Task { self?.close(stream: continuation, urlSession: urlSession) }
217+
Task { self?.close(stream: continuation) }
205218
}
206219
#else
207220
continuation.onTermination = { @Sendable _ in
208221
sessionDelegateTask.cancel()
209222
Task { [weak self] in
210-
await self?.close(stream: continuation, urlSession: urlSession)
223+
await self?.close(stream: continuation)
211224
}
212225
}
213226
#endif
@@ -220,11 +233,10 @@ public extension EventSource {
220233

221234
private func handleSessionError(
222235
_ error: Error?,
223-
stream continuation: AsyncStream<EventType>.Continuation,
224-
urlSession: URLSession
236+
stream continuation: AsyncStream<EventType>.Continuation
225237
) {
226238
guard readyState != .closed else {
227-
close(stream: continuation, urlSession: urlSession)
239+
close(stream: continuation)
228240
return
229241
}
230242

@@ -234,13 +246,12 @@ public extension EventSource {
234246
}
235247

236248
// Close connection
237-
close(stream: continuation, urlSession: urlSession)
249+
close(stream: continuation)
238250
}
239251

240252
private func handleSessionResponse(
241253
_ response: URLResponse,
242254
stream continuation: AsyncStream<EventType>.Continuation,
243-
urlSession: URLSession,
244255
completionHandler: @escaping (URLSession.ResponseDisposition) -> Void
245256
) {
246257
guard readyState != .closed else {
@@ -256,7 +267,7 @@ public extension EventSource {
256267
// Stop connection when 204 response code, otherwise keep open
257268
guard httpResponse.statusCode != 204 else {
258269
completionHandler(.cancel)
259-
close(stream: continuation, urlSession: urlSession)
270+
close(stream: continuation)
260271
return
261272
}
262273

@@ -274,26 +285,24 @@ public extension EventSource {
274285
/// Closes the connection, if one was made,
275286
/// and sets the `readyState` property to `.closed`.
276287
/// - Returns: State before closing.
277-
private func close(stream continuation: AsyncStream<EventType>.Continuation, urlSession: URLSession) {
288+
private func close(stream continuation: AsyncStream<EventType>.Continuation) {
278289
let previousState = self.readyState
279290
if previousState != .closed {
280291
continuation.yield(.closed)
281292
continuation.finish()
282293
}
283-
cancel(urlSession: urlSession)
294+
cancel()
284295
}
285296

286297
private func parseMessages(
287298
from data: Data,
288-
stream continuation: AsyncStream<EventType>.Continuation,
289-
urlSession: URLSession
299+
stream continuation: AsyncStream<EventType>.Continuation
290300
) {
291301
if let httpResponseErrorStatusCode {
292302
self.httpResponseErrorStatusCode = nil
293303
handleSessionError(
294304
EventSourceError.connectionError(statusCode: httpResponseErrorStatusCode, response: data),
295-
stream: continuation,
296-
urlSession: urlSession
305+
stream: continuation
297306
)
298307
return
299308
}
@@ -325,10 +334,11 @@ public extension EventSource {
325334
/// The event stream supports cooperative task cancellation. However, it should be noted that
326335
/// canceling the parent Task only cancels the underlying `URLSessionDataTask` of
327336
/// ``EventSource/EventSource/DataTask``; this does not actually stop the ongoing request.
328-
public func cancel(urlSession: URLSession) {
337+
public func cancel() {
329338
readyState = .closed
330339
lastMessageId = ""
331-
urlSession.invalidateAndCancel()
340+
urlSession?.invalidateAndCancel()
341+
urlSession = nil
332342
}
333343
}
334344
}

0 commit comments

Comments
 (0)