Merge pull request #911 from pushkarnk/timeout-bug

Author: swift-ci

Foundation/NSURLSession/EasyHandle.swift

@@ -56,6 +56,8 @@ internal final class _EasyHandle {
     fileprivate var headerList: _CurlStringList?
     fileprivate var pauseState: _PauseState = []
     internal var fileLength: Int64 = 0
+    internal var timeoutTimer: _TimeoutSource!
+
     init(delegate: _EasyHandleDelegate) {
         self.delegate = delegate
         setupCallbacks()
@@ -387,31 +389,47 @@ fileprivate extension _EasyHandle {
 }
 
 fileprivate extension _EasyHandle {
+
+    func resetTimer() {
+        //simply create a new timer with the same queue, timeout and handler
+        //this must cancel the old handler and reset the timer
+        timeoutTimer = _TimeoutSource(queue: timeoutTimer.queue, milliseconds: timeoutTimer.milliseconds, handler: timeoutTimer.handler)
+    }
+
     /// Forward the libcurl callbacks into Swift methods
     func setupCallbacks() {
         // write
         try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionWRITEDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
 
         try! CFURLSession_easy_setopt_wc(rawHandle, CFURLSessionOptionWRITEFUNCTION) { (data: UnsafeMutablePointer<Int8>, size: Int, nmemb: Int, userdata: UnsafeMutableRawPointer?) -> Int in
             guard let handle = _EasyHandle.from(callbackUserData: userdata) else { return 0 }
+            defer {
+                handle.resetTimer()
+            }
             return handle.didReceive(data: data, size: size, nmemb: nmemb)
-            }.asError()
+        }.asError()
 
         // read
         try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionREADDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
         try! CFURLSession_easy_setopt_wc(rawHandle, CFURLSessionOptionREADFUNCTION) { (data: UnsafeMutablePointer<Int8>, size: Int, nmemb: Int, userdata: UnsafeMutableRawPointer?) -> Int in
             guard let handle = _EasyHandle.from(callbackUserData: userdata) else { return 0 }
+            defer {
+                handle.resetTimer()
+            }
             return handle.fill(writeBuffer: data, size: size, nmemb: nmemb)
-            }.asError()
+        }.asError()
 
         // header
         try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionHEADERDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
         try! CFURLSession_easy_setopt_wc(rawHandle, CFURLSessionOptionHEADERFUNCTION) { (data: UnsafeMutablePointer<Int8>, size: Int, nmemb: Int, userdata: UnsafeMutableRawPointer?) -> Int in
             guard let handle = _EasyHandle.from(callbackUserData: userdata) else { return 0 }
+            defer {
+                handle.resetTimer()
+            }
             var length = Double()
             try! CFURLSession_easy_getinfo_double(handle.rawHandle, CFURLSessionInfoCONTENT_LENGTH_DOWNLOAD, &length).asError()
             return handle.didReceive(headerData: data, size: size, nmemb: nmemb, fileLength: length)
-            }.asError()
+        }.asError()
 
         // socket options
         try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionSOCKOPTDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
@@ -424,7 +442,7 @@ fileprivate extension _EasyHandle {
             } catch {
                 return 1
             }
-            }.asError()
+        }.asError()
         // seeking in input stream
         try! CFURLSession_easy_setopt_ptr(rawHandle, CFURLSessionOptionSEEKDATA, UnsafeMutableRawPointer(Unmanaged.passUnretained(self).toOpaque())).asError()
         try! CFURLSession_easy_setopt_seek(rawHandle, CFURLSessionOptionSEEKFUNCTION, { (userdata, offset, origin) -> Int32 in

Foundation/NSURLSession/MultiHandle.swift

@@ -300,11 +300,15 @@ fileprivate extension URLSession._MultiHandle._SocketRegisterAction {
 
 /// A helper class that wraps a libdispatch timer.
 ///
-/// Used to implement the timeout of `URLSession.MultiHandle`.
-fileprivate class _TimeoutSource {
+/// Used to implement the timeout of `URLSession.MultiHandle` and `URLSession.EasyHandle`
+class _TimeoutSource {
     let rawSource: DispatchSource 
     let milliseconds: Int
+    let queue: DispatchQueue        //needed to restart the timer for EasyHandles
+    let handler: DispatchWorkItem   //needed to restart the timer for EasyHandles
     init(queue: DispatchQueue, milliseconds: Int, handler: DispatchWorkItem) {
+        self.queue = queue
+        self.handler = handler
         self.milliseconds = milliseconds
         self.rawSource = DispatchSource.makeTimerSource(queue: queue) as! DispatchSource
 

Foundation/NSURLSession/NSURLSessionTask.swift

@@ -562,7 +562,14 @@ fileprivate extension URLSessionTask {
         //set the request timeout
         //TODO: the timeout value needs to be reset on every data transfer
         let s = session as! URLSession
-        easyHandle.set(timeout: Int(s.configuration.timeoutIntervalForRequest))
+        let timeoutInterval = Int(s.configuration.timeoutIntervalForRequest) * 1000
+        let timeoutHandler = DispatchWorkItem { [weak self] in
+            guard let currentTask = self else { fatalError("Timeout on a task that doesn't exist") } //this guard must always pass
+            currentTask.internalState = .transferFailed
+            let urlError = URLError(_nsError: NSError(domain: NSURLErrorDomain, code: NSURLErrorTimedOut, userInfo: nil))
+            currentTask.completeTask(withError: urlError)
+        }
+        easyHandle.timeoutTimer = _TimeoutSource(queue: workQueue, milliseconds: timeoutInterval, handler: timeoutHandler)
 
         easyHandle.set(automaticBodyDecompression: true)
         easyHandle.set(requestMethod: request.httpMethod ?? "GET")
@@ -823,6 +830,9 @@ extension URLSessionTask {
         }
         self.response = response
 
+        //We don't want a timeout to be triggered after this. The timeout timer needs to be cancelled.
+        easyHandle.timeoutTimer = nil
+
         //because we deregister the task with the session on internalState being set to taskCompleted
         //we need to do the latter after the delegate/handler was notified/invoked
         switch session.behaviour(for: self) {
@@ -874,6 +884,10 @@ extension URLSessionTask {
         guard case .transferFailed = internalState else {
             fatalError("Trying to complete the task, but its transfer isn't complete / failed.")
         }
+
+        //We don't want a timeout to be triggered after this. The timeout timer needs to be cancelled.
+        easyHandle.timeoutTimer = nil
+
         switch session.behaviour(for: self) {
         case .taskDelegate(let delegate):
             guard let s = session as? URLSession else { fatalError() }

TestFoundation/HTTPServer.swift

@@ -77,6 +77,7 @@ class _TCPSocket {
             return sockaddr_in(sin_len: 0, sin_family: sa_family_t(AF_INET), sin_port: CFSwapInt16HostToBig(port), sin_addr: in_addr(s_addr: INADDR_ANY), sin_zero: (0,0,0,0,0,0,0,0) )
         #endif
     }
+
     func acceptConnection(notify: ServerSemaphore) throws {
         _ = try attempt("listen", valid: isZero, listen(listenSocket, SOMAXCONN))
         try socketAddress.withMemoryRebound(to: sockaddr.self, capacity: MemoryLayout<sockaddr>.size, {
@@ -112,7 +113,6 @@ class _TCPSocket {
             for item in texts {
                 sleep(UInt32(sendDelay))
                 var bytes = Array(item.utf8)
-                print(item)
                 _  = try attempt("write", valid: isNotNegative, CInt(write(connectionSocket, &bytes, bytes.count)))
             }
         } else {
@@ -160,7 +160,7 @@ class _HTTPServer {
         } else {
             deadlineTime = .now()
         }
-        
+
         DispatchQueue.main.asyncAfter(deadline: deadlineTime) {
             do {
                 try self.socket.writeData(header: response.header, body: response.body, sendDelay: sendDelay, bodyChunks: bodyChunks)

TestFoundation/TestNSURLSession.swift

@@ -33,7 +33,7 @@ class TestURLSession : XCTestCase {
             ("test_taskError", test_taskError),
             ("test_taskCopy", test_taskCopy),
             ("test_cancelTask", test_cancelTask),
-//            ("test_taskTimeout", test_taskTimeout),
+            ("test_taskTimeout", test_taskTimeout),
         ]
     }
 
@@ -47,7 +47,7 @@ class TestURLSession : XCTestCase {
                 try test.readAndRespond()
                 test.stop()
             } catch let e as ServerError {
-                if e.operation != "bind" { continue }
+                if e.operation == "bind" { continue }
                 throw e
             }
         }