Răsfoiți Sursa

Merge pull request #1 from letgoapp/syncrhonize-get-access

Fixed concurrency on get calls
Eli Kohen 8 ani în urmă
părinte
comite
6d0e669cb9

+ 4 - 0
Distrib/KeychainSwiftDistrib.swift

@@ -47,6 +47,8 @@ open class KeychainSwift {
    
   */
   open var synchronizable: Bool = false
+
+  private let getLock = NSLock()
   
   /// Instantiate a KeychainSwift object
   public init() { }
@@ -170,6 +172,8 @@ open class KeychainSwift {
   
   */
   open func getData(_ key: String) -> Data? {
+    getLock.lock()
+    defer { getLock.unlock() }
     let prefixedKey = keyWithPrefix(key)
     
     var query: [String: Any] = [

+ 6 - 0
KeychainSwift.xcodeproj/project.pbxproj

@@ -51,6 +51,8 @@
 		7ED6C9BF1B1C13AA00FE8090 /* KeychainSwift.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 7ED6C96C1B1C118F00FE8090 /* KeychainSwift.framework */; };
 		7ED6C9C01B1C13AA00FE8090 /* KeychainSwift.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 7ED6C96C1B1C118F00FE8090 /* KeychainSwift.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
 		7EE5B9A11E32F6D400AA56FF /* KeychainSwiftCBridge.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7EE5B9A01E32F6D400AA56FF /* KeychainSwiftCBridge.swift */; };
+		C7E1DE4C1E4B7C9F003818F6 /* ConcurrencyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C7E1DE4A1E4B7C9F003818F6 /* ConcurrencyTests.swift */; };
+		C7E1DE4D1E4B7CAB003818F6 /* ConcurrencyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C7E1DE4A1E4B7C9F003818F6 /* ConcurrencyTests.swift */; };
 /* End PBXBuildFile section */
 
 /* Begin PBXContainerItemProxy section */
@@ -148,6 +150,7 @@
 		7ED6C9C61B1C16BC00FE8090 /* concatenate_swift_files.sh */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.script.sh; path = concatenate_swift_files.sh; sourceTree = "<group>"; };
 		7ED6C9C91B1C16EE00FE8090 /* KeychainSwiftDistrib.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = KeychainSwiftDistrib.swift; sourceTree = "<group>"; };
 		7EE5B9A01E32F6D400AA56FF /* KeychainSwiftCBridge.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = KeychainSwiftCBridge.swift; sourceTree = "<group>"; };
+		C7E1DE4A1E4B7C9F003818F6 /* ConcurrencyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ConcurrencyTests.swift; sourceTree = "<group>"; };
 /* End PBXFileReference section */
 
 /* Begin PBXFrameworksBuildPhase section */
@@ -293,6 +296,7 @@
 				2B7E7C4C1B81A3EC00D1C3B9 /* KeychainSwiftPrefixedTests.swift */,
 				7ED6C9931B1C12A200FE8090 /* KeychainSwiftTests-Bridging-Header.h */,
 				7E282DD61CED212500B55133 /* SynchronizableTests.swift */,
+				C7E1DE4A1E4B7C9F003818F6 /* ConcurrencyTests.swift */,
 				7E3A6B611D3F62C2007C5B1F /* macOS Tests */,
 				7ED6C97C1B1C118F00FE8090 /* Supporting Files */,
 			);
@@ -730,6 +734,7 @@
 				7E3A6B631D3F62C2007C5B1F /* macOS_Tests.swift in Sources */,
 				7E3A6B711D3F66C0007C5B1F /* AccessGroupTests.swift in Sources */,
 				7E3A6B741D3F66C0007C5B1F /* SynchronizableTests.swift in Sources */,
+				C7E1DE4D1E4B7CAB003818F6 /* ConcurrencyTests.swift in Sources */,
 				7E3A6B691D3F62CC007C5B1F /* KeychainSwiftAccessOptions.swift in Sources */,
 				7E3A6B6A1D3F62CC007C5B1F /* TegKeychainConstants.swift in Sources */,
 				7E3A6B681D3F62CC007C5B1F /* KeychainSwift.swift in Sources */,
@@ -764,6 +769,7 @@
 				2B7E7C4D1B81A3EC00D1C3B9 /* KeychainSwiftPrefixedTests.swift in Sources */,
 				7ED6C9971B1C12B300FE8090 /* KeychainSwiftAccessOptions.swift in Sources */,
 				7E282DD71CED212500B55133 /* SynchronizableTests.swift in Sources */,
+				C7E1DE4C1E4B7C9F003818F6 /* ConcurrencyTests.swift in Sources */,
 				7ED6C9961B1C12B100FE8090 /* KeychainSwift.swift in Sources */,
 				7E87495F1BDB83D00045CF75 /* AccessGroupTests.swift in Sources */,
 				7E3A6B6D1D3F6431007C5B1F /* ClearTests.swift in Sources */,

