Prevent ghost windows when quitting during site startup#3608
Open
ivan-ottinger wants to merge 1 commit into
Open
Prevent ghost windows when quitting during site startup#3608ivan-ottinger wants to merge 1 commit into
ivan-ottinger wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents “ghost” Studio windows from being re-created during shutdown by blocking late IPC fan-out to the renderer once the app has entered the will-quit phase. This fits into the Electron main-process lifecycle management by ensuring background async events can’t implicitly trigger getMainWindow() window creation during quit.
Changes:
- Add a module-level
isAppQuittinggate insendIpcEventToRenderer()plus an exportedmarkAppQuitting()setter. - Call
markAppQuitting()at the start of theapp.on('will-quit')handler so IPC sends no-op during the shutdown drain window.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/studio/src/ipc-utils.ts | Adds an app-quitting guard to prevent sendIpcEventToRenderer() from calling getMainWindow() during shutdown. |
| apps/studio/src/index.ts | Marks the app as quitting at the start of will-quit so late async IPC events don’t spawn new windows. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issues
How AI was used in this PR
Claude Code investigated the bug, identified the root cause (create-if-missing branch in
getMainWindow()racing with async IPC fan-out during thewill-quitshutdown window), proposed the minimal gate atsendIpcEventToRenderer, and applied the change. I reviewed the diff and confirmed both the fix and its placement before committing.Proposed Changes
isAppQuittingflag inapps/studio/src/ipc-utils.tswith an exportedmarkAppQuitting()setter. While set,sendIpcEventToRendererreturns early without callinggetMainWindow().markAppQuitting()at the start of thewill-quithandler inapps/studio/src/index.ts, before any other cleanup.Why this works
When the user confirms "Stop sites" on quit,
will-quitcallsevent.preventDefault()and waits up to 6 s forstopAllServers()to drain. During that window, in-flight async callbacks (site-startup events, snapshot events, sync progress, etc.) keep firing throughsendIpcEventToRenderer(). Each call routes throughgetMainWindow()— and because the original window has typically already been closed by Electron's shutdown sequence,getMainWindow()falls through tocreateMainWindow()and spawns a freshBrowserWindow. One pending event = one ghost window. With ~6 sites starting simultaneously on a Linux VM, you reliably see ~6 windows reopen.Blocking the fan-out at the
sendIpcEventToRendererchokepoint is the smallest fix: it catches every async-IPC path without changinggetMainWindow's signature or touching its 15+ other call sites (menus, deeplinks, update dialog, etc.), which are user-triggered and can't fire post-will-quitanyway.will-quit(notbefore-quit) is the right gate point:before-quitcan still be cancelled by the sync-in-progress dialog or the "Cancel" button on the running-sites dialog. Oncewill-quitfires, the app is committed.Regression origin
The mechanic was introduced in #86 (May 2024) for OAuth deep-link handling. It became today's user-visible bug in #849 (Jan 2025), which routed
sendIpcEventToRendererthroughgetMainWindow(). The bug got more reproducible recently due to Linux VM testing (slower shutdown widens the race window) plus PR #3350 removing an accidental escape hatch that bypassed the quit dialog when the CLI wasn't installed.Testing Instructions
Reproduce on
trunkfirst, then verify the fix on this branch.Repro on trunk (Linux/Windows easier; macOS rarer):
Verify on this branch:
Pre-merge Checklist