From 528db7d9c50d2e4dde9becc320b1762cdeb00993 Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Fri, 21 Nov 2025 06:21:04 +1100 Subject: [PATCH] fix(apple): Prevent Swift6 crash on iPadOS (#10916) UserDefaults change notifications should always be handled on the main thread. This wasn't the case when PencilKit posted UserDefaults notifications from a background thread during its initialization on iPadOS, causing a Swift 6 MainActor violation crash. Ultimately, the root cause of this issue was not abiding to strict Swift6 concurrency checks by using unsafe code: even when the UserDefaults themselves were `nonisolated(unsafe)` and bypassed the checks, it was not the case for Apple PencilKit framework ultimately initialised in wrong context. Note: There are a few ways to fix, we're settling on Combine pattern as it's used elsewhere in the codebase (e.g. in Store). For other solutions, see: https://stackoverflow.com/questions/74729010/swift-concurrency-notification-callbacks-on-mainactor-objects --- .../FirezoneKit/Models/Configuration.swift | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift index b5f429022..6a8773445 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift @@ -5,6 +5,7 @@ // // A thin wrapper around UserDefaults for user and admin managed app configuration. +import Combine import Foundation import Sentry @@ -15,6 +16,7 @@ import Sentry @MainActor public class Configuration: ObservableObject { static let shared = Configuration() + private var cancellables = Set() @Published private(set) var publishedInternetResourceEnabled = false @Published private(set) var publishedHideAdminPortalMenuItem = false @@ -115,11 +117,7 @@ public class Configuration: ObservableObject { static let supportURL = "supportURL" } - // nonisolated(unsafe) is required because: - // 1. UserDefaults is thread-safe - // 2. Used only for NotificationCenter observer registration (in init/deinit) - // 3. deinit is nonisolated and needs access to remove the observer - private nonisolated(unsafe) var defaults: UserDefaults + private var defaults: UserDefaults init(userDefaults: UserDefaults = UserDefaults.standard) { defaults = userDefaults @@ -128,17 +126,12 @@ public class Configuration: ObservableObject { self.publishedHideAdminPortalMenuItem = hideAdminPortalMenuItem self.publishedHideResourceList = hideResourceList - NotificationCenter.default.addObserver( - self, - selector: #selector(handleUserDefaultsChanged), - name: UserDefaults.didChangeNotification, - object: defaults - ) - } - - deinit { - NotificationCenter.default.removeObserver( - self, name: UserDefaults.didChangeNotification, object: defaults) + NotificationCenter.default.publisher(for: UserDefaults.didChangeNotification, object: defaults) + .receive(on: DispatchQueue.main) + .sink { [weak self] _ in + self?.handleUserDefaultsChanged() + } + .store(in: &cancellables) } func toTunnelConfiguration() -> TunnelConfiguration { @@ -172,7 +165,7 @@ public class Configuration: ObservableObject { } #endif - @objc private func handleUserDefaultsChanged(_ notification: Notification) { + private func handleUserDefaultsChanged() { #if os(macOS) // This is idempotent Task { do { try await updateAppService() } }