From fe6a5c59a1eecfc15c795c2f0422a7ce77f00991 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Mon, 29 Apr 2024 21:45:20 +0200 Subject: [PATCH 1/3] Give Virtualization.framework a chance to stop the VM on tart stop We were letting the CancellationError bubble up all the way until it terminated app, which meant we didn't hit the shutdown code in run(), stopping the VM and the network. We now catch CancellationError and proceed to gracefully shut down. We only stop the VM if it's still running, as a VM that has been stopped via the menu can't be stopped again. --- Sources/tart/VM.swift | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/Sources/tart/VM.swift b/Sources/tart/VM.swift index 6a6be9d7..9bdc1deb 100644 --- a/Sources/tart/VM.swift +++ b/Sources/tart/VM.swift @@ -244,10 +244,18 @@ class VM: NSObject, VZVirtualMachineDelegate, ObservableObject { } func run() async throws { - try await sema.waitUnlessCancelled() + do { + try await sema.waitUnlessCancelled() + } catch is CancellationError { + // Triggered by "tart stop", Ctrl+C, or closing the + // VM window, so shut down the VM gracefully below. + } if Task.isCancelled { - try await stop() + if (self.virtualMachine.state == VZVirtualMachine.State.running) { + print("Stopping VM...") + try await stop() + } } try await network.stop() From c640a25b6a094f6b48a3af088a79588168e4382c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Mon, 29 Apr 2024 21:45:31 +0200 Subject: [PATCH 2/3] Gracefully shut down VM when Tart is quit via menu Normally the quit action will result in AppKit calling exit(), but we want to gracefully shut down the VM, so we use the same path as for closing of the VM window, namely signal our own process with SIGINT or SIGUSR1. If that doesn't work we let AppKit terminate as before. This fixes the "Warning: NSActivity <_NSActivityAssertion: 0x600001f785a0> was ended multiple times" warning seen on the console when quitting Tart via the menu. --- Sources/tart/Commands/Run.swift | 124 +++++++++++++++++--------------- 1 file changed, 66 insertions(+), 58 deletions(-) diff --git a/Sources/tart/Commands/Run.swift b/Sources/tart/Commands/Run.swift index e877bf3b..30efe8f2 100644 --- a/Sources/tart/Commands/Run.swift +++ b/Sources/tart/Commands/Run.swift @@ -567,69 +567,69 @@ struct Run: AsyncParsableCommand { nsApp.setActivationPolicy(.regular) nsApp.activate(ignoringOtherApps: true) - struct MainApp: App { - static var suspendable: Bool = false - static var capturesSystemKeys: Bool = false - - @NSApplicationDelegateAdaptor private var appDelegate: MinimalMenuAppDelegate - - var body: some Scene { - WindowGroup(vm!.name) { - Group { - VMView(vm: vm!, capturesSystemKeys: MainApp.capturesSystemKeys).onAppear { - NSWindow.allowsAutomaticWindowTabbing = false - }.onDisappear { - let ret = kill(getpid(), MainApp.suspendable ? SIGUSR1 : SIGINT) - if ret != 0 { - // Fallback to the old termination method that doesn't - // propagate the cancellation to Task's in case graceful - // termination via kill(2) is not successful - NSApplication.shared.terminate(self) - } - } - }.frame( - minWidth: CGFloat(vm!.config.display.width), - idealWidth: CGFloat(vm!.config.display.width), - maxWidth: .infinity, - minHeight: CGFloat(vm!.config.display.height), - idealHeight: CGFloat(vm!.config.display.height), - maxHeight: .infinity - ) - }.commands { - // Remove some standard menu options - CommandGroup(replacing: .help, addition: {}) - CommandGroup(replacing: .newItem, addition: {}) - CommandGroup(replacing: .pasteboard, addition: {}) - CommandGroup(replacing: .textEditing, addition: {}) - CommandGroup(replacing: .undoRedo, addition: {}) - CommandGroup(replacing: .windowSize, addition: {}) - // Replace some standard menu options - CommandGroup(replacing: .appInfo) { AboutTart(config: vm!.config) } - CommandMenu("Control") { - Button("Start") { - Task { try await vm!.virtualMachine.start() } - } - Button("Stop") { - Task { try await vm!.virtualMachine.stop() } - } - Button("Request Stop") { - Task { try vm!.virtualMachine.requestStop() } - } - if #available(macOS 14, *) { - if (MainApp.suspendable) { - Button("Suspend") { - kill(getpid(), SIGUSR1) - } - } + MainApp.suspendable = suspendable + MainApp.capturesSystemKeys = captureSystemKeys + MainApp.main() + } +} + +struct MainApp: App { + static var suspendable: Bool = false + static var capturesSystemKeys: Bool = false + + @NSApplicationDelegateAdaptor private var appDelegate: MinimalMenuAppDelegate + + var body: some Scene { + WindowGroup(vm!.name) { + Group { + VMView(vm: vm!, capturesSystemKeys: MainApp.capturesSystemKeys).onAppear { + NSWindow.allowsAutomaticWindowTabbing = false + }.onDisappear { + let ret = kill(getpid(), MainApp.suspendable ? SIGUSR1 : SIGINT) + if ret != 0 { + // Fallback to the old termination method that doesn't + // propagate the cancellation to Task's in case graceful + // termination via kill(2) is not successful + NSApplication.shared.terminate(self) + } + } + }.frame( + minWidth: CGFloat(vm!.config.display.width), + idealWidth: CGFloat(vm!.config.display.width), + maxWidth: .infinity, + minHeight: CGFloat(vm!.config.display.height), + idealHeight: CGFloat(vm!.config.display.height), + maxHeight: .infinity + ) + }.commands { + // Remove some standard menu options + CommandGroup(replacing: .help, addition: {}) + CommandGroup(replacing: .newItem, addition: {}) + CommandGroup(replacing: .pasteboard, addition: {}) + CommandGroup(replacing: .textEditing, addition: {}) + CommandGroup(replacing: .undoRedo, addition: {}) + CommandGroup(replacing: .windowSize, addition: {}) + // Replace some standard menu options + CommandGroup(replacing: .appInfo) { AboutTart(config: vm!.config) } + CommandMenu("Control") { + Button("Start") { + Task { try await vm!.virtualMachine.start() } + } + Button("Stop") { + Task { try await vm!.virtualMachine.stop() } + } + Button("Request Stop") { + Task { try vm!.virtualMachine.requestStop() } + } + if #available(macOS 14, *) { + if (MainApp.suspendable) { + Button("Suspend") { + kill(getpid(), SIGUSR1) } } } } } - - MainApp.suspendable = suspendable - MainApp.capturesSystemKeys = captureSystemKeys - MainApp.main() } } @@ -640,6 +640,14 @@ class MinimalMenuAppDelegate: NSObject, NSApplicationDelegate, ObservableObject func applicationDidFinishLaunching(_ : Notification) { NSApplication.shared.mainMenu?.removeItem(at: indexOfEditMenu) } + + func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply { + if (kill(getpid(), MainApp.suspendable ? SIGUSR1 : SIGINT) == 0) { + return .terminateLater + } else { + return .terminateNow + } + } } struct AboutTart: View { From 43652de40a6ed4859fa14c559d636e9aa6fe9696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tor=20Arne=20Vestb=C3=B8?= Date: Mon, 29 Apr 2024 21:45:36 +0200 Subject: [PATCH 3/3] Activate Tart after application finishes launching This ensures that the VM window has been shown by the time we activate, so that we consistently activate and bring the VM window to the front. --- Sources/tart/Commands/Run.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/tart/Commands/Run.swift b/Sources/tart/Commands/Run.swift index 30efe8f2..3010aeed 100644 --- a/Sources/tart/Commands/Run.swift +++ b/Sources/tart/Commands/Run.swift @@ -563,10 +563,6 @@ struct Run: AsyncParsableCommand { } private func runUI(_ suspendable: Bool, _ captureSystemKeys: Bool) { - let nsApp = NSApplication.shared - nsApp.setActivationPolicy(.regular) - nsApp.activate(ignoringOtherApps: true) - MainApp.suspendable = suspendable MainApp.capturesSystemKeys = captureSystemKeys MainApp.main() @@ -639,6 +635,10 @@ class MinimalMenuAppDelegate: NSObject, NSApplicationDelegate, ObservableObject func applicationDidFinishLaunching(_ : Notification) { NSApplication.shared.mainMenu?.removeItem(at: indexOfEditMenu) + + let nsApp = NSApplication.shared + nsApp.setActivationPolicy(.regular) + nsApp.activate(ignoringOtherApps: true) } func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply {