Skip to content

Working around intermittent URLSession Test Failures on async wait. #982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 62 additions & 20 deletions Foundation/NSURLSession/NSURLSessionTask.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ open class URLSessionTask : NSObject, NSCopying {
}
/// All operations must run on this queue.
fileprivate let workQueue: DispatchQueue
/// This queue is used to make public attributes thread safe. It's a
/// **concurrent** queue and must be used with a barries when writing. This
/// allows multiple concurrent readers or a single writer.
fileprivate let taskAttributesIsolation: DispatchQueue
/// Using dispatch semaphore to make public attributes thread safe.
/// A semaphore is a simpler option against the usage of concurrent queue
/// as the critical sections are very short.
fileprivate let semaphore = DispatchSemaphore(value: 1)

public override init() {
// Darwin Foundation oddly allows calling this initializer, even though
Expand All @@ -88,7 +88,6 @@ open class URLSessionTask : NSObject, NSCopying {
originalRequest = nil
body = .none
workQueue = DispatchQueue(label: "URLSessionTask.notused.0")
taskAttributesIsolation = DispatchQueue(label: "URLSessionTask.notused.1", attributes: DispatchQueue.Attributes.concurrent)
let fileName = NSTemporaryDirectory() + NSUUID().uuidString + ".tmp"
_ = FileManager.default.createFile(atPath: fileName, contents: nil)
self.tempFileURL = URL(fileURLWithPath: fileName)
Expand All @@ -105,7 +104,6 @@ open class URLSessionTask : NSObject, NSCopying {
internal init(session: URLSession, request: URLRequest, taskIdentifier: Int, body: _Body) {
self.session = session
self.workQueue = session.workQueue
self.taskAttributesIsolation = session.taskAttributesIsolation
self.taskIdentifier = taskIdentifier
self.originalRequest = request
self.body = body
Expand Down Expand Up @@ -136,17 +134,32 @@ open class URLSessionTask : NSObject, NSCopying {
/// May differ from originalRequest due to http server redirection
/*@NSCopying*/ open fileprivate(set) var currentRequest: URLRequest? {
get {
return taskAttributesIsolation.sync { self._currentRequest }
semaphore.wait()
defer {
semaphore.signal()
}
return self._currentRequest
}
set {
semaphore.wait()
self._currentRequest = newValue
semaphore.signal()
}
//TODO: dispatch_barrier_async
set { taskAttributesIsolation.async(flags: .barrier) { self._currentRequest = newValue } }
}
fileprivate var _currentRequest: URLRequest? = nil
/*@NSCopying*/ open fileprivate(set) var response: URLResponse? {
get {
return taskAttributesIsolation.sync { self._response }
semaphore.wait()
defer {
semaphore.signal()
}
return self._response
}
set {
semaphore.wait()
self._response = newValue
semaphore.signal()
}
set { taskAttributesIsolation.async(flags: .barrier) { self._response = newValue } }
}
fileprivate var _response: URLResponse? = nil

Expand All @@ -158,20 +171,35 @@ open class URLSessionTask : NSObject, NSCopying {
/// Number of body bytes already received
open fileprivate(set) var countOfBytesReceived: Int64 {
get {
return taskAttributesIsolation.sync { self._countOfBytesReceived }
semaphore.wait()
defer {
semaphore.signal()
}
return self._countOfBytesReceived
}
set {
semaphore.wait()
self._countOfBytesReceived = newValue
semaphore.signal()
}
set { taskAttributesIsolation.async(flags: .barrier) { self._countOfBytesReceived = newValue } }
}
fileprivate var _countOfBytesReceived: Int64 = 0

/// Number of body bytes already sent */
open fileprivate(set) var countOfBytesSent: Int64 {
get {
return taskAttributesIsolation.sync { self._countOfBytesSent }
semaphore.wait()
defer {
semaphore.signal()
}
return self._countOfBytesSent
}
set {
semaphore.wait()
self._countOfBytesSent = newValue
semaphore.signal()
}
set { taskAttributesIsolation.async(flags: .barrier) { self._countOfBytesSent = newValue } }
}

fileprivate var _countOfBytesSent: Int64 = 0

/// Number of body bytes we expect to send, derived from the Content-Length of the HTTP request */
Expand Down Expand Up @@ -207,9 +235,17 @@ open class URLSessionTask : NSObject, NSCopying {
*/
open var state: URLSessionTask.State {
get {
return taskAttributesIsolation.sync { self._state }
semaphore.wait()
defer {
semaphore.signal()
}
return self._state
}
set {
semaphore.wait()
self._state = newValue
semaphore.signal()
}
set { taskAttributesIsolation.async(flags: .barrier) { self._state = newValue } }
}
fileprivate var _state: URLSessionTask.State = .suspended

Expand Down Expand Up @@ -289,10 +325,16 @@ open class URLSessionTask : NSObject, NSCopying {
/// URLSessionTask.highPriority, but use is not restricted to these.
open var priority: Float {
get {
return taskAttributesIsolation.sync { self._priority }
semaphore.wait()
defer {
semaphore.signal()
}
return self._priority
}
set {
taskAttributesIsolation.async(flags: .barrier) { self._priority = newValue }
semaphore.wait()
self._priority = newValue
semaphore.signal()
}
}
fileprivate var _priority: Float = URLSessionTask.defaultPriority
Expand Down
3 changes: 1 addition & 2 deletions TestFoundation/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ XCTMain([
testCase(TestURLRequest.allTests),
testCase(TestNSURLResponse.allTests),
testCase(TestNSHTTPURLResponse.allTests),
//Disabling because of https://bugs.swift.org/browse/SR-4655 and https://bugs.swift.org/browse/SR-4647
// testCase(TestURLSession.allTests),
testCase(TestURLSession.allTests),
testCase(TestNSNull.allTests),
testCase(TestNSUUID.allTests),
testCase(TestNSValue.allTests),
Expand Down