From 787eac86ac66ece6fa11586f0d071c04bc85a357 Mon Sep 17 00:00:00 2001 From: Jamil Date: Mon, 20 Jan 2025 19:33:22 -0800 Subject: [PATCH] fix(apple): Use Task.detached when loading sysex and vpn config (#7815) When the app starts, we perform various checks in the `AppViewModel.init` which read and write to disk, which can potentially be slow (a few seconds), especially for busy rotational hard drives. These were performed inside a regular `Task` closure, but since AppViewModel is annotated `@MainActor`, that meant this Task blocked the main UI thread until the operations completed. In practice this wasn't an issue because it simply manifested as the app taking a couple seconds to launch under these conditions. We fix this by simply using a `Task.detached` which will run the operations on another thread. Now, the first window will pop up sooner and immediately show the `ProgressView()` (i.e. a loading spinner icon) until these operations complete. A few minor reorganizing of the `os()` macro was also performed because some of the variables now need to be `await`ed because they live on the main thread. refs #7798 --- .../Sources/FirezoneKit/Stores/Store.swift | 14 ++++++++------ .../Sources/FirezoneKit/Views/AppView.swift | 19 +++++++++++-------- .../FirezoneKit/Views/GrantVPNView.swift | 6 ++++-- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift index c3026ebfa..5fd147efe 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Stores/Store.swift @@ -115,11 +115,11 @@ public final class Store: ObservableObject { } } - func checkedIfInstalled() async throws { #if os(macOS) + func checkedSystemExtensionStatus() async throws -> SystemExtensionStatus { let checker = SystemExtensionManager() - self.systemExtensionStatus = try await withCheckedThrowingContinuation { + let status = try await withCheckedThrowingContinuation { (continuation: CheckedContinuation) in checker.checkStatus( @@ -130,14 +130,16 @@ public final class Store: ObservableObject { // If already installed but the wrong version, go ahead and install. // This shouldn't prompt the user. - if self.systemExtensionStatus == .needsReplacement { + if status == .needsReplacement { try await installSystemExtension() } -#endif + + self.systemExtensionStatus = status + + return status } func installSystemExtension() async throws { -#if os(macOS) let installer = SystemExtensionManager() // Apple recommends installing the system extension as early as possible after app launch. @@ -150,8 +152,8 @@ public final class Store: ObservableObject { continuation: continuation ) } -#endif } +#endif func grantVPNPermission() async throws { // Create a new VPN configuration in system settings. diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift index 01e554338..5168c0e64 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/AppView.swift @@ -32,27 +32,30 @@ public class AppViewModel: ObservableObject { self.favorites = favorites self.store = store - Task { + Task.detached { [weak self] in + guard let self else { return } + do { try await self.store.bindToVPNConfigurationUpdates() + let vpnConfigurationStatus = await self.store.status #if os(macOS) - try await self.store.checkedIfInstalled() - let isInstalled = self.store.systemExtensionStatus == .installed + let systemExtensionStatus = try await self.store.checkedSystemExtensionStatus() - if !isInstalled || self.store.status == .invalid { + if systemExtensionStatus != .installed + || vpnConfigurationStatus == .invalid { // Show the main Window if VPN permission needs to be granted - AppViewModel.WindowDefinition.main.openWindow() + await AppViewModel.WindowDefinition.main.openWindow() } else { - AppViewModel.WindowDefinition.main.window()?.close() + await AppViewModel.WindowDefinition.main.window()?.close() } #endif - if self.store.status == .disconnected { + if vpnConfigurationStatus == .disconnected { // Try to connect on start - self.store.vpnConfigurationManager.start() + await self.store.vpnConfigurationManager.start() } } catch { Log.error(error) diff --git a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift index edcc9f207..5966d3fb8 100644 --- a/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift +++ b/swift/apple/FirezoneKit/Sources/FirezoneKit/Views/GrantVPNView.swift @@ -27,21 +27,23 @@ final class GrantVPNViewModel: ObservableObject { #endif } +#if os(macOS) func installSystemExtensionButtonTapped() { Task { do { try await store.installSystemExtension() -#if os(macOS) + // The window has a tendency to go to the background after installing // the system extension NSApp.activate(ignoringOtherApps: true) -#endif + } catch { Log.error(error) } } } +#endif func grantPermissionButtonTapped() { Log.log("\(#function)")