Merge pull request #1182 from weissi/jw-fix-httpcookiestorage-threading
diff --git a/Foundation/HTTPCookieStorage.swift b/Foundation/HTTPCookieStorage.swift
index 045b69d..2209818 100644
--- a/Foundation/HTTPCookieStorage.swift
+++ b/Foundation/HTTPCookieStorage.swift
@@ -36,14 +36,34 @@
*/
open class HTTPCookieStorage: NSObject {
+ /* both sharedStorage and sharedCookieStorages are synchronized on sharedSyncQ */
private static var sharedStorage: HTTPCookieStorage?
private static var sharedCookieStorages: [String: HTTPCookieStorage] = [:] //for group storage containers
+ private static let sharedSyncQ = DispatchQueue(label: "org.swift.HTTPCookieStorage.sharedSyncQ")
+
+ /* only modified in init */
private var cookieFilePath: String!
- private let workQueue: DispatchQueue = DispatchQueue(label: "HTTPCookieStorage.workqueue")
- var allCookies: [String: HTTPCookie]
+
+ /* synchronized on syncQ, please don't use _allCookies directly outside of init/deinit */
+ private var _allCookies: [String: HTTPCookie]
+ private var allCookies: [String: HTTPCookie] {
+ get {
+ if #available(OSX 10.12, iOS 10.0, tvOS 10.0, watchOS 3.0, *) {
+ dispatchPrecondition(condition: DispatchPredicate.onQueue(self.syncQ))
+ }
+ return self._allCookies
+ }
+ set {
+ if #available(OSX 10.12, iOS 10.0, tvOS 10.0, watchOS 3.0, *) {
+ dispatchPrecondition(condition: DispatchPredicate.onQueue(self.syncQ))
+ }
+ self._allCookies = newValue
+ }
+ }
+ private let syncQ = DispatchQueue(label: "org.swift.HTTPCookieStorage.syncQ")
private init(cookieStorageName: String) {
- allCookies = [:]
+ _allCookies = [:]
cookieAcceptPolicy = .always
super.init()
let bundlePath = Bundle.main.bundlePath
@@ -59,9 +79,11 @@
private func loadPersistedCookies() {
guard let cookies = NSMutableDictionary(contentsOfFile: cookieFilePath) else { return }
var cookies0 = _SwiftValue.fetch(cookies) as? [String: [String: Any]] ?? [:]
- for key in cookies0.keys {
- if let cookie = createCookie(cookies0[key]!) {
- allCookies[key] = cookie
+ self.syncQ.sync {
+ for key in cookies0.keys {
+ if let cookie = createCookie(cookies0[key]!) {
+ allCookies[key] = cookie
+ }
}
}
}
@@ -87,11 +109,7 @@
}
open var cookies: [HTTPCookie]? {
- var theCookies: [HTTPCookie]?
- workQueue.sync {
- theCookies = Array(self.allCookies.values)
- }
- return theCookies
+ return Array(self.syncQ.sync { self.allCookies.values })
}
/*!
@@ -103,10 +121,12 @@
*/
open class var shared: HTTPCookieStorage {
get {
- if sharedStorage == nil {
- sharedStorage = HTTPCookieStorage(cookieStorageName: "shared")
+ return sharedSyncQ.sync {
+ if sharedStorage == nil {
+ sharedStorage = HTTPCookieStorage(cookieStorageName: "shared")
+ }
+ return sharedStorage!
}
- return sharedStorage!
}
}
@@ -122,12 +142,14 @@
method with the same identifier will return the same cookie storage instance.
*/
open class func sharedCookieStorage(forGroupContainerIdentifier identifier: String) -> HTTPCookieStorage {
- guard let cookieStorage = sharedCookieStorages[identifier] else {
- let newCookieStorage = HTTPCookieStorage(cookieStorageName: identifier)
- sharedCookieStorages[identifier] = newCookieStorage
- return newCookieStorage
+ return sharedSyncQ.sync {
+ guard let cookieStorage = sharedCookieStorages[identifier] else {
+ let newCookieStorage = HTTPCookieStorage(cookieStorageName: identifier)
+ sharedCookieStorages[identifier] = newCookieStorage
+ return newCookieStorage
+ }
+ return cookieStorage
}
- return cookieStorage
}
@@ -138,7 +160,7 @@
same name, domain and path, if any.
*/
open func setCookie(_ cookie: HTTPCookie) {
- workQueue.sync {
+ self.syncQ.sync {
guard cookieAcceptPolicy != .never else { return }
//add or override
@@ -173,9 +195,13 @@
}
private func updatePersistentStore() {
+ if #available(OSX 10.12, iOS 10.0, tvOS 10.0, watchOS 3.0, *) {
+ dispatchPrecondition(condition: DispatchPredicate.onQueue(self.syncQ))
+ }
+
//persist cookies
var persistDictionary: [String : [String : Any]] = [:]
- let persistable = allCookies.filter { (_, value) in
+ let persistable = self.allCookies.filter { (_, value) in
value.expiresDate != nil &&
value.isSessionOnly == false &&
value.expiresDate!.timeIntervalSinceNow > 0
@@ -190,14 +216,22 @@
}
/*!
+ @method lockedDeleteCookie:
+ @abstract Delete the specified cookie, for internal callers already on syncQ.
+ */
+ private func lockedDeleteCookie(_ cookie: HTTPCookie) {
+ let key = cookie.domain + cookie.path + cookie.name
+ self.allCookies.removeValue(forKey: key)
+ updatePersistentStore()
+ }
+
+ /*!
@method deleteCookie:
@abstract Delete the specified cookie
*/
open func deleteCookie(_ cookie: HTTPCookie) {
- let key = cookie.domain + cookie.path + cookie.name
- workQueue.sync {
- self.allCookies.removeValue(forKey: key)
- updatePersistentStore()
+ self.syncQ.sync {
+ self.lockedDeleteCookie(cookie)
}
}
@@ -206,13 +240,15 @@
@abstract Delete all cookies from the cookie storage since the provided date.
*/
open func removeCookies(since date: Date) {
- let cookiesSinceDate = allCookies.values.filter {
- $0.properties![.created] as! Double > date.timeIntervalSinceReferenceDate
+ self.syncQ.sync {
+ let cookiesSinceDate = self.allCookies.values.filter {
+ $0.properties![.created] as! Double > date.timeIntervalSinceReferenceDate
+ }
+ for cookie in cookiesSinceDate {
+ lockedDeleteCookie(cookie)
+ }
+ updatePersistentStore()
}
- for cookie in cookiesSinceDate {
- deleteCookie(cookie)
- }
- updatePersistentStore()
}
/*!
@@ -226,12 +262,8 @@
into a set of header fields to add to a request.
*/
open func cookies(for url: URL) -> [HTTPCookie]? {
- var cookies: [HTTPCookie]?
guard let host = url.host else { return nil }
- workQueue.sync {
- cookies = Array(allCookies.values.filter{ $0.domain == host })
- }
- return cookies
+ return Array(self.syncQ.sync(execute: {allCookies}).values.filter{ $0.domain == host })
}
/*!
diff --git a/TestFoundation/TestHTTPCookieStorage.swift b/TestFoundation/TestHTTPCookieStorage.swift
index b195a08..f65a01e 100644
--- a/TestFoundation/TestHTTPCookieStorage.swift
+++ b/TestFoundation/TestHTTPCookieStorage.swift
@@ -14,6 +14,7 @@
import SwiftFoundation
import SwiftXCTest
#endif
+import Dispatch
class TestHTTPCookieStorage: XCTestCase {
@@ -24,6 +25,7 @@
static var allTests: [(String, (TestHTTPCookieStorage) -> () throws -> Void)] {
return [
+ ("test_sharedCookieStorageAccessedFromMultipleThreads", test_sharedCookieStorageAccessedFromMultipleThreads),
("test_BasicStorageAndRetrieval", test_BasicStorageAndRetrieval),
("test_deleteCookie", test_deleteCookie),
("test_removeCookies", test_removeCookies),
@@ -34,6 +36,27 @@
]
}
+ func test_sharedCookieStorageAccessedFromMultipleThreads() {
+ let q = DispatchQueue.global()
+ let syncQ = DispatchQueue(label: "TestHTTPCookieStorage.syncQ")
+ var allCookieStorages: [HTTPCookieStorage] = []
+ let g = DispatchGroup()
+ for _ in 0..<64 {
+ g.enter()
+ q.async {
+ let mySharedCookieStore = HTTPCookieStorage.shared
+ syncQ.async {
+ allCookieStorages.append(mySharedCookieStore)
+ g.leave()
+ }
+ }
+ }
+ g.wait()
+ let cookieStorages = syncQ.sync { allCookieStorages }
+ let mySharedCookieStore = HTTPCookieStorage.shared
+ XCTAssertTrue(cookieStorages.reduce(true, { $0 && $1 === mySharedCookieStore }), "\(cookieStorages)")
+ }
+
func test_BasicStorageAndRetrieval() {
basicStorageAndRetrieval(with: .shared)
basicStorageAndRetrieval(with: .groupContainer("test"))
