Skip to content

fix: prevent zombie instances on app restart #23757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jasonnnz merged 3 commits into from
Apr 6, 2026
Merged

fix: prevent zombie instances on app restart #23757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0dcc6e2
fix: prevent zombie instances on app restart
Apr 6, 2026
80716e2
fix: reset isRestarting flag on relaunch failure
Apr 6, 2026
4becbb7
fix: reconnect connectionManager on restart relaunch failure
Apr 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions clients/macos/vellum-assistant/App/AppDelegate+AuthLifecycle.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,16 @@ extension AppDelegate {
}

@objc func performRestart() {
isRestarting = true
Comment thread
Jasonnnz marked this conversation as resolved.
let bundleURL = Bundle.main.bundleURL

// Disconnect SSE and health checks *before* the CLI kills the
// daemon/gateway. Otherwise the health check detects the daemon
// dying, triggers autoWakeIfAssistantDied(), and wakes the daemon
// right back up — fighting with the shutdown. (Same pattern as
// performRetireAsync().)
connectionManager.disconnect()

// Write a timestamped sentinel so the new instance's single-instance
// guard knows this is an intentional restart, not a duplicate launch.
// The sentinel contains the current Unix timestamp; the new instance
Expand All @@ -165,6 +173,13 @@ extension AppDelegate {
// Clean up the sentinel so a failed restart doesn't leave
// a file that could bypass the guard on the next launch.
try? FileManager.default.removeItem(at: sentinelPath)
self?.isRestarting = false
// Reconnect SSE and health checks so the app doesn't stay
// in a disconnected state after a failed relaunch attempt.
// (Same pattern as performRetireAsync()'s cancel path.)
Task { @MainActor [weak self] in
try? await self?.connectionManager.connect()
}
Comment on lines +176 to +182
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 isRestarting = false written off the main actor in NSWorkspace completion handler

AppDelegate is @MainActor, so isRestarting is main-actor-isolated. However, in the error path of performRestart(), the NSWorkspace.shared.openApplication completion handler runs on a background serial queue (not the main actor). The assignment self?.isRestarting = false at line 176 writes this main-actor-isolated property from a non-isolated context — a data race.

On Apple Silicon (ARM64, weak memory model), the write may never become visible to the main actor. If the restart fails and isRestarting remains stale true, a subsequent app quit triggers applicationShouldTerminate (AppDelegate.swift:753) which reads isRestarting, sees true, and returns .terminateNow — bypassing the vellumCli.stop() cleanup. The reconnection on line 180-182 is correctly wrapped in Task { @MainActor ... } but the flag reset is not.

Suggested change
self?.isRestarting = false
// Reconnect SSE and health checks so the app doesn't stay
// in a disconnected state after a failed relaunch attempt.
// (Same pattern as performRetireAsync()'s cancel path.)
Task { @MainActor [weak self] in
try? await self?.connectionManager.connect()
}
Task { @MainActor [weak self] in
self?.isRestarting = false
// Reconnect SSE and health checks so the app doesn't stay
// in a disconnected state after a failed relaunch attempt.
// (Same pattern as performRetireAsync()'s cancel path.)
try? await self?.connectionManager.connect()
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

return
Comment thread
Jasonnnz marked this conversation as resolved.
}
Task { @MainActor [weak self] in
Expand Down
9 changes: 9 additions & 0 deletions clients/macos/vellum-assistant/App/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public final class AppDelegate: NSObject, NSApplicationDelegate {
var lastRegisteredQuickInputHotkey: String?
var globalHotkeyObserver: AnyCancellable?
var escapeMonitor: Any?
var isRestarting = false
var hasSetupHotKey = false
var fnVGlobalMonitor: Any?
var fnVLocalMonitor: Any?
Expand Down Expand Up @@ -745,6 +746,14 @@ public final class AppDelegate: NSObject, NSApplicationDelegate {
///
/// Reference: https://developer.apple.com/documentation/appkit/nsapplicationdelegate/applicationshouldterminate(_:)
public func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply {
// During a restart, performRestart() already stopped the CLI and
// disconnected the connection manager. Skip the async
// .terminateLater path whose MainActor.run dispatch is fragile
// during AppKit shutdown and can leave the process as a zombie.
if isRestarting {
return .terminateNow
}

let cli = vellumCli
Task.detached {
await cli.stop()
Expand Down