Increasing thread safety for authentications.

Attempting to fix multiple crashes associated with the call to persist() triggered by the authentications setter called in restore().
mastodon · Oct 30, 2024 · 20f7d76 · 20f7d76
1 parent a9e088a
commit 20f7d76
Show file tree

Hide file tree

Showing 9 changed files with 35 additions and 22 deletions.
diff --git a/Mastodon/Supporting Files/AppDelegate.swift b/Mastodon/Supporting Files/AppDelegate.swift
@@ -17,7 +17,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
     let appContext = AppContext()
 
     func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
-        AuthenticationServiceProvider.shared.restore()
+        AuthenticationServiceProvider.shared.prepareForUse()
 
         AppSecret.default.register()
 

diff --git a/MastodonIntent/IntentHandler.swift b/MastodonIntent/IntentHandler.swift
@@ -11,7 +11,7 @@ import MastodonCore
 class IntentHandler: INExtension {
 
     override func handler(for intent: INIntent) -> Any {
-        AuthenticationServiceProvider.shared.restore()
+        AuthenticationServiceProvider.shared.prepareForUse()
 
         switch intent {
         case is SendPostIntent:

diff --git a/MastodonSDK/Sources/MastodonCore/AuthenticationServiceProvider.swift b/MastodonSDK/Sources/MastodonCore/AuthenticationServiceProvider.swift
@@ -19,7 +19,7 @@ public class AuthenticationServiceProvider: ObservableObject {
 
     @Published public var authentications: [MastodonAuthentication] = [] {
         didSet {
-            persist() // todo: Is this too heavy and too often here???
+            persist(authentications)
         }
     }
 
@@ -53,6 +53,7 @@ public class AuthenticationServiceProvider: ObservableObject {
         return self
     }
 
+    @MainActor
     func delete(authentication: MastodonAuthentication) throws {
         try Self.keychain.remove(authentication.persistenceIdentifier)
         authentications.removeAll(where: { $0 == authentication })
@@ -81,14 +82,22 @@ public extension AuthenticationServiceProvider {
     func authenticationSortedByActivation() -> [MastodonAuthentication] { // fixme: why do we need this?
         return authentications.sorted(by: { $0.activedAt > $1.activedAt })
     }
+
+    func prepareForUse() {
+        if authentications.isEmpty {
+            restoreFromKeychain()
+        }
+    }
 
-    func restore() {
-        authentications = Self.keychain.allKeys().compactMap {
-            guard
-                let encoded = Self.keychain[$0],
-                let data = Data(base64Encoded: encoded)
-            else { return nil }
-            return try? JSONDecoder().decode(MastodonAuthentication.self, from: data)
+    private func restoreFromKeychain() {
+        DispatchQueue.main.async {
+            self.authentications = Self.keychain.allKeys().compactMap {
+                guard
+                    let encoded = Self.keychain[$0],
+                    let data = Data(base64Encoded: encoded)
+                else { return nil }
+                return try? JSONDecoder().decode(MastodonAuthentication.self, from: data)
+            }
         }
     }
 
@@ -117,8 +126,10 @@ public extension AuthenticationServiceProvider {
                 logger.log(level: .default, "All account authentications were successful.")
             }
 
-            self.authentications = migratedAuthentications
-            userDefaults.didMigrateAuthentications = true
+            DispatchQueue.main.async {
+                self.authentications = migratedAuthentications
+                self.userDefaults.didMigrateAuthentications = true
+            }
         } catch {
             userDefaults.didMigrateAuthentications = false
             logger.log(level: .error, "Could not migrate legacy authentications")
@@ -148,9 +159,11 @@ public extension AuthenticationServiceProvider {
 
 // MARK: - Private
 private extension AuthenticationServiceProvider {
-    func persist() {
-        for authentication in authentications {
-            Self.keychain[authentication.persistenceIdentifier] = try? JSONEncoder().encode(authentication).base64EncodedString()
+    func persist(_ authentications: [MastodonAuthentication]) {
+        DispatchQueue.main.async {
+            for authentication in authentications {
+                Self.keychain[authentication.persistenceIdentifier] = try? JSONEncoder().encode(authentication).base64EncodedString()
+            }
         }
     }
 }
diff --git a/MastodonSDK/Sources/MastodonCore/Service/AuthenticationService.swift b/MastodonSDK/Sources/MastodonCore/Service/AuthenticationService.swift
@@ -137,13 +137,13 @@ extension AuthenticationService {
     }
 
     public func signOutMastodonUser(authentication: MastodonAuthentication) async throws {
-        try AuthenticationServiceProvider.shared.delete(authentication: authentication)
+        try await AuthenticationServiceProvider.shared.delete(authentication: authentication)
         _ = try await apiService?.cancelSubscription(domain: authentication.domain, authorization: authentication.authorization)
     }
 
     public func signOutMastodonUser(authenticationBox: MastodonAuthenticationBox) async throws {
         do {
-            try AuthenticationServiceProvider.shared.delete(authentication: authenticationBox.authentication)
+            try await AuthenticationServiceProvider.shared.delete(authentication: authenticationBox.authentication)
         } catch {
             assertionFailure("Failed to delete Authentication: \(error)")
         }

diff --git a/ShareActionExtension/Scene/ShareViewController.swift b/ShareActionExtension/Scene/ShareViewController.swift
@@ -160,7 +160,7 @@ extension ShareViewController {
 
 extension ShareViewController {
     private func setupAuthContext() throws -> AuthContext? {
-        AuthenticationServiceProvider.shared.restore()
+        AuthenticationServiceProvider.shared.prepareForUse()
 
         let authentication = AuthenticationServiceProvider.shared.authenticationSortedByActivation().first
         let authContext = authentication.flatMap { AuthContext(authentication: $0) }

diff --git a/WidgetExtension/Variants/FollowersCount/FollowersCountWidget.swift b/WidgetExtension/Variants/FollowersCount/FollowersCountWidget.swift
@@ -73,7 +73,7 @@ private extension FollowersCountWidgetProvider {
     func loadCurrentEntry(for configuration: FollowersCountIntent, in context: Context, completion: @escaping (FollowersCountEntry) -> Void) {
         Task {
 
-            AuthenticationServiceProvider.shared.restore()
+            AuthenticationServiceProvider.shared.prepareForUse()
 
             guard
                 let authBox = WidgetExtension.appContext

diff --git a/WidgetExtension/Variants/Hashtag/HashtagWidget.swift b/WidgetExtension/Variants/Hashtag/HashtagWidget.swift
@@ -25,7 +25,7 @@ struct HashtagWidgetProvider: IntentTimelineProvider {
 extension HashtagWidgetProvider {
     func loadMostRecentHashtag(for configuration: HashtagIntent, in context: Context, completion: @escaping (HashtagWidgetTimelineEntry) -> Void ) {
 
-        AuthenticationServiceProvider.shared.restore()
+        AuthenticationServiceProvider.shared.prepareForUse()
 
         guard
             let authBox = WidgetExtension.appContext

diff --git a/WidgetExtension/Variants/LatestFollowers/LatestFollowersWidget.swift b/WidgetExtension/Variants/LatestFollowers/LatestFollowersWidget.swift
@@ -80,7 +80,7 @@ private extension LatestFollowersWidgetProvider {
     func loadCurrentEntry(for configuration: LatestFollowersIntent, in context: Context, completion: @escaping (LatestFollowersEntry) -> Void) {
         Task { @MainActor in
 
-            AuthenticationServiceProvider.shared.restore()
+            AuthenticationServiceProvider.shared.prepareForUse()
 
             guard
                 let authBox = WidgetExtension.appContext

diff --git a/WidgetExtension/Variants/MultiFollowersCount/MultiFollowersCountWidget.swift b/WidgetExtension/Variants/MultiFollowersCount/MultiFollowersCountWidget.swift
@@ -73,7 +73,7 @@ private extension MultiFollowersCountWidgetProvider {
     func loadCurrentEntry(for configuration: MultiFollowersCountIntent, in context: Context, completion: @escaping (MultiFollowersCountEntry) -> Void) {
         Task {
 
-            AuthenticationServiceProvider.shared.restore()
+            AuthenticationServiceProvider.shared.prepareForUse()
 
             guard
                 let authBox = WidgetExtension.appContext