CredentialWindowProvider not executed on Main Thread

[martindufort]

While trying to authenticate using the default CredentialWindowProvider, XCode complains that access to keyWindow... is not performed on the Main Thread.

Here's a snapshot...

[mattmassicotte]

Ahh, yes, this is my mistake. I was working on a few different techniques for fixing some concurrency issues. I have not yet come up with a perfect solution, but 2ef4626cc0356ad34d0fe194ef7cfe4b6a2b16b4 should at least prevent this from happening.

[martindufort]

This is working properly. Can be closed off....

[mattmassicotte]

Unfortunately, only sort of. Strict concurrency is proving difficult to implement without either isolating the Authenticator object to the main actor or requiring Sendable everywhere. And both of these options come with major drawbacks. I'm going to keep this open while I investigate more solutions.

[tonyarnold]

Given all that you've learned in the last few months, do you have a better handle on the "right" answer to this problem @mattmassicotte? I would have (naively) assumed that conforming types to Sendable is the recommended approach?

[mattmassicotte]

Thank you for keeping me honest here. I'm gonig to revisit this issue this coming week.

[mattmassicotte]

"this coming week" he said nearly 3 months ago...

[mattmassicotte]

@tonyarnold @martindufort I spent some time on this, and I now have a branch available that should make this work cleanly provided you can build with very recent Swift 6 compiler. Xcode 16 beta 2 is not recent enough, I don't think. But even then, WebAuthenticationSession is problematic. So if you are relying on that API, I don't know if this can fully work yet.

https://github.com/ChimeHQ/OAuthenticator/tree/swift6

[tonyarnold]

Thanks Mattie! I'll check out the branch next week when Xcode 16b3 drops (here's hoping it has a Swift compiler from some time in the last month!).

[mattmassicotte]

No rush at all. Just wanted to make sure you were aware it was now available.

[tonyarnold]

I tried your swift6 branch tonight, and immediately hit on the Authenticator.Configuration type not being sendable, and thus not being able to be used to initialise an instance:

let configuration = Authenticator.Configuration(
  appCredentials: credentials,
  loginStorage: loginStorage,
  tokenHandling: FixedGitHub.OAuthAppTokenHandling(),
  mode: mode,
  authenticationStatusHandler: authenticationStatusHandler
)

let authenticator = Authenticator(config: configuration)

Results in:

Sending 'configuration' risks causing data races
Sending 'self'-isolated 'configuration' to actor-isolated callee risks causing data races between actor-isolated and 'self'-isolated uses

[mattmassicotte]

Well that's weird. Is configuration a local variable? It should be transferrable using region-based isolation. Were you using Xcode 16? Given the wording of those errors, it looks like you were...

Oh wait, I'm sure it is how the configuration property was being set up - it's referencing properties of self preventing the transfer...

[tonyarnold]

Yep, Xcode 16.0 beta 3 on macOS Sonoma.

Here's the full snippet, including the init that houses the call:

public actor GitHubClient {
    public init(
        clientIdentifier: String,
        clientSecret: String,
        scopes: [String],
        callbackURL: URL,
        loginStorage: LoginStorage?,
        mode: Authenticator.UserAuthenticationMode = .manualOnly,
        authenticationStatusHandler: Authenticator.AuthenticationStatusHandler? = nil
    ) async throws {
        let credentials = AppCredentials(
            clientId: clientIdentifier,
            clientPassword: clientSecret,
            scopes: scopes,
            callbackURL: callbackURL
        )

        let configuration = Authenticator.Configuration(
            appCredentials: credentials,
            loginStorage: loginStorage,
            tokenHandling: FixedGitHub.OAuthAppTokenHandling(),
            mode: mode,
            authenticationStatusHandler: authenticationStatusHandler
        )

        let authenticator = Authenticator(config: configuration)
        self.authenticator = authenticator

        let transport = await FoundationClientTransport(
            responseProvider: authenticator.responseProvider
        )
        self.client = try Client(
            serverURL: Servers.server1(),
            configuration: .init(dateTranscoder: .rfc3339),
            transport: transport
        )
    }
}

[mattmassicotte]

These things are hard to eye-ball, but I think that loginStorage or authenticationStatusHandler are the problem. I might be able to using sending in a few places to push this error up.

Making Authenticator.Configuration Sendable is easy fix on my end, but could make it much harder to use...

[tonyarnold]

And it's probably not the only issue: you're right that both of those other properties are likely to blame for this as well.

I can see why you avoided this issue for a bit 😅

[mattmassicotte]

I've decided to try adding some more sendable requirements here. I may not be too bad, because if you are using it with actors (including MainActor stuff) that'll be fine.