Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fixed threading #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fixed threading #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
108 changes: 54 additions & 54 deletions Sources/KeychainSwift.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,23 @@ A collection of helper functions for saving text and data in the keychain.
*/
open class KeychainSwift {

var lastQueryParameters: [String: Any]? // Used by the unit tests

private var lastQueryParametersAtomic: Atomic<[String: Any]?> = Atomic([:])// Used by the unit tests
var lastQueryParameters:[String: Any]? {
get {
return lastQueryParametersAtomic.value
}
set {
lastQueryParametersAtomic.mutate {$0 = newValue}
}
}

/// Contains result code from the last operation. Value is noErr (0) for a successful result.
open var lastResultCode: OSStatus = noErr
private let lastResultCodeAtomic: Atomic<OSStatus> = Atomic<OSStatus>(noErr)
open var lastResultCode: OSStatus {
get {
return lastResultCodeAtomic.value
}
}

var keyPrefix = "" // Can be useful in test.

Expand All @@ -32,9 +45,6 @@ open class KeychainSwift {

*/
open var synchronizable: Bool = false

private let lock = NSLock()


/// Instantiate a KeychainSwift object
public init() { }
Expand Down Expand Up @@ -85,12 +95,7 @@ open class KeychainSwift {
open func set(_ value: Data, forKey key: String,
withAccess access: KeychainSwiftAccessOptions? = nil) -> Bool {

// The lock prevents the code to be run simlultaneously
// from multiple threads which may result in crashing
lock.lock()
defer { lock.unlock() }

deleteNoLock(key) // Delete any existing key before saving it
delete(key) // Delete any existing key before saving it

let accessible = access?.value ?? KeychainSwiftAccessOptions.defaultOption.value

Expand All @@ -105,11 +110,12 @@ open class KeychainSwift {

query = addAccessGroupWhenPresent(query)
query = addSynchronizableIfRequired(query, addingItems: true)
lastQueryParameters = query
lastQueryParametersAtomic.mutate {$0 = query}

lastResultCode = SecItemAdd(query as CFDictionary, nil)
let res = SecItemAdd(query as CFDictionary, nil)
lastResultCodeAtomic.mutate {$0 = res}

return lastResultCode == noErr
return res == noErr
}

/**
Expand Down Expand Up @@ -148,7 +154,7 @@ open class KeychainSwift {
return currentString
}

lastResultCode = -67853 // errSecInvalidEncoding
lastResultCodeAtomic.mutate {$0 = -67853} // errSecInvalidEncoding
}

return nil
Expand All @@ -164,10 +170,6 @@ open class KeychainSwift {

*/
open func getData(_ key: String, asReference: Bool = false) -> Data? {
// The lock prevents the code to be run simlultaneously
// from multiple threads which may result in crashing
lock.lock()
defer { lock.unlock() }

let prefixedKey = keyWithPrefix(key)

Expand All @@ -185,15 +187,15 @@ open class KeychainSwift {

query = addAccessGroupWhenPresent(query)
query = addSynchronizableIfRequired(query, addingItems: false)
lastQueryParameters = query
lastQueryParametersAtomic.mutate {$0 = query}

var result: AnyObject?

lastResultCode = withUnsafeMutablePointer(to: &result) {
SecItemCopyMatching(query as CFDictionary, UnsafeMutablePointer($0))
let res = withUnsafeMutablePointer(to: &result) {
SecItemCopyMatching(query as CFDictionary, UnsafeMutablePointer($0))
}
lastResultCodeAtomic.mutate {$0 = res }

if lastResultCode == noErr {
if res == noErr {
return result as? Data
}

Expand Down Expand Up @@ -224,24 +226,6 @@ open class KeychainSwift {
*/
@discardableResult
open func delete(_ key: String) -> Bool {
// The lock prevents the code to be run simlultaneously
// from multiple threads which may result in crashing
lock.lock()
defer { lock.unlock() }

return deleteNoLock(key)
}

/**

Same as `delete` but is only accessed internally, since it is not thread safe.

- parameter key: The key that is used to delete the keychain item.
- returns: True if the item was successfully deleted.

*/
@discardableResult
func deleteNoLock(_ key: String) -> Bool {
let prefixedKey = keyWithPrefix(key)

var query: [String: Any] = [
Expand All @@ -251,11 +235,12 @@ open class KeychainSwift {

query = addAccessGroupWhenPresent(query)
query = addSynchronizableIfRequired(query, addingItems: false)
lastQueryParameters = query
lastQueryParametersAtomic.mutate {$0 = query}

lastResultCode = SecItemDelete(query as CFDictionary)
let res = SecItemDelete(query as CFDictionary)
lastResultCodeAtomic.mutate {$0 = res}

return lastResultCode == noErr
return res == noErr
}

/**
Expand All @@ -267,19 +252,14 @@ open class KeychainSwift {
*/
@discardableResult
open func clear() -> Bool {
// The lock prevents the code to be run simlultaneously
// from multiple threads which may result in crashing
lock.lock()
defer { lock.unlock() }

var query: [String: Any] = [ kSecClass as String : kSecClassGenericPassword ]
query = addAccessGroupWhenPresent(query)
query = addSynchronizableIfRequired(query, addingItems: false)
lastQueryParameters = query
lastQueryParametersAtomic.mutate {$0 = query}

lastResultCode = SecItemDelete(query as CFDictionary)
lastResultCodeAtomic.mutate {$0 = SecItemDelete(query as CFDictionary)}

return lastResultCode == noErr
return lastResultCodeAtomic.value == noErr
}

/// Returns the key with currently set prefix.
Expand Down Expand Up @@ -312,3 +292,23 @@ open class KeychainSwift {
return result
}
}

final class Atomic<A> {
private let queue = DispatchQueue(label: "Atomic serial queue")
private var _value: A
init(_ value: A) {
self._value = value
}

var value: A {
get {
return queue.sync { self._value }
}
}

func mutate(_ transform: (inout A) -> ()) {
queue.sync {
transform(&self._value)
}
}
}
17 changes: 9 additions & 8 deletions Tests/KeychainSwiftTests/ConcurrencyTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,35 +133,35 @@ class ConcurrencyTests: XCTestCase {
let writeQueue = DispatchQueue(label: "WriteQueue", attributes: [])
writeQueue.async {
for _ in 0..<500 {
let written: Bool = synchronize({ completion in
synchronize({ completion in
DispatchQueue.global(qos: .background).async {
DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(5)) {
let result = self.obj.set(dataToWrite, forKey: "test-key")
if result {
writes += 1
}
completion(result)
}
}
}, timeoutWith: false)
if written {
writes = writes + 1
}
}
expectation.fulfill()
}

let writeQueue2 = DispatchQueue(label: "WriteQueue2", attributes: [])
writeQueue2.async {
for _ in 0..<500 {
let written: Bool = synchronize({ completion in
synchronize({ completion in
DispatchQueue.global(qos: .background).async {
DispatchQueue.main.asyncAfter(deadline: .now() + .milliseconds(5)) {
let result = self.obj.set(dataToWrite, forKey: "test-key")
if result {
writes += 1
}
completion(result)
}
}
}, timeoutWith: false)
if written {
writes = writes + 1
}
}
expectation2.fulfill()
}
Expand All @@ -179,6 +179,7 @@ class ConcurrencyTests: XCTestCase {

// Synchronizes a asynch closure
// Ref: https://forums.developer.apple.com/thread/11519
@discardableResult
func synchronize<ResultType>(_ asynchClosure: (_ completion: @escaping (ResultType) -> ()) -> Void,
timeout: DispatchTime = DispatchTime.distantFuture, timeoutWith: @autoclosure @escaping () -> ResultType) -> ResultType {
let sem = DispatchSemaphore(value: 0)
Expand Down