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.

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

Annotate thread safe types with NS_SWIFT_SENDABLE #876

Open
LarsPetersHH opened this issue Sep 4, 2024 · 1 comment
Open

Annotate thread safe types with NS_SWIFT_SENDABLE #876

LarsPetersHH opened this issue Sep 4, 2024 · 1 comment
Labels
enhancement triage Issues that need to be triaged

Comments

@LarsPetersHH
Copy link

Problem
I ran into a problem migrating code to Swift 6., because the compiler cannot be certain that OIDServiceConfiguration is used in a thread safe manner.
Consider the following function:

func discoverConfiguration(discoveryUrl: URL) async throws -> OIDServiceConfiguration {
    try await withCheckedThrowingContinuation { continuation in
        OIDAuthorizationService.discoverConfiguration(forDiscoveryURL: discoveryUrl) { config, _ in
            if let config {
                continuation.resume(returning: config) // Error: Sending 'config' risks causing data races
                // Task-isolated 'config' is passed as a 'sending' parameter; Uses in callee may race with later task-isolated uses
            } else {
                // ...
            }
        }
    }
}

Because OIDServiceConfiguration is returned via callback closure, the compiler has to assume that it may be used by the sending function (+[OIDAuthorizationService discoverConfiguration]) even after the closure is called. Because OIDServiceConfiguration is not declared Sendable this results in a compile error (see inline comments).

Solution
Annotate OIDServiceConfiguration with NS_SWIFT_SENDABLE.

This makes the assumption that OIDServiceConfiguration is in fact thread safe. Looking at the code, I see only one point which might make the class not thread safe: OIDServiceConfiguration holds a reference to OIDServiceDiscovery which in turn holds a reference to NSDictionary (NSDictionary *_discoveryDictionary) which may contain non thread safe objects. But in real life those would only be PODs (String, Int) which are thread safe, correct?

Ideally, all thread safe types in AppAuth would be annotated with NS_SWIFT_SENDABLE.

Workaround
Wrap the OIDServiceConfiguration object into an @unchecked Sendable object to send it to the continuation. This disables the compiler checks.

Additional Information
No compile error with Swift 5 – not even with complete concurrency checks enabled.

@LarsPetersHH LarsPetersHH added enhancement triage Issues that need to be triaged labels Sep 4, 2024
@Carrione
Copy link

Carrione commented Nov 8, 2024

🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement triage Issues that need to be triaged
Projects
None yet
Development

No branches or pull requests

2 participants