(Some folks) await async support

[pwesten]

👋🏻😁🙏🏻

[efirestone]

Additional context: Apple documentation explicitly says:

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

and now that we have async/await functionality, it feels like a great time/opportunity to make this properly asynchronous.

[dfed]

We should absolutely mark Valet objects as @unchecked Sendable since they are already thread-safe.

We could also make a breaking change to make Valet objects actors rather than final classes: that would be the correct model for a type that is both thread-safe and shouldn't operate on the main queue. However, enabling values to be read synchronously is valuable to consumers: even though doing keychain operations on main queue is not a good practice, I wouldn't want to prevent a consumer from being able to read keychain values from main if they so desire. My 2c is that the most flexible way forward is to conform to @unchecked Sendable and let consumers write their own actor wrapper around Valet to ensure that they don't synchronously read from keychain on the main queue.

Longer-term, in our next breaking API change (which we're overdue for – we're still supporting Xcode 11), it may make sense to:

1. Remove the locks within Valet types and remove @unchecked Sendable, effectively making Valet types unsafe for use across multiple threads.
2. Expose Valet initializers to consumers and remove the static methods that always return the same instance of a Valet object.
3. Create ValetActor (would love a better name here) types that are of type actor and wrap the final class Valet types.

This approach would create consumer flexibility, remove our use of locks (which I'm honestly not sure we need today – I never read the open-source Security framework's SecItem calls to determine if they were already thread-safe), and create async APIs for folk who are optimizing for main queue performance.

Open to suggestions here. I'm also open to re-assigning ownership of this task – I haven't worked for Square since 2016 and am trying to focus my open source time on projects hosted on my own page.

[dfed]

After chewing on this a little and deploying Swift Concurrency heavily in a few apps and projects, I'm thinking that:

• It is important for Valet objects to enable retrieving data synchronously, even if doing so from the main queue is potentially harmful.
• Our aggressive, static locking is not needed, but we should keep each Valet thread-correct by utilizing an instance-scoped lock under the hood.
• Valet is simple enough to wrap – and honestly should be wrapped by consumers since we don't vend a mockable Valet type – that it shouldn't be Valet's job to provide an actor API.

The more I think about this, the more that taking on this API change would lead to an increase in API surface and README length without providing much that consumers of this library couldn't very quickly write themselves in a way that works better for them. Thoughts?

[dfed]

Now that we support Swift Concurrency with #308, I am tempted to call this issue addressed. Will leave this open for a bit in case folk want to make a case for an alternative approach.

[dfed]

Closing as completed. While we don't currently have an async API, we support being used in async APIs now which is pretty darned close.