Skip to content

Commit d4e8ecd

Browse files
author
Pushkar Kulkarni
committed
[URLSession]Fix for a timeout bug(SR-2681)
1 parent 4eac74e commit d4e8ecd

File tree

3 files changed

+39
-7
lines changed

3 files changed

+39
-7
lines changed

Foundation/NSURLSession/EasyHandle.swift

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ internal final class _EasyHandle {
5656
fileprivate var headerList: _CurlStringList?
5757
fileprivate var pauseState: _PauseState = []
5858
internal var fileLength: Int64 = 0
59+
internal var timeoutTimer: _TimeoutSource!
60+
5961
init(delegate: _EasyHandleDelegate) {
6062
self.delegate = delegate
6163
setupCallbacks()
@@ -387,31 +389,47 @@ fileprivate extension _EasyHandle {
387389
}
388390

389391
fileprivate extension _EasyHandle {
392+
393+
func resetTimer() {
394+
//simply create a new timer with the same queue, timeout and handler
395+
//this must cancel the old handler and reset the timer
396+
timeoutTimer = _TimeoutSource(queue: timeoutTimer.queue, milliseconds: timeoutTimer.milliseconds, handler: timeoutTimer.handler)
397+
}
398+
390399
/// Forward the libcurl callbacks into Swift methods
391400
func setupCallbacks() {
392401
// write
393402
try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionWRITEDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
394403

395404
try! CFURLSession_easy_setopt_wc(rawHandle, CFURLSessionOptionWRITEFUNCTION) { (data: UnsafeMutablePointer<Int8>, size: Int, nmemb: Int, userdata: UnsafeMutableRawPointer?) -> Int in
396405
guard let handle = _EasyHandle.from(callbackUserData: userdata) else { return 0 }
406+
defer {
407+
handle.resetTimer()
408+
}
397409
return handle.didReceive(data: data, size: size, nmemb: nmemb)
398-
}.asError()
410+
}.asError()
399411

400412
// read
401413
try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionREADDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
402414
try! CFURLSession_easy_setopt_wc(rawHandle, CFURLSessionOptionREADFUNCTION) { (data: UnsafeMutablePointer<Int8>, size: Int, nmemb: Int, userdata: UnsafeMutableRawPointer?) -> Int in
403415
guard let handle = _EasyHandle.from(callbackUserData: userdata) else { return 0 }
416+
defer {
417+
handle.resetTimer()
418+
}
404419
return handle.fill(writeBuffer: data, size: size, nmemb: nmemb)
405-
}.asError()
420+
}.asError()
406421

407422
// header
408423
try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionHEADERDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
409424
try! CFURLSession_easy_setopt_wc(rawHandle, CFURLSessionOptionHEADERFUNCTION) { (data: UnsafeMutablePointer<Int8>, size: Int, nmemb: Int, userdata: UnsafeMutableRawPointer?) -> Int in
410425
guard let handle = _EasyHandle.from(callbackUserData: userdata) else { return 0 }
426+
defer {
427+
handle.resetTimer()
428+
}
411429
var length = Double()
412430
try! CFURLSession_easy_getinfo_double(handle.rawHandle, CFURLSessionInfoCONTENT_LENGTH_DOWNLOAD, &length).asError()
413431
return handle.didReceive(headerData: data, size: size, nmemb: nmemb, fileLength: length)
414-
}.asError()
432+
}.asError()
415433

416434
// socket options
417435
try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionSOCKOPTDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
@@ -424,7 +442,7 @@ fileprivate extension _EasyHandle {
424442
} catch {
425443
return 1
426444
}
427-
}.asError()
445+
}.asError()
428446
// seeking in input stream
429447
try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionSEEKDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
430448
try! CFURLSession_easy_setopt_seek(rawHandle, CFURLSessionOptionSEEKFUNCTION, { (userdata, offset, origin) -> Int32 in

Foundation/NSURLSession/MultiHandle.swift

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,11 +300,15 @@ fileprivate extension URLSession._MultiHandle._SocketRegisterAction {
300300

301301
/// A helper class that wraps a libdispatch timer.
302302
///
303-
/// Used to implement the timeout of `URLSession.MultiHandle`.
304-
fileprivate class _TimeoutSource {
303+
/// Used to implement the timeout of `URLSession.MultiHandle` and `URLSession.EasyHandle`
304+
class _TimeoutSource {
305305
let rawSource: DispatchSource
306306
let milliseconds: Int
307+
let queue: DispatchQueue //needed to restart the timer for EasyHandles
308+
let handler: DispatchWorkItem //needed to restart the timer for EasyHandles
307309
init(queue: DispatchQueue, milliseconds: Int, handler: DispatchWorkItem) {
310+
self.queue = queue
311+
self.handler = handler
308312
self.milliseconds = milliseconds
309313
self.rawSource = DispatchSource.makeTimerSource(queue: queue) as! DispatchSource
310314

Foundation/NSURLSession/NSURLSessionTask.swift

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,7 +562,14 @@ fileprivate extension URLSessionTask {
562562
//set the request timeout
563563
//TODO: the timeout value needs to be reset on every data transfer
564564
let s = session as! URLSession
565-
easyHandle.set(timeout: Int(s.configuration.timeoutIntervalForRequest))
565+
let timeoutInterval = Int(s.configuration.timeoutIntervalForRequest) * 1000
566+
let timeoutHandler = DispatchWorkItem { [weak self] in
567+
guard let currentTask = self else { fatalError("Timeout on a task that doesn't exist") } //this guard must always pass
568+
currentTask.internalState = .transferFailed
569+
let urlError = URLError(_nsError: NSError(domain: NSURLErrorDomain, code: NSURLErrorTimedOut, userInfo: nil))
570+
currentTask.completeTask(withError: urlError)
571+
}
572+
easyHandle.timeoutTimer = _TimeoutSource(queue: workQueue, milliseconds: timeoutInterval, handler: timeoutHandler)
566573

567574
easyHandle.set(automaticBodyDecompression: true)
568575
easyHandle.set(requestMethod: request.httpMethod ?? "GET")
@@ -823,6 +830,9 @@ extension URLSessionTask {
823830
}
824831
self.response = response
825832

833+
//We don't want a timeout to be triggered after this. The timeout timer needs to be cancelled.
834+
easyHandle.timeoutTimer = nil
835+
826836
//because we deregister the task with the session on internalState being set to taskCompleted
827837
//we need to do the latter after the delegate/handler was notified/invoked
828838
switch session.behaviour(for: self) {

0 commit comments

Comments
 (0)