From bf95dc45a307bb85329328e4382f49626335bfce Mon Sep 17 00:00:00 2001 From: Mariusz Klochowicz Date: Wed, 5 Nov 2025 14:54:49 +1030 Subject: [PATCH] refactor(apple): Upgrade to Swift 6.2 with concurrency checks (#10682) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR upgrades the Swift client from Swift 5 to Swift 6.2, addressing all concurrency-related warnings and runtime crashes that come with Swift 6's strict concurrency checking. ## Swift 6 Concurrency Primer **`actor`** - A new reference type that provides thread-safe, serialised access to mutable state. Unlike classes, actors ensure that only one piece of code can access their mutable properties at a time. Access to actor methods/properties requires await and automatically hops to the actor's isolated executor. **`@MainActor`** - An attribute that marks code to run on the main thread. Essential for UI updates and anything that touches UIKit/AppKit. When a class/function is marked @MainActor, all its methods and properties inherit this isolation. **`@Sendable`** - A protocol indicating that a type can be safely passed across concurrency domains (between actors, tasks, etc.). Value types (structs, enums) with Sendable stored properties are automatically Sendable. Reference types (classes) need explicit @unchecked Sendable if they manage thread-safety manually. **`nonisolated`** - Opts out of the containing type's actor isolation. For example, a nonisolated method in a @MainActor class can be called from any thread without await. Useful for static methods or thread-safe operations. **`@concurrent`** - Used on closure parameters in delegate methods. Indicates the closure may be called from any thread, preventing the closure from inheriting the surrounding context's actor isolation. Critical for callbacks from system frameworks that call from background threads. **Data Races** - Swift 6 enforces at compile-time (and optionally at runtime) that mutable state cannot be accessed concurrently from multiple threads. This eliminates entire classes of bugs that were previously only caught through testing or production crashes. ## Swift Language Upgrade - **Bump Swift 5 → 6.2**: Enabled strict concurrency checking throughout the codebase - **Enable ExistentialAny (SE-0335)**: Adds compile-time safety by making protocol type erasure explicit (e.g., any Protocol instead of implicit Protocol) - **Runtime safety configuration**: Added environment variables to log concurrency violations during development instead of crashing, allowing gradual migration ## Concurrency Fixes ### Actor Isolation - **TelemetryState actor** (Telemetry.swift:10): Extracted mutable telemetry state into a dedicated actor to eliminate data races from concurrent access - **SessionNotification @MainActor isolation** (SessionNotification.swift:25): Properly isolated the class to MainActor since it manages UI-related callbacks - **IPCClient caching** (IPCClient.swift): Fixed actor re-entrance issues and resource hash-based optimisation by caching the client instance in Store ### Thread-Safe Callbacks - **WebAuthSession @concurrent delegate** (WebAuthSession.swift:46): The authentication callback is invoked from a background thread by ASWebAuthenticationSession. Marked the wrapper function as @concurrent to prevent MainActor inference on the completion handler closure, then explicitly hopped back to MainActor for the session.start() call. This fixes EXC_BAD_INSTRUCTION crashes at _dispatch_assert_queue_fail. - **SessionNotification @concurrent delegate** (SessionNotification.swift:131): Similarly marked the notification delegate method as @concurrent and used Task { @MainActor in } to safely invoke the MainActor-isolated signInHandler ### Sendable Conformances - Added Sendable to Resource, Site, Token, Configuration, and other model types that are passed between actors and tasks - **LogWriter immutability** (Log.swift): Made jsonData immutable to prevent capturing mutable variables in @Sendable closures ### Nonisolated Methods - **Static notification display** (SessionNotification.swift:73): Marked showSignedOutNotificationiOS() as nonisolated since it's called from the Network Extension (different process) and only uses thread-safe APIs Fixes #10674 Fixes #10675 --- .../apple/Firezone.xcodeproj/project.pbxproj | 16 ++-- swift/apple/Firezone/Info.plist | 7 ++ swift/apple/FirezoneKit/Package.swift | 2 +- .../FirezoneKit/Helpers/DeviceMetadata.swift | 3 +- .../FirezoneKit/Helpers/IPCClient.swift | 81 ++++++++++--------- .../Sources/FirezoneKit/Helpers/Log.swift | 37 +++++---- .../FirezoneKit/Helpers/LogExporter.swift | 4 +- .../FirezoneKit/Helpers/Telemetry.swift | 72 +++++++++-------- .../Managers/VPNConfigurationManager.swift | 4 +- .../FirezoneKit/Models/Configuration.swift | 8 +- .../Sources/FirezoneKit/Models/Resource.swift | 12 +-- .../Models/SessionNotification.swift | 13 +-- .../Sources/FirezoneKit/Models/Site.swift | 2 +- .../Sources/FirezoneKit/Models/Token.swift | 4 +- .../FirezoneKit/Models/WebAuthSession.swift | 46 ++++++++--- .../Sources/FirezoneKit/Stores/Store.swift | 21 ++++- .../Views/UpdateNotification.swift | 6 +- .../FirezoneNetworkExtension/Adapter.swift | 74 ++++++++++++++--- .../BindResolvers.swift | 10 ++- .../FirezoneNetworkExtension/Info.iOS.plist | 7 ++ .../FirezoneNetworkExtension/Info.macOS.plist | 7 ++ .../NetworkSettings.swift | 2 +- .../PacketTunnelProvider.swift | 26 +++--- 23 files changed, 308 insertions(+), 156 deletions(-) diff --git a/swift/apple/Firezone.xcodeproj/project.pbxproj b/swift/apple/Firezone.xcodeproj/project.pbxproj index e632b80ee..587b8b827 100644 --- a/swift/apple/Firezone.xcodeproj/project.pbxproj +++ b/swift/apple/Firezone.xcodeproj/project.pbxproj @@ -595,7 +595,7 @@ SWIFT_OBJC_BRIDGING_HEADER = "FirezoneNetworkExtension/FirezoneNetworkExtension-Bridging-Header.h"; SWIFT_OBJC_INTERFACE_HEADER_NAME = ""; SWIFT_OPTIMIZATION_LEVEL = "-Onone"; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; TARGETED_DEVICE_FAMILY = "1,2"; TVOS_DEPLOYMENT_TARGET = ""; WATCHOS_DEPLOYMENT_TARGET = ""; @@ -637,7 +637,7 @@ SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_OBJC_BRIDGING_HEADER = "FirezoneNetworkExtension/FirezoneNetworkExtension-Bridging-Header.h"; SWIFT_OBJC_INTERFACE_HEADER_NAME = ""; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; TARGETED_DEVICE_FAMILY = "1,2"; TVOS_DEPLOYMENT_TARGET = ""; VALIDATE_PRODUCT = YES; @@ -681,7 +681,7 @@ SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_INCLUDE_PATHS = "$(PROJECT_DIR)/FirezoneNetworkExtension/Connlib/Generated"; SWIFT_OBJC_BRIDGING_HEADER = "FirezoneNetworkExtension/FirezoneNetworkExtension-Bridging-Header.h"; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; }; name = Debug; }; @@ -723,7 +723,7 @@ SWIFT_EMIT_LOC_STRINGS = YES; SWIFT_INCLUDE_PATHS = "$(PROJECT_DIR)/FirezoneNetworkExtension/Connlib/Generated"; SWIFT_OBJC_BRIDGING_HEADER = "FirezoneNetworkExtension/FirezoneNetworkExtension-Bridging-Header.h"; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; }; name = Release; }; @@ -789,9 +789,11 @@ MTL_ENABLE_DEBUG_INFO = INCLUDE_SOURCE; MTL_FAST_MATH = YES; ONLY_ACTIVE_ARCH = YES; + OTHER_SWIFT_FLAGS = "-enable-upcoming-feature ExistentialAny"; SUPPORTED_PLATFORMS = "macosx iphoneos"; SWIFT_ACTIVE_COMPILATION_CONDITIONS = DEBUG; SWIFT_OPTIMIZATION_LEVEL = "-Onone"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_TREAT_WARNINGS_AS_ERRORS = YES; }; name = Debug; @@ -851,9 +853,11 @@ MACOSX_DEPLOYMENT_TARGET = 12.4; MTL_ENABLE_DEBUG_INFO = NO; MTL_FAST_MATH = YES; + OTHER_SWIFT_FLAGS = "-enable-upcoming-feature ExistentialAny"; SUPPORTED_PLATFORMS = "macosx iphoneos"; SWIFT_COMPILATION_MODE = wholemodule; SWIFT_OPTIMIZATION_LEVEL = "-O"; + SWIFT_STRICT_CONCURRENCY = complete; SWIFT_TREAT_WARNINGS_AS_ERRORS = YES; }; name = Release; @@ -899,7 +903,7 @@ SWIFT_INSTALL_OBJC_HEADER = NO; SWIFT_OBJC_INTERFACE_HEADER_NAME = ""; SWIFT_OPTIMIZATION_LEVEL = "-Onone"; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; TARGETED_DEVICE_FAMILY = "1,2"; TVOS_DEPLOYMENT_TARGET = ""; WATCHOS_DEPLOYMENT_TARGET = ""; @@ -948,7 +952,7 @@ SWIFT_INSTALL_MODULE = NO; SWIFT_INSTALL_OBJC_HEADER = NO; SWIFT_OBJC_INTERFACE_HEADER_NAME = ""; - SWIFT_VERSION = 5.0; + SWIFT_VERSION = 6.2; TARGETED_DEVICE_FAMILY = "1,2"; TVOS_DEPLOYMENT_TARGET = ""; WATCHOS_DEPLOYMENT_TARGET = ""; diff --git a/swift/apple/Firezone/Info.plist b/swift/apple/Firezone/Info.plist index 3bee9dcf5..f5dca407c 100644 --- a/swift/apple/Firezone/Info.plist +++ b/swift/apple/Firezone/Info.plist @@ -47,5 +47,12 @@ GitSha $(GIT_SHA) + LSEnvironment + + SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL + 1 + SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE + nocrash + diff --git a/swift/apple/FirezoneKit/Package.swift b/swift/apple/FirezoneKit/Package.swift index 3716696b0..39ff025d1 100644 --- a/swift/apple/FirezoneKit/Package.swift +++ b/swift/apple/FirezoneKit/Package.swift @@ -1,4 +1,4 @@ -// swift-tools-version: 5.7 +// swift-tools-version: 6.0 // The swift-tools-version declares the minimum version of Swift required to build this package. import PackageDescription diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/DeviceMetadata.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/DeviceMetadata.swift index ba22f3c46..f2a9c99ff 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/DeviceMetadata.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/DeviceMetadata.swift @@ -12,13 +12,13 @@ import Foundation #endif public class DeviceMetadata { + @MainActor public static func getDeviceName() -> String { // Returns a generic device name on iOS 16 and higher // See https://github.com/firezone/firezone/issues/3034 #if os(iOS) return UIDevice.current.name #else - // Use hostname return ProcessInfo.processInfo.hostName #endif } @@ -33,6 +33,7 @@ public class DeviceMetadata { } #if os(iOS) + @MainActor public static func getIdentifierForVendor() -> String? { return UIDevice.current.identifierForVendor?.uuidString } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift index 801eca058..14885fa85 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/IPCClient.swift @@ -6,12 +6,11 @@ import CryptoKit import Foundation -import NetworkExtension +@preconcurrency import NetworkExtension // TODO: Use a more abstract IPC protocol to make this less terse -// TODO: Consider making this an actor to guarantee strict ordering -class IPCClient { +actor IPCClient { enum Error: Swift.Error { case decodeIPCDataFailed case noIPCData @@ -31,7 +30,7 @@ class IPCClient { // IPC only makes sense if there's a valid session. Session in this case refers to the `connection` field of // the NETunnelProviderManager instance. - let session: NETunnelProviderSession + nonisolated let session: NETunnelProviderSession // Track the "version" of the resource list so we can more efficiently // retrieve it from the Provider @@ -46,8 +45,8 @@ class IPCClient { } // Encoder used to send messages to the tunnel - let encoder = PropertyListEncoder() - let decoder = PropertyListDecoder() + nonisolated let encoder = PropertyListEncoder() + nonisolated let decoder = PropertyListDecoder() // Auto-connect @MainActor @@ -78,7 +77,7 @@ class IPCClient { try stop() } - func stop() throws { + nonisolated func stop() throws { try session().stopTunnel() } @@ -98,9 +97,8 @@ class IPCClient { } #endif - @MainActor func setConfiguration(_ configuration: Configuration) async throws { - let tunnelConfiguration = configuration.toTunnelConfiguration() + let tunnelConfiguration = await configuration.toTunnelConfiguration() let message = ProviderMessage.setConfiguration(tunnelConfiguration) if sessionStatus() != .connected { @@ -111,37 +109,38 @@ class IPCClient { } func fetchResources() async throws -> ResourceList { - return try await withCheckedThrowingContinuation { continuation in + // Capture current hash before entering continuation + let currentHash = resourceListHash + + // Get data from the provider - continuation returns just the data + let data = try await withCheckedThrowingContinuation { continuation in do { // Request list of resources from the provider. We send the hash of the resource list we already have. // If it differs, we'll get the full list in the callback. If not, we'll get nil. try session([.connected]).sendProviderMessage( - encoder.encode(ProviderMessage.getResourceList(resourceListHash)) + encoder.encode(ProviderMessage.getResourceList(currentHash)) ) { data in - guard let data = data - else { - // No data returned; Resources haven't changed - continuation.resume(returning: self.resourcesListCache) - - return - } - - // Save hash to compare against - self.resourceListHash = Data(SHA256.hash(data: data)) - - do { - let decoded = try self.decoder.decode([Resource].self, from: data) - self.resourcesListCache = ResourceList.loaded(decoded) - - continuation.resume(returning: self.resourcesListCache) - } catch { - continuation.resume(throwing: error) - } + continuation.resume(returning: data) } } catch { continuation.resume(throwing: error) } } + + // Back on the actor - safe to access and mutate state directly + guard let data = data else { + // No data returned; Resources haven't changed + return resourcesListCache + } + + // Save hash to compare against + resourceListHash = Data(SHA256.hash(data: data)) + + // Decode and cache the new resource list + let decoded = try decoder.decode([Resource].self, from: data) + resourcesListCache = ResourceList.loaded(decoded) + + return resourcesListCache } func clearLogs() async throws { @@ -176,7 +175,7 @@ class IPCClient { // Call this with a closure that will append each chunk to a buffer // of some sort, like a file. The completed buffer is a valid Apple Archive // in AAR format. - func exportLogs( + nonisolated func exportLogs( appender: @escaping (LogChunk) -> Void, errorHandler: @escaping (Error) -> Void ) { @@ -220,8 +219,9 @@ class IPCClient { // Subscribe to system notifications about our VPN status changing // and let our handler know about them. - func subscribeToVPNStatusUpdates(handler: @escaping @MainActor (NEVPNStatus) async throws -> Void) - { + nonisolated func subscribeToVPNStatusUpdates( + handler: @escaping @MainActor (NEVPNStatus) async throws -> Void + ) { Task { for await notification in NotificationCenter.default.notifications( named: .NEVPNStatusDidChange) @@ -232,9 +232,8 @@ class IPCClient { } if session.status == .disconnected { - // Reset resource list - resourceListHash = Data() - resourcesListCache = ResourceList.loading + // Reset resource list on disconnect + await self.resetResourceList() } do { try await handler(session.status) } catch { Log.error(error) } @@ -242,11 +241,17 @@ class IPCClient { } } - func sessionStatus() -> NEVPNStatus { + private func resetResourceList() { + resourceListHash = Data() + resourcesListCache = ResourceList.loading + } + + nonisolated func sessionStatus() -> NEVPNStatus { return session.status } - private func session(_ requiredStatuses: Set = []) throws -> NETunnelProviderSession + nonisolated private func session(_ requiredStatuses: Set = []) throws + -> NETunnelProviderSession { if requiredStatuses.isEmpty || requiredStatuses.contains(session.status) { return session diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Log.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Log.swift index 80462c16f..9bbd7ec16 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Log.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Log.swift @@ -8,25 +8,29 @@ import Foundation import OSLog public final class Log { - private static var logger = + private static let logger: Logger = { switch Bundle.main.bundleIdentifier { case "dev.firezone.firezone": - Logger(subsystem: "dev.firezone.firezone", category: "app") + return Logger(subsystem: "dev.firezone.firezone", category: "app") case "dev.firezone.firezone.network-extension": - Logger(subsystem: "dev.firezone.firezone", category: "tunnel") + return Logger(subsystem: "dev.firezone.firezone", category: "tunnel") default: fatalError("Unknown bundle id: \(Bundle.main.bundleIdentifier!)") } + }() - private static var logWriter = + private static let logWriter: LogWriter? = { + let folderURL: URL? switch Bundle.main.bundleIdentifier { case "dev.firezone.firezone": - LogWriter(folderURL: SharedAccess.appLogFolderURL, logger: logger) + folderURL = SharedAccess.appLogFolderURL case "dev.firezone.firezone.network-extension": - LogWriter(folderURL: SharedAccess.tunnelLogFolderURL, logger: logger) + folderURL = SharedAccess.tunnelLogFolderURL default: fatalError("Unknown bundle id: \(Bundle.main.bundleIdentifier!)") } + return LogWriter(folderURL: folderURL, logger: logger) + }() public static func log(_ message: String) { debug(message) @@ -67,11 +71,6 @@ public final class Log { let fileManager = FileManager.default var totalSize: Int64 = 0 - func sizeOfFile(at url: URL, with resourceValues: URLResourceValues) -> Int64 { - guard resourceValues.isRegularFile == true else { return 0 } - return Int64(resourceValues.totalFileAllocatedSize ?? resourceValues.totalFileSize ?? 0) - } - // Tally size of each log file in parallel await withTaskGroup(of: Int64.self) { taskGroup in fileManager.forEachFileUnder( @@ -82,8 +81,12 @@ public final class Log { .isRegularFileKey, ] ) { url, resourceValues in - taskGroup.addTask { - return sizeOfFile(at: url, with: resourceValues) + // Extract non-Sendable values before passing to @Sendable closure + guard resourceValues.isRegularFile == true else { return } + let size = Int64(resourceValues.totalFileAllocatedSize ?? resourceValues.totalFileSize ?? 0) + + taskGroup.addTask { @Sendable in + return size } } @@ -118,7 +121,9 @@ public final class Log { } } -private final class LogWriter { +/// Thread-safe: All mutable state access is serialised through workQueue. +/// Log writes are queued asynchronously to avoid blocking the caller. +private final class LogWriter: @unchecked Sendable { enum Severity: String, Codable { case trace = "TRACE" case debug = "DEBUG" @@ -228,14 +233,12 @@ private final class LogWriter { severity: severity, message: message) - guard var jsonData = try? jsonEncoder.encode(logEntry) + guard let jsonData = try? jsonEncoder.encode(logEntry) + Data("\n".utf8) else { logger.error("Could not encode log message to JSON!") return } - jsonData.append(Data("\n".utf8)) - workQueue.async { [weak self] in guard let self = self else { return } guard let handle = self.ensureFileExists() else { return } diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift index 06b53ba5d..e907ceb96 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/LogExporter.swift @@ -154,7 +154,9 @@ import System #endif extension LogExporter { - private static let fileManager = FileManager.default + /// Thread-safe: FileManager.default is documented as thread-safe by Apple. + /// Reference: https://developer.apple.com/documentation/foundation/filemanager + private nonisolated(unsafe) static let fileManager = FileManager.default static func now() -> String { let dateFormatter = ISO8601DateFormatter() diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Telemetry.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Telemetry.swift index f8b98a0a6..c25d9bf0c 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Telemetry.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Helpers/Telemetry.swift @@ -6,30 +6,50 @@ import Sentry +/// Actor that manages telemetry state with thread-safe access. +actor TelemetryState { + private var firezoneId: String? + private var accountSlug: String? + + func setFirezoneId(_ id: String?) { + firezoneId = id + updateUser() + } + + func setAccountSlug(_ slug: String?) { + accountSlug = slug + updateUser() + } + + private func updateUser() { + guard let firezoneId, let accountSlug else { + return + } + + SentrySDK.configureScope { configuration in + // Matches the format we use in rust/telemetry/lib.rs + let user = User(userId: firezoneId) + user.data = ["account_slug": accountSlug] + + configuration.setUser(user) + } + } +} + public enum Telemetry { // We can only create a new User object after Sentry is started; not retrieve // the existing one. So we need to collect these fields from various codepaths // during initialization / sign in so we can build a new User object any time // one of these is updated. - private static var _firezoneId: String? - private static var _accountSlug: String? - public static var firezoneId: String? { - get { - return self._firezoneId - } - set { - self._firezoneId = newValue - updateUser(id: self._firezoneId, slug: self._accountSlug) - } + + private static let state = TelemetryState() + + public static func setFirezoneId(_ id: String?) async { + await state.setFirezoneId(id) } - public static var accountSlug: String? { - get { - return self._accountSlug - } - set { - self._accountSlug = newValue - updateUser(id: self._firezoneId, slug: self._accountSlug) - } + + public static func setAccountSlug(_ slug: String?) async { + await state.setAccountSlug(slug) } public static func start() { @@ -68,22 +88,6 @@ public enum Telemetry { SentrySDK.capture(error: err) } - private static func updateUser(id: String?, slug: String?) { - guard let id, - let slug - else { - return - } - - SentrySDK.configureScope { configuration in - // Matches the format we use in rust/telemetry/lib.rs - let user = User(userId: id) - user.data = ["account_slug": slug] - - configuration.setUser(user) - } - } - private static func distributionType() -> String { // Apps from the app store have a receipt file if BundleHelper.isAppStore() { diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift index 4b6ea0b46..4bfcaee49 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Managers/VPNConfigurationManager.swift @@ -21,7 +21,9 @@ enum VPNConfigurationManagerError: Error { } } -public class VPNConfigurationManager { +/// Thread-safe: Immutable wrapper around NETunnelProviderManager. +/// Only contains a 'let' property. NETunnelProviderManager handles its own synchronisation. +public class VPNConfigurationManager: @unchecked Sendable { // Persists our tunnel settings let manager: NETunnelProviderManager diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift index 3e47ba129..cc00edd3a 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Configuration.swift @@ -108,7 +108,11 @@ public class Configuration: ObservableObject { static let supportURL = "supportURL" } - private var defaults: UserDefaults + // 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 init(userDefaults: UserDefaults = UserDefaults.standard) { defaults = userDefaults @@ -176,7 +180,7 @@ public class Configuration: ObservableObject { } // Configuration does not conform to Decodable, so introduce a simpler type here to encode for IPC -public struct TunnelConfiguration: Codable { +public struct TunnelConfiguration: Codable, Sendable { public let apiURL: String public let accountSlug: String public let logFilter: String diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Resource.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Resource.swift index f39bd73ef..c6d0864c6 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Resource.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Resource.swift @@ -9,11 +9,11 @@ import Foundation class StatusSymbol { - static var enabled: String = "<->" - static var disabled: String = "—" + static let enabled: String = "<->" + static let disabled: String = "—" } -public enum ResourceList { +public enum ResourceList: Sendable { case loading case loaded([Resource]) @@ -27,7 +27,7 @@ public enum ResourceList { } } -public struct Resource: Codable, Identifiable, Equatable { +public struct Resource: Codable, Identifiable, Equatable, Sendable { public let id: String public var name: String public var address: String? @@ -59,7 +59,7 @@ public struct Resource: Codable, Identifiable, Equatable { } } -public enum ResourceStatus: String, Codable { +public enum ResourceStatus: String, Codable, Sendable { case offline = "Offline" case online = "Online" case unknown = "Unknown" @@ -91,7 +91,7 @@ public enum ResourceStatus: String, Codable { } } -public enum ResourceType: String, Codable { +public enum ResourceType: String, Codable, Sendable { case dns case cidr case ip diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift index 94c7205ad..f31c32c38 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/SessionNotification.swift @@ -22,6 +22,7 @@ public enum NotificationIndentifier: String { case dismissNotificationAction } +@MainActor public class SessionNotification: NSObject { public var signInHandler = {} private let notificationCenter = UNUserNotificationCenter.current() @@ -69,7 +70,7 @@ public class SessionNotification: NSObject { #if os(iOS) // In iOS, use User Notifications. // This gets called from the tunnel side. - public static func showSignedOutNotificationiOS() { + nonisolated public static func showSignedOutNotificationiOS() { UNUserNotificationCenter.current().getNotificationSettings { notificationSettings in if notificationSettings.authorizationStatus == .authorized { Log.log( @@ -127,7 +128,7 @@ public class SessionNotification: NSObject { #if os(iOS) extension SessionNotification: UNUserNotificationCenterDelegate { - public func userNotificationCenter( + nonisolated public func userNotificationCenter( _ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void @@ -139,12 +140,12 @@ public class SessionNotification: NSObject { actionId == NotificationIndentifier.signInNotificationAction.rawValue { // User clicked on 'Sign In' in the notification - signInHandler() + Task { @MainActor in + signInHandler() + } } - DispatchQueue.main.async { - completionHandler() - } + completionHandler() } } #endif diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Site.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Site.swift index 84d6a41bc..f67fd9944 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Site.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Site.swift @@ -7,7 +7,7 @@ import Foundation -public struct Site: Codable, Identifiable, Equatable { +public struct Site: Codable, Identifiable, Equatable, Sendable { public let id: String public var name: String diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Token.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Token.swift index 5d4497ac9..7dd91d430 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Token.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/Token.swift @@ -15,7 +15,9 @@ public struct Token: CustomStringConvertible { private static let label = "Firezone token" #endif - private static let query: [CFString: Any] = [ + /// Thread-safe: Immutable dictionary initialised at compile time. + /// CFString keys and constant string values are both Sendable. + private nonisolated(unsafe) static let query: [CFString: Any] = [ kSecAttrLabel: "Firezone token", kSecAttrAccount: "1", kSecAttrService: BundleHelper.appGroupId, diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift index 93ad842d2..d09cd975c 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Models/WebAuthSession.swift @@ -13,7 +13,6 @@ import Foundation @MainActor struct WebAuthSession { private static let scheme = "firezone-fd0020211111" - static let anchor = PresentationAnchor() static func signIn(store: Store, configuration: Configuration? = nil) async throws { let configuration = configuration ?? Configuration.shared @@ -27,8 +26,33 @@ struct WebAuthSession { throw AuthClientError.invalidAuthURL } - let authResponse: AuthResponse? = try await withCheckedThrowingContinuation { continuation in - let session = ASWebAuthenticationSession(url: url, callbackURLScheme: scheme) { + // Create anchor on MainActor, then pass to concurrent function + let anchor = PresentationAnchor() + + // Call @concurrent function to avoid MainActor inference on callback closure + let authResponse = try await performAuthentication( + url: url, + callbackScheme: scheme, + authClient: authClient, + anchor: anchor + ) + + if let authResponse { + try await store.signIn(authResponse: authResponse) + } + } + + // @concurrent runs on global executor, preventing closure from inheriting MainActor isolation + @concurrent private static func performAuthentication( + url: URL, + callbackScheme: String, + authClient: AuthClient, + anchor: PresentationAnchor + ) async throws -> AuthResponse? { + // Anchor passed as parameter, keeping strong reference + + return try await withCheckedThrowingContinuation { continuation in + let session = ASWebAuthenticationSession(url: url, callbackURLScheme: callbackScheme) { returnedURL, error in do { if let error = error as? ASWebAuthenticationSessionError, @@ -55,18 +79,20 @@ struct WebAuthSession { // load cookies session.prefersEphemeralWebBrowserSession = false - // Start auth - session.start() - } - - if let authResponse { - try await store.signIn(authResponse: authResponse) + // Start auth - must be called on MainActor + // Use Task to asynchronously hop to MainActor from concurrent context + // (cannot use await MainActor.run - withCheckedThrowingContinuation requires sync closure) + Task { @MainActor in + session.start() + } } } } // Required shim to use as "presentationAnchor" for the Webview. Why Apple? -final class PresentationAnchor: NSObject, ASWebAuthenticationPresentationContextProviding { +final class PresentationAnchor: NSObject, ASWebAuthenticationPresentationContextProviding, + Sendable +{ @MainActor func presentationAnchor(for _: ASWebAuthenticationSession) -> ASPresentationAnchor { ASPresentationAnchor() diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift index 81b3ed114..56359143a 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift @@ -42,6 +42,9 @@ public final class Store: ObservableObject { private var vpnConfigurationManager: VPNConfigurationManager? private var cancellables: Set = [] + // Cached IPCClient instance - one per Store instance + private var cachedIPCClient: IPCClient? + // Track which session expired alerts have been shown to prevent duplicates private var shownAlertIds: Set @@ -107,7 +110,8 @@ public final class Store: ObservableObject { #endif private func setupTunnelObservers() async throws { - let vpnStatusChangeHandler: (NEVPNStatus) async throws -> Void = { [weak self] status in + let vpnStatusChangeHandler: @Sendable (NEVPNStatus) async throws -> Void = { + [weak self] status in try await self?.handleVPNStatusChange(newVPNStatus: status) } try ipcClient().subscribeToVPNStatusUpdates(handler: vpnStatusChangeHandler) @@ -200,16 +204,27 @@ public final class Store: ObservableObject { // Create a new VPN configuration in system settings. self.vpnConfigurationManager = try await VPNConfigurationManager() + // Invalidate cached IPCClient since we have a new configuration + cachedIPCClient = nil + try await setupTunnelObservers() } func ipcClient() throws -> IPCClient { + // Return cached instance if it exists + if let cachedIPCClient = cachedIPCClient { + return cachedIPCClient + } + + // Create new instance and cache it guard let session = try manager().session() else { throw VPNConfigurationManagerError.managerNotInitialized } - return IPCClient(session: session) + let client = IPCClient(session: session) + cachedIPCClient = client + return client } func manager() throws -> VPNConfigurationManager { @@ -248,7 +263,7 @@ public final class Store: ObservableObject { UserDefaults.standard.set(actorName, forKey: "actorName") configuration.accountSlug = accountSlug - Telemetry.accountSlug = accountSlug + await Telemetry.setAccountSlug(accountSlug) try await manager().enable() diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift index 98bc7fab0..2eae95242 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/UpdateNotification.swift @@ -23,7 +23,11 @@ } } - private var timer: Timer? + // nonisolated(unsafe) is required because: + // 1. Timer.invalidate() is thread-safe and can be called from any thread + // 2. Only accessed from MainActor-isolated methods (init, start/stop) and nonisolated deinit + // 3. deinit is nonisolated and needs access to invalidate the timer + private nonisolated(unsafe) var timer: Timer? private let notificationAdapter: NotificationAdapter = NotificationAdapter() private let versionCheckUrl: URL private let marketingVersion: SemanticVersion diff --git a/swift/apple/FirezoneNetworkExtension/Adapter.swift b/swift/apple/FirezoneNetworkExtension/Adapter.swift index a9b1336f6..5f9ba57eb 100644 --- a/swift/apple/FirezoneNetworkExtension/Adapter.swift +++ b/swift/apple/FirezoneNetworkExtension/Adapter.swift @@ -12,6 +12,24 @@ import Foundation import NetworkExtension import OSLog +/// Thread-safe wrapper for mutable state using NSLock. +/// Provides similar API to OSAllocatedUnfairLock but compatible with iOS 15+. +/// We can't use OSAllocatedUnfairLock as it requires iOS 16+. +final class LockedState: @unchecked Sendable { + private let lock = NSLock() + private var _value: Value + + init(initialState: Value) { + _value = initialState + } + + func withLock(_ body: (inout Value) -> Result) -> Result { + lock.lock() + defer { lock.unlock() } + return body(&_value) + } +} + enum AdapterError: Error { /// Failure to perform an operation in such state. case invalidSession(Session?) @@ -106,7 +124,7 @@ class Adapter: @unchecked Sendable { /// - https://github.com/firezone/firezone/issues/3343 /// - https://github.com/firezone/firezone/issues/3235 /// - https://github.com/firezone/firezone/issues/3175 - private lazy var pathUpdateHandler: (Network.NWPath) -> Void = { [weak self] path in + private lazy var pathUpdateHandler: @Sendable (Network.NWPath) -> Void = { [weak self] path in guard let self else { return } if path.status == .unsatisfied { @@ -184,19 +202,39 @@ class Adapter: @unchecked Sendable { func start() throws { Log.log("Adapter.start: Starting session for account: \(accountSlug)") - // Get device metadata - let deviceName = DeviceMetadata.getDeviceName() + // Get device metadata - synchronously get values from MainActor + #if os(iOS) + let deviceMetadata = LockedState<(String, String?)>(initialState: ("", nil)) + #else + let deviceMetadata = LockedState(initialState: "") + #endif + let semaphore = DispatchSemaphore(value: 0) + + Task { @MainActor in + let name = DeviceMetadata.getDeviceName() + #if os(iOS) + let identifier = DeviceMetadata.getIdentifierForVendor() + deviceMetadata.withLock { $0 = (name, identifier) } + #else + deviceMetadata.withLock { $0 = name } + #endif + semaphore.signal() + } + semaphore.wait() + let osVersion = DeviceMetadata.getOSVersion() let logDir = SharedAccess.connlibLogFolderURL?.path ?? "/tmp/firezone" #if os(iOS) + let (deviceName, identifierForVendor) = deviceMetadata.withLock { $0 } let deviceInfo = DeviceInfo( firebaseInstallationId: nil, deviceUuid: nil, deviceSerial: nil, - identifierForVendor: DeviceMetadata.getIdentifierForVendor() + identifierForVendor: identifierForVendor ) #else + let deviceName = deviceMetadata.withLock { $0 } let deviceInfo = DeviceInfo( firebaseInstallationId: nil, deviceUuid: getDeviceUuid(), @@ -291,22 +329,28 @@ class Adapter: @unchecked Sendable { /// Get the current set of resources in the completionHandler, only returning /// them if the resource list has changed. func getResourcesIfVersionDifferentFrom( - hash: Data, completionHandler: @escaping (Data?) -> Void + hash: Data, completionHandler: @escaping @Sendable (Data?) -> Void ) { - // This is async to avoid blocking the main UI thread - workQueue.async { [weak self] in - guard let self = self else { return } + Task { [weak self] in + guard let self = self else { + completionHandler(nil) + return + } // Convert uniffi resources to FirezoneKit resources and encode with PropertyList guard let uniffiResources = self.resources - else { return completionHandler(nil) } + else { + completionHandler(nil) + return + } let firezoneResources = uniffiResources.map { self.convertResource($0) } guard let encoded = try? PropertyListEncoder().encode(firezoneResources) else { Log.log("Failed to encode resources as PropertyList") - return completionHandler(nil) + completionHandler(nil) + return } if hash == Data(SHA256.hash(data: encoded)) { @@ -557,7 +601,11 @@ class Adapter: @unchecked Sendable { return BindResolvers.getServers() } - var resolvers: [String] = [] + // Use a class box to safely capture result across @Sendable closure boundary + final class ResolversBox: @unchecked Sendable { + var value: [String] = [] + } + let resolversBox = ResolversBox() // The caller is in an async context, so it's ok to block this thread here. let semaphore = DispatchSemaphore(value: 0) @@ -570,7 +618,7 @@ class Adapter: @unchecked Sendable { guard let networkSettings = self.networkSettings else { return } // Only now can we get the system resolvers - resolvers = BindResolvers.getServers() + resolversBox.value = BindResolvers.getServers() // Restore connlib's DNS resolvers networkSettings.clearDummyMatchDomain() @@ -578,7 +626,7 @@ class Adapter: @unchecked Sendable { } semaphore.wait() - return resolvers + return resolversBox.value } } #endif diff --git a/swift/apple/FirezoneNetworkExtension/BindResolvers.swift b/swift/apple/FirezoneNetworkExtension/BindResolvers.swift index 3470e3223..cabbfe915 100644 --- a/swift/apple/FirezoneNetworkExtension/BindResolvers.swift +++ b/swift/apple/FirezoneNetworkExtension/BindResolvers.swift @@ -55,6 +55,14 @@ enum BindResolvers { NI_NUMERICHOST) } } - return String(cString: hostBuffer) + // Truncate null termination and decode as UTF-8 + // Convert CChar (Int8) to UInt8 for String(decoding:) + if let nullIndex = hostBuffer.firstIndex(of: 0) { + let bytes = hostBuffer[.. AppGroupIdentifier $(APP_GROUP_ID) + LSEnvironment + + SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL + 1 + SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE + nocrash + diff --git a/swift/apple/FirezoneNetworkExtension/Info.macOS.plist b/swift/apple/FirezoneNetworkExtension/Info.macOS.plist index b582aa148..02517a23d 100644 --- a/swift/apple/FirezoneNetworkExtension/Info.macOS.plist +++ b/swift/apple/FirezoneNetworkExtension/Info.macOS.plist @@ -26,6 +26,13 @@ $(APP_GROUP_ID) NSHumanReadableCopyright Copyright © 2025 Firezone, Inc. All rights reserved. + LSEnvironment + + SWIFT_UNEXPECTED_EXECUTOR_LOG_LEVEL + 1 + SWIFT_IS_CURRENT_EXECUTOR_LEGACY_MODE_OVERRIDE + nocrash + NetworkExtension NEMachServiceName diff --git a/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift b/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift index 819090fd9..e51fd36ec 100644 --- a/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift +++ b/swift/apple/FirezoneNetworkExtension/NetworkSettings.swift @@ -53,7 +53,7 @@ class NetworkSettings { self.matchDomains.append(contentsOf: self.searchDomains) } - func apply(completionHandler: (() -> Void)? = nil) { + func apply(completionHandler: (@Sendable () -> Void)? = nil) { // We don't really know the connlib gateway IP address at this point, but just using 127.0.0.1 is okay // because the OS doesn't really need this IP address. // NEPacketTunnelNetworkSettings taking in tunnelRemoteAddress is probably a bad abstraction caused by diff --git a/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift b/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift index 856409e84..61155e3ff 100644 --- a/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift +++ b/swift/apple/FirezoneNetworkExtension/PacketTunnelProvider.swift @@ -54,7 +54,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { override func startTunnel( options: [String: NSObject]?, - completionHandler: @escaping (Error?) -> Void + completionHandler: @escaping @Sendable (Error?) -> Void ) { // Dummy start to get the extension running on macOS after upgrade if options?["dryRun"] as? Bool == true { @@ -114,7 +114,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { // Configure telemetry Telemetry.setEnvironmentOrClose(apiURL) - Telemetry.accountSlug = accountSlug + Task { await Telemetry.setAccountSlug(accountSlug) } let enabled = legacyConfiguration?["internetResourceEnabled"] let internetResourceEnabled = @@ -152,7 +152,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { // When called by the system, we call Adapter.stop() from here. // When initiated by connlib, we've already called stop() there. override func stopTunnel( - with reason: NEProviderStopReason, completionHandler: @escaping () -> Void + with reason: NEProviderStopReason, completionHandler: @escaping @Sendable () -> Void ) { Log.log("stopTunnel: Reason: \(reason)") @@ -164,8 +164,9 @@ class PacketTunnelProvider: NEPacketTunnelProvider { // It would be helpful to be able to encapsulate Errors here. To do that // we need to update ProviderMessage to encode/decode Result to and from Data. // TODO: Move to a more abstract IPC protocol - @MainActor - override func handleAppMessage(_ message: Data, completionHandler: ((Data?) -> Void)? = nil) { + override func handleAppMessage( + _ message: Data, completionHandler: (@Sendable (Data?) -> Void)? = nil + ) { do { let providerMessage = try PropertyListDecoder().decode(ProviderMessage.self, from: message) @@ -221,7 +222,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { return Token(passedToken) ?? keychainToken } - func clearLogs(_ completionHandler: ((Data?) -> Void)? = nil) { + func clearLogs(_ completionHandler: (@Sendable (Data?) -> Void)? = nil) { do { try Log.clear(in: SharedAccess.logFolderURL) } catch { @@ -231,7 +232,7 @@ class PacketTunnelProvider: NEPacketTunnelProvider { completionHandler?(nil) } - func getLogFolderSize(_ completionHandler: ((Data?) -> Void)? = nil) { + func getLogFolderSize(_ completionHandler: (@Sendable (Data?) -> Void)? = nil) { guard let logFolderURL = SharedAccess.logFolderURL else { completionHandler?(nil) @@ -239,17 +240,18 @@ class PacketTunnelProvider: NEPacketTunnelProvider { return } - getLogFolderSizeTask = Task { + let task = Task { let size = await Log.size(of: logFolderURL) let data = withUnsafeBytes(of: size) { Data($0) } - // Ensure completionHandler is called on the same actor as handleAppMessage and isn't cancelled by deinit - if getLogFolderSizeTask?.isCancelled ?? true { return } - await MainActor.run { completionHandler?(data) } + // Call completion handler with data if not cancelled + guard !Task.isCancelled else { return } + completionHandler?(data) } + getLogFolderSizeTask = task } - func exportLogs(_ completionHandler: @escaping (Data?) -> Void) { + func exportLogs(_ completionHandler: @escaping @Sendable (Data?) -> Void) { func sendChunk(_ tunnelLogArchive: TunnelLogArchive) { do { let (chunk, done) = try tunnelLogArchive.readChunk()