From d6b614b8bef61c08091cb240cdf7dcce6b3eee20 Mon Sep 17 00:00:00 2001 From: Rocky Date: Tue, 20 Oct 2026 23:00:12 +0200 Subject: [PATCH] refactor(concurrency): isolate ProviderConfig to the main actor and drop unneeded @unchecked Sendable - ProviderConfig is now @MainActor instead of @unchecked Sendable; AppL10n reads the persisted UI language key directly so off-main string lookups stay valid. - EnglishLearningStore serializes its read-modify-write with a type-wide lock (several instances share one defaults key). - URLSession-only transports, GatewayGrantKeychainStore and Keychain conform to Sendable without @unchecked. UserDefaults/DeviceCheck wrappers keep @unchecked because those SDK types are not Sendable. --- .../Services/PublicContentHTTPTransport.swift | 2 +- OSGKeyboard/Utilities/AppL10n.swift | 11 +++++++++-- OSGKeyboard/Views/SettingsSecondaryPages.swift | 1 + .../Features/Account/AccountAPIClient.swift | 2 +- .../Core/Configuration/LiveConfigurationStore.swift | 2 ++ .../Analytics/AnalyticsUploadCoordinator.swift | 2 +- .../ManagedGateway/GatewayGrantCredentialStore.swift | 2 +- OSGKeyboardShared/Models/ProviderConfig.swift | 8 ++++---- OSGKeyboardShared/Services/Keychain.swift | 2 +- OSGKeyboardShared/Services/LLMClient.swift | 1 + OSGKeyboardShared/Typing/EnglishLearningStore.swift | 12 +++++++++--- OSGKeyboardTests/KeychainTests.swift | 2 ++ OSGKeyboardTests/LLMClientTests.swift | 5 +++++ 13 files changed, 38 insertions(+), 14 deletions(-) diff --git a/OSGKeyboard/Services/PublicContentHTTPTransport.swift b/OSGKeyboard/Services/PublicContentHTTPTransport.swift index b227b55..6642065 100644 --- a/OSGKeyboard/Services/PublicContentHTTPTransport.swift +++ b/OSGKeyboard/Services/PublicContentHTTPTransport.swift @@ -12,7 +12,7 @@ protocol PublicContentHTTPTransport: Sendable { final class URLSessionPublicContentHTTPTransport: PublicContentHTTPTransport, - @unchecked Sendable { + Sendable { private let session: URLSession init(session: URLSession = .shared) { diff --git a/OSGKeyboard/Utilities/AppL10n.swift b/OSGKeyboard/Utilities/AppL10n.swift index ed52270..18fdde4 100644 --- a/OSGKeyboard/Utilities/AppL10n.swift +++ b/OSGKeyboard/Utilities/AppL10n.swift @@ -30,9 +30,16 @@ enum AppL10n { /// screen that exists so a provisioning mistake does *not* become a crash /// loop. `.auto` follows the system language, which is the right answer /// when no stored preference is reachable anyway. + /// + /// Reads the persisted key directly rather than `ProviderConfig.shared` + /// (main-actor isolated) because localized strings are also resolved off + /// the main actor. `ProviderConfig` writes this key synchronously in its + /// `uiLanguage` didSet, so both paths observe the same value. private static var resolvedUILanguage: AppUILanguage { - guard AppGroup.isAvailable else { return .auto } - return ProviderConfig.shared.uiLanguage + guard let defaults = AppGroup.defaultsIfAvailable else { return .auto } + return AppUILanguage.fromStored( + defaults.string(forKey: AppGroupConfiguration.Keys.uiLanguage) + ) } static func format( diff --git a/OSGKeyboard/Views/SettingsSecondaryPages.swift b/OSGKeyboard/Views/SettingsSecondaryPages.swift index cb87eb7..d8880b0 100644 --- a/OSGKeyboard/Views/SettingsSecondaryPages.swift +++ b/OSGKeyboard/Views/SettingsSecondaryPages.swift @@ -91,6 +91,7 @@ struct SettingsVersionRow: View { // MARK: - Config entry summaries (shown on Settings home) +@MainActor enum SettingsConfigSummary { static func speechRecognition(config: ProviderConfig) -> String { if config.engineMode == "local" { diff --git a/OSGKeyboardHostSupport/Features/Account/AccountAPIClient.swift b/OSGKeyboardHostSupport/Features/Account/AccountAPIClient.swift index c6350d3..8827b65 100644 --- a/OSGKeyboardHostSupport/Features/Account/AccountAPIClient.swift +++ b/OSGKeyboardHostSupport/Features/Account/AccountAPIClient.swift @@ -13,7 +13,7 @@ public protocol AccountHTTPTransport: Sendable { func data(for request: URLRequest) async throws -> (Data, HTTPURLResponse) } -public final class URLSessionAccountHTTPTransport: AccountHTTPTransport, @unchecked Sendable { +public final class URLSessionAccountHTTPTransport: AccountHTTPTransport, Sendable { private let session: URLSession public init(session: URLSession = .shared) { diff --git a/OSGKeyboardShared/Core/Configuration/LiveConfigurationStore.swift b/OSGKeyboardShared/Core/Configuration/LiveConfigurationStore.swift index 1c26f53..ba2380d 100644 --- a/OSGKeyboardShared/Core/Configuration/LiveConfigurationStore.swift +++ b/OSGKeyboardShared/Core/Configuration/LiveConfigurationStore.swift @@ -97,6 +97,7 @@ public struct LiveConfigurationSnapshot: @unchecked Sendable { } /// Build from live `ProviderConfig` plus persisted App Group extras. + @MainActor public init(config: ProviderConfig, fallback: AppGroupStore) { self.init( providerId: config.providerId, @@ -136,6 +137,7 @@ public struct LiveConfigurationStore: ConfigurationStore, @unchecked Sendable { self.init(snapshot: LiveConfigurationSnapshot(store: store)) } + @MainActor public init(config: ProviderConfig, fallback: AppGroupStore) { self.init(snapshot: LiveConfigurationSnapshot(config: config, fallback: fallback)) } diff --git a/OSGKeyboardShared/Features/Analytics/AnalyticsUploadCoordinator.swift b/OSGKeyboardShared/Features/Analytics/AnalyticsUploadCoordinator.swift index 86bfd34..45d706d 100644 --- a/OSGKeyboardShared/Features/Analytics/AnalyticsUploadCoordinator.swift +++ b/OSGKeyboardShared/Features/Analytics/AnalyticsUploadCoordinator.swift @@ -6,7 +6,7 @@ import Foundation -public final class URLSessionAnalyticsNetwork: AnalyticsNetworking, @unchecked Sendable { +public final class URLSessionAnalyticsNetwork: AnalyticsNetworking, Sendable { private let session: URLSession public init(session: URLSession = .shared) { diff --git a/OSGKeyboardShared/Features/ManagedGateway/GatewayGrantCredentialStore.swift b/OSGKeyboardShared/Features/ManagedGateway/GatewayGrantCredentialStore.swift index 32f884e..bc27b00 100644 --- a/OSGKeyboardShared/Features/ManagedGateway/GatewayGrantCredentialStore.swift +++ b/OSGKeyboardShared/Features/ManagedGateway/GatewayGrantCredentialStore.swift @@ -13,7 +13,7 @@ public protocol GatewayGrantCredentialStore: Sendable { func delete() async throws } -public struct GatewayGrantKeychainStore: GatewayGrantCredentialStore, @unchecked Sendable { +public struct GatewayGrantKeychainStore: GatewayGrantCredentialStore, Sendable { public enum Slot: String, Sendable { case account = "scope-limited.active" case oobe = "scope-limited.oobe" diff --git a/OSGKeyboardShared/Models/ProviderConfig.swift b/OSGKeyboardShared/Models/ProviderConfig.swift index 9a34aa5..f34ea4f 100644 --- a/OSGKeyboardShared/Models/ProviderConfig.swift +++ b/OSGKeyboardShared/Models/ProviderConfig.swift @@ -12,10 +12,10 @@ import Combine import Foundation -/// UI-owned ObservableObject; construct and mutate it on the main thread. -/// `@unchecked Sendable` does not make `@Published` thread-safe. Credential -/// observers write only Keychain, while non-secret configuration uses App Group defaults. -public final class ProviderConfig: ObservableObject, @unchecked Sendable { +/// UI-owned ObservableObject, isolated to the main actor. Credential observers +/// write only Keychain, while non-secret configuration uses App Group defaults. +@MainActor +public final class ProviderConfig: ObservableObject { public static let shared = ProviderConfig() @Published public var providerId: String { diff --git a/OSGKeyboardShared/Services/Keychain.swift b/OSGKeyboardShared/Services/Keychain.swift index 759baae..d4f956a 100644 --- a/OSGKeyboardShared/Services/Keychain.swift +++ b/OSGKeyboardShared/Services/Keychain.swift @@ -12,7 +12,7 @@ import Foundation import Security -public enum Keychain: @unchecked Sendable { +public enum Keychain: Sendable { public enum KeychainError: Error, Sendable, Equatable { case unexpectedStatus(OSStatus) diff --git a/OSGKeyboardShared/Services/LLMClient.swift b/OSGKeyboardShared/Services/LLMClient.swift index 7010b3c..8a0eba8 100644 --- a/OSGKeyboardShared/Services/LLMClient.swift +++ b/OSGKeyboardShared/Services/LLMClient.swift @@ -418,6 +418,7 @@ public struct OpenAICompatibleClient: LLMClient { public enum LLMClientFactory { /// Build a client from the current `ProviderConfig`. + @MainActor public static func make(from config: ProviderConfig) -> LLMClient { make( providerId: config.providerId, diff --git a/OSGKeyboardShared/Typing/EnglishLearningStore.swift b/OSGKeyboardShared/Typing/EnglishLearningStore.swift index 1ef2899..655ca0a 100644 --- a/OSGKeyboardShared/Typing/EnglishLearningStore.swift +++ b/OSGKeyboardShared/Typing/EnglishLearningStore.swift @@ -10,6 +10,10 @@ import Foundation public final class EnglishLearningStore: @unchecked Sendable { public static let defaultsKey = "englishTyping.learnedBoosts.v1" + /// Static because several instances share the same defaults key; a + /// per-instance lock would not serialize their read-modify-write. + private static let mutationLock = NSLock() + private let defaults: UserDefaults public init(defaults: UserDefaults = AppGroupStore().defaults) { @@ -42,8 +46,10 @@ public final class EnglishLearningStore: @unchecked Sendable { private func mutate(word: String, delta: Int) { let key = word.lowercased() guard !key.isEmpty else { return } - var map = snapshot() - map[key] = min(10_000, (map[key] ?? 0) + delta) - defaults.set(map, forKey: Self.defaultsKey) + Self.mutationLock.withLock { + var map = snapshot() + map[key] = min(10_000, (map[key] ?? 0) + delta) + defaults.set(map, forKey: Self.defaultsKey) + } } } diff --git a/OSGKeyboardTests/KeychainTests.swift b/OSGKeyboardTests/KeychainTests.swift index d6cb7da..5f525a9 100644 --- a/OSGKeyboardTests/KeychainTests.swift +++ b/OSGKeyboardTests/KeychainTests.swift @@ -235,6 +235,7 @@ final class KeychainTests: XCTestCase { /// value must be moved to the Keychain and removed from UserDefaults /// — otherwise a fresh install in a clean simulator would inherit the /// stale plaintext value. + @MainActor func testLegacyUserDefaultsKeyIsMigratedToKeychain() { let suiteName = "group.com.osgkeyboard.shared.tests.migration.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suiteName)! @@ -271,6 +272,7 @@ final class KeychainTests: XCTestCase { /// — possible on a downgrade or a torn update — Keychain wins. We /// don't delete the UserDefaults value, but the running app reads from /// the Keychain only. + @MainActor func testKeychainWinsOverLegacyWhenBothPresent() { let suiteName = "group.com.osgkeyboard.shared.tests.migration2.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suiteName)! diff --git a/OSGKeyboardTests/LLMClientTests.swift b/OSGKeyboardTests/LLMClientTests.swift index 7a4b9ab..a6349e2 100644 --- a/OSGKeyboardTests/LLMClientTests.swift +++ b/OSGKeyboardTests/LLMClientTests.swift @@ -45,6 +45,7 @@ final class LLMClientTests: XCTestCase { // MARK: - ProviderConfig persistence + @MainActor func testProviderConfigPersistsAcrossInstances() { let suiteName = "group.com.osgkeyboard.shared.tests.persist.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suiteName)! @@ -64,6 +65,7 @@ final class LLMClientTests: XCTestCase { } /// Local ASR needs no cloud ASR key, but polish still requires a user API key. + @MainActor func testLocalEngineWithoutAPIKeyIsNotPolishConfigured() { let suiteName = "group.com.osgkeyboard.shared.tests.isconfigured.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suiteName)! @@ -299,6 +301,7 @@ final class LLMClientTests: XCTestCase { /// Cross-process App Group contract: what `ProviderConfig` writes must /// be readable through `AppGroupStore` on the same suite. + @MainActor func testAppGroupCrossProcessLegacyOffModeMigratesToPolish() async { let suiteName = "group.com.osgkeyboard.shared.tests.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suiteName)! @@ -377,6 +380,7 @@ final class LLMClientTests: XCTestCase { ) } + @MainActor func testLiveConfigurationStoreSnapshotsAnyConfigurationStore() throws { let suiteName = "group.com.osgkeyboard.shared.tests.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suiteName)! @@ -561,6 +565,7 @@ final class LLMClientTests: XCTestCase { XCTAssertNil(body["thinking_config"]) } + @MainActor func testLLMThinkingDefaultIsOffInFreshConfiguration() { let suiteName = "group.com.osgkeyboard.shared.tests.thinking.\(UUID().uuidString)" let defaults = UserDefaults(suiteName: suiteName)!