fix(apple): Use Task.detached when starting from MainActor (#7766)

When starting a Task, by default it's launched with the same priority as
the calling code.

In the UI these are run on the `MainActor` with highest priority by
default. If the worker thread running the Task closure gets blocked, it
will cause the UI to hang.

To fix this, we use `Task.detached` which runs the closure without a
specific priority, which is lower than the UI thread.

Furthermore, `weak self` is used to prevent retain cycles if the parent
thread `deinit`s.

This was causing an issue primarily when making IPC calls because those
will sometimes hang until the XPC service is launched for the first
time.

---------

Signed-off-by: Jamil <jamilbk@users.noreply.github.com>
This commit is contained in:
Jamil
2025-01-16 07:26:03 -08:00
committed by GitHub
parent e4cfe6d5a2
commit 10847fd549
5 changed files with 53 additions and 53 deletions

View File

@@ -174,7 +174,7 @@ public class VPNConfigurationManager {
}
}
func loadFromPreferences(vpnStateUpdateHandler: @escaping (NEVPNStatus, Settings?, String?) -> Void) async throws {
func loadFromPreferences(vpnStateUpdateHandler: @escaping @MainActor (NEVPNStatus, Settings?, String?) -> Void) async throws {
// loadAllFromPreferences() returns list of VPN configurations created by our main app's bundle ID.
// Since our bundle ID can change (by us), find the one that's current and ignore the others.
let managers = try await NETunnelProviderManager.loadAllFromPreferences()
@@ -206,7 +206,7 @@ public class VPNConfigurationManager {
Telemetry.accountSlug = providerConfiguration[VPNConfigurationManagerKeys.accountSlug]
// Share what we found with our caller
vpnStateUpdateHandler(status, settings, actorName)
await vpnStateUpdateHandler(status, settings, actorName)
// Stop looking for our tunnel
break
@@ -216,7 +216,7 @@ public class VPNConfigurationManager {
// If no tunnel configuration was found, update state to
// prompt user to create one.
if manager == nil {
vpnStateUpdateHandler(.invalid, nil, nil)
await vpnStateUpdateHandler(.invalid, nil, nil)
}
// Hook up status updates
@@ -475,7 +475,7 @@ public class VPNConfigurationManager {
// Subscribe to system notifications about our VPN status changing
// and let our handler know about them.
private func subscribeToVPNStatusUpdates(handler: @escaping (NEVPNStatus, Settings?, String?) -> Void) {
private func subscribeToVPNStatusUpdates(handler: @escaping @MainActor (NEVPNStatus, Settings?, String?) -> Void) {
Log.log("\(#function)")
for task in tunnelObservingTasks {
@@ -500,7 +500,7 @@ public class VPNConfigurationManager {
resourcesListCache = ResourceList.loading
}
handler(session.status, nil, nil)
await handler(session.status, nil, nil)
}
}
)

View File

@@ -70,19 +70,17 @@ public final class Store: ObservableObject {
func bindToVPNConfigurationUpdates() async throws {
// Load our existing VPN configuration and set an update handler
try await self.vpnConfigurationManager.loadFromPreferences(
vpnStateUpdateHandler: { [weak self] status, settings, actorName in
vpnStateUpdateHandler: { @MainActor [weak self] status, settings, actorName in
guard let self else { return }
DispatchQueue.main.async {
self.status = status
self.status = status
if let settings {
self.settings = settings
}
if let settings {
self.settings = settings
}
if let actorName {
self.actorName = actorName
}
if let actorName {
self.actorName = actorName
}
if status == .disconnected {
@@ -98,7 +96,9 @@ public final class Store: ObservableObject {
/// reason and alert the user if it was due to receiving a 401 from the
/// portal.
private func maybeShowSignedOutAlert() {
Task {
Task.detached { [weak self] in
guard let self else { return }
do {
if let savedValue = try await self.vpnConfigurationManager.consumeStopReason(),
let rawValue = Int(savedValue),
@@ -106,7 +106,7 @@ public final class Store: ObservableObject {
case .authenticationCanceled = reason
{
#if os(macOS)
self.sessionNotification.showSignedOutAlertmacOS()
await self.sessionNotification.showSignedOutAlertmacOS()
#endif
}
} catch {
@@ -190,7 +190,7 @@ public final class Store: ObservableObject {
func signIn(authResponse: AuthResponse) async throws {
// Save actorName
DispatchQueue.main.async { self.actorName = authResponse.actorName }
await MainActor.run { self.actorName = authResponse.actorName }
try await self.vpnConfigurationManager.saveSettings(settings)
try await self.vpnConfigurationManager.saveAuthResponse(authResponse)
@@ -212,9 +212,8 @@ public final class Store: ObservableObject {
self.vpnConfigurationManager.fetchResources(callback: callback)
let intervalInSeconds: TimeInterval = 1
let timer = Timer(timeInterval: intervalInSeconds, repeats: true) { [weak self] _ in
Task { @MainActor in
guard let self else { return }
self.vpnConfigurationManager.fetchResources(callback: callback)
Task.detached {
await self?.vpnConfigurationManager.fetchResources(callback: callback)
}
}
RunLoop.main.add(timer, forMode: .common)
@@ -226,11 +225,13 @@ public final class Store: ObservableObject {
resourcesTimer = nil
}
func save(_ newSettings: Settings) async throws {
Task {
func save(_ newSettings: Settings) {
Task.detached { [weak self] in
guard let self else { return }
do {
try await self.vpnConfigurationManager.saveSettings(newSettings)
DispatchQueue.main.async { self.settings = newSettings }
await DispatchQueue.main.async { self.settings = newSettings }
} catch {
Log.error(error)
}
@@ -241,8 +242,6 @@ public final class Store: ObservableObject {
self.vpnConfigurationManager.toggleInternetResource(enabled: enabled)
var newSettings = settings
newSettings.internetResourceEnabled = self.vpnConfigurationManager.internetResourceEnabled
Task {
try await save(newSettings)
}
save(newSettings)
}
}

View File

@@ -36,9 +36,7 @@ struct FirstTimeView: View {
.buttonStyle(.borderedProminent)
.controlSize(.large)
Button("Open menu") {
DispatchQueue.main.async {
menuBar.showMenu()
}
menuBar.showMenu()
AppViewModel.WindowDefinition.main.window()?.close()
}
.buttonStyle(.borderedProminent)

View File

@@ -265,13 +265,15 @@ public final class MenuBar: NSObject, ObservableObject {
}
@objc private func signOutButtonTapped() {
Task {
try await model.store.signOut()
Task.detached { [weak self] in
try await self?.model.store.signOut()
}
}
@objc private func grantPermissionMenuItemTapped() {
Task {
Task.detached { [weak self] in
guard let self else { return }
do {
// If we get here, it means either system extension got disabled or
// our VPN configuration got removed. Since we don't know which, reinstall
@@ -314,9 +316,11 @@ public final class MenuBar: NSObject, ObservableObject {
}
@objc private func quitButtonTapped() {
Task {
model.store.stop()
NSApp.terminate(self)
Task.detached { [weak self] in
guard let self else { return }
await self.model.store.stop()
await NSApp.terminate(self)
}
}
@@ -357,8 +361,8 @@ public final class MenuBar: NSObject, ObservableObject {
guard connectingAnimationTimer == nil else { return }
let timer = Timer(timeInterval: 0.25, repeats: true) { [weak self] _ in
guard let self = self else { return }
Task {
await self.connectingAnimationShowNextFrame()
Task.detached { [weak self] in
await self?.connectingAnimationShowNextFrame()
}
}
RunLoop.main.add(timer, forMode: .common)

View File

@@ -47,16 +47,13 @@ public final class SettingsViewModel: ObservableObject {
}
func saveSettings() {
Task {
if [.connected, .connecting, .reasserting].contains(store.status) {
_ = try await store.signOut()
Task.detached { [weak self] in
do { try await self?.store.signOut() } catch { Log.error(error) }
}
}
do {
try await store.save(settings)
} catch {
Log.error(error)
}
}
store.save(settings)
}
// Calculates the total size of our logs by summing the size of the
@@ -493,11 +490,13 @@ public struct SettingsView: View {
isProcessing: $isExportingLogs,
action: {
self.isExportingLogs = true
Task {
Task.detached {
let archiveURL = LogExporter.tempFile()
try await LogExporter.export(to: archiveURL)
self.logTempZipFileURL = archiveURL
self.isPresentingExportLogShareSheet = true
await MainActor.run {
self.logTempZipFileURL = archiveURL
self.isPresentingExportLogShareSheet = true
}
}
}
)
@@ -606,7 +605,7 @@ public struct SettingsView: View {
return
}
Task {
Task.detached {
do {
try await LogExporter.export(
to: destinationURL,
@@ -619,15 +618,15 @@ public struct SettingsView: View {
} catch {
Log.error(error)
let alert = NSAlert()
alert.messageText = "Error exporting logs: \(error.localizedDescription)"
alert.alertStyle = .critical
let alert = await NSAlert()
await MainActor.run {
alert.messageText = "Error exporting logs: \(error.localizedDescription)"
alert.alertStyle = .critical
let _ = alert.runModal()
}
}
self.isExportingLogs = false
await MainActor.run { self.isExportingLogs = false }
}
}
}