+ 4 - 0
Sources/KeychainSwift.swift

@@ -32,6 +32,8 @@ open class KeychainSwift {
    
   */
   open var synchronizable: Bool = false
+
+  private let readLock = NSLock()
   
   /// Instantiate a KeychainSwift object
   public init() { }
@@ -155,6 +157,8 @@ open class KeychainSwift {
   
   */
   open func getData(_ key: String) -> Data? {
+    readLock.lock()
+    defer { readLock.unlock() }
     let prefixedKey = keyWithPrefix(key)
     
     var query: [String: Any] = [

+ 121 - 0
Tests/ConcurrencyTests.swift

@@ -0,0 +1,121 @@
+//
+//  ConcurrencyTests.swift
+//  KeychainSwift
+//
+//  Created by Eli Kohen on 08/02/2017.
+//  Copyright © 2017 Marketplacer. All rights reserved.
+//
+
+import XCTest
+
+class ConcurrencyTests: XCTestCase {
+
+    var obj: KeychainSwift!
+
+    override func setUp() {
+        super.setUp()
+
+        obj = KeychainSwift()
+        obj.clear()
+        obj.lastQueryParameters = nil
+        obj.synchronizable = false
+    }
+
+    // MARK: - addSynchronizableIfRequired
+
+    func testConcurrencyDoesntCrash() {
+
+        let expectation = self.expectation(description: "Wait for write loop")
+
+        let dataToWrite = "{ asdf ñlk BNALSKDJFÑLAKSJDFÑLKJ ZÑCLXKJ ÑALSKDFJÑLKASJDFÑLKJASDÑFLKJAÑSDLKFJÑLKJ}"
+        obj.set(dataToWrite, forKey: "test-key")
+
+        var writes = 0
+
+        let readQueue = DispatchQueue(label: "ReadQueue", attributes: [])
+        readQueue.async {
+            for _ in 0..<400 {
+                let _: String? = synchronize( { completion in
+                    let result: String? = self.obj.get("test-key")
+                    DispatchQueue.global(qos: .background).async {
+                        DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(5)) {
+                            completion(result)
+                        }
+                    }
+                }, timeoutWith: nil)
+            }
+        }
+        let readQueue2 = DispatchQueue(label: "ReadQueue2", attributes: [])
+        readQueue2.async {
+            for _ in 0..<400 {
+                let _: String? = synchronize( { completion in
+                    let result: String? = self.obj.get("test-key")
+                    DispatchQueue.global(qos: .background).async {
+                        DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(5)) {
+                            completion(result)
+                        }
+                    }
+                }, timeoutWith: nil)
+            }
+        }
+        let readQueue3 = DispatchQueue(label: "ReadQueue3", attributes: [])
+        readQueue3.async {
+            for _ in 0..<400 {
+                let _: String? = synchronize( { completion in
+                    let result: String? = self.obj.get("test-key")
+                    DispatchQueue.global(qos: .background).async {
+                        DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(5)) {
+                            completion(result)
+                        }
+                    }
+                }, timeoutWith: nil)
+            }
+        }
+
+        let writeQueue = DispatchQueue(label: "WriteQueue", attributes: [])
+        writeQueue.async {
+            for _ in 0..<500 {
+                let written: Bool = synchronize({ completion in
+                    DispatchQueue.global(qos: .background).async {
+                        DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(5)) {
+                            let result = self.obj.set(dataToWrite, forKey: "test-key")
+                            completion(result)
+                        }
+                    }
+                }, timeoutWith: false)
+                if written {
+                    writes = writes + 1
+                }
+            }
+            expectation.fulfill()
+        }
+
+        for _ in 0..<1000 {
+            self.obj.set(dataToWrite, forKey: "test-key")
+            let _ = self.obj.get("test-key")
+        }
+        self.waitForExpectations(timeout: 20, handler: nil)
+
+        XCTAssertEqual(500, writes)
+    }
+}
+
+
+// Synchronizes a asynch closure
+// Ref: https://forums.developer.apple.com/thread/11519
+func synchronize<ResultType>(_ asynchClosure: (_ completion: @escaping (ResultType) -> ()) -> Void,
+                        timeout: DispatchTime = DispatchTime.distantFuture, timeoutWith: @autoclosure @escaping () -> ResultType) -> ResultType {
+    let sem = DispatchSemaphore(value: 0)
+
+    var result: ResultType?
+
+    asynchClosure { (r: ResultType) -> () in
+        result = r
+        sem.signal()
+    }
+    _ = sem.wait(timeout: timeout)
+    if result == nil {
+        result = timeoutWith()
+    }
+    return result!
+}