How can I achieve thread safety in an async method?-CodePudding

I am trying to read the keychain but the documentation warns me about the following:

SecItemCopyMatching blocks the calling thread, so it can cause your app’s UI to hang if called from the main thread. Instead, call SecItemCopyMatching from a background dispatch queue or async function.

Source

So I want to write an asynchronous method that runs in the background.

actor Keychain {
    public static let standard = Keychain()
    
    public enum Error: Swift.Error {
        case failed(String)
    }
    
    public func get(_ key: String) async throws -> Data? {
        let backgroundTask = Task(priority: .background) {
            var query: [String: Any] = [
                type(of: self).klass       : kSecClassGenericPassword,
                type(of: self).attrAccount : key,
                type(of: self).matchLimit  : kSecMatchLimitOne,
                type(of: self).returnData  : kCFBooleanTrue as CFBoolean
            ]
            
            var item: CFTypeRef?
            let status = SecItemCopyMatching(query as CFDictionary, &item)
            
            guard status == errSecSuccess else {
                if let errorMessage = SecCopyErrorMessageString(status, nil) {
                    throw Error.failed(String(errorMessage))
                } else {
                    throw Error.failed("unsupported")
                }
            }
            
            return item as? Data
        }
        return try await backgroundTask.value
    }
}

My question is.. will the actor already make it thread safe?

Normally I would add a NSLock to be safe.

public func get(_ key: String) async throws -> Data? {
    lock.lock()
    defer { lock.unlock() }
    
    (...)
    
    return try await task.value
}

However now I get a warning Instance method 'lock' is unavailable from asynchronous contexts; Use async-safe scoped locking instead; this is an error in Swift 6.

So how I am able to achieve this?

CodePudding user response：

Yes, actors will make it thread-safe. No need for locks or the like. See WWDC 2021 video Protect mutable state with Swift actors.

BTW, since you've got this on an actor, you really don't need to make get(_:) an async method. The actor already runs on a background thread. So remove the async qualifier and then remove the Task:

actor Keychain {
    ...
    
    public func get(_ key: String) throws -> Data? {
        var query: [String: Any] = [
            type(of: self).klass       : kSecClassGenericPassword,
            type(of: self).attrAccount : key,
            type(of: self).matchLimit  : kSecMatchLimitOne,
            type(of: self).returnData  : kCFBooleanTrue as CFBoolean
        ]
        
        var item: CFTypeRef?
        let status = SecItemCopyMatching(query as CFDictionary, &item)
        
        guard status == errSecSuccess else {
            if let errorMessage = SecCopyErrorMessageString(status, nil) {
                throw Error.failed(String(errorMessage))
            } else {
                throw Error.failed("unsupported")
            }
        }
        
        return item as? Data
    }
}

CodePudding user response：

It looks like you are using all local variables (no instance or static variables) so you should be fine.

CodePudding user response：

Why do you put Keychain operations in a async block that is not needed. You are good to go without it. Keychain operations are Thread safe and that is guaranteed by Apple. See this documentation. Apple

So do it without the async block and put all ui changes in the main thread (if it is not already). UI changes need to be in the main thread.