Skip to content

Prevent ghost windows when quitting during site startup#3608

Open
ivan-ottinger wants to merge 1 commit into
trunkfrom
stu-1750-prevent-ghost-windows-on-quit
Open

Prevent ghost windows when quitting during site startup#3608
ivan-ottinger wants to merge 1 commit into
trunkfrom
stu-1750-prevent-ghost-windows-on-quit

Conversation

@ivan-ottinger
Copy link
Copy Markdown
Contributor

@ivan-ottinger ivan-ottinger commented May 25, 2026

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 the will-quit shutdown window), proposed the minimal gate at sendIpcEventToRenderer, and applied the change. I reviewed the diff and confirmed both the fix and its placement before committing.

Proposed Changes

  • Add an isAppQuitting flag in apps/studio/src/ipc-utils.ts with an exported markAppQuitting() setter. While set, sendIpcEventToRenderer returns early without calling getMainWindow().
  • Call markAppQuitting() at the start of the will-quit handler in apps/studio/src/index.ts, before any other cleanup.

Why this works

When the user confirms "Stop sites" on quit, will-quit calls event.preventDefault() and waits up to 6 s for stopAllServers() to drain. During that window, in-flight async callbacks (site-startup events, snapshot events, sync progress, etc.) keep firing through sendIpcEventToRenderer(). Each call routes through getMainWindow() — and because the original window has typically already been closed by Electron's shutdown sequence, getMainWindow() falls through to createMainWindow() and spawns a fresh BrowserWindow. 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 sendIpcEventToRenderer chokepoint is the smallest fix: it catches every async-IPC path without changing getMainWindow's signature or touching its 15+ other call sites (menus, deeplinks, update dialog, etc.), which are user-triggered and can't fire post-will-quit anyway.

will-quit (not before-quit) is the right gate point: before-quit can still be cancelled by the sync-in-progress dialog or the "Cancel" button on the running-sites dialog. Once will-quit fires, 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 sendIpcEventToRenderer through getMainWindow(). 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 trunk first, then verify the fix on this branch.

Repro on trunk (Linux/Windows easier; macOS rarer):

  1. Have several Studio sites created (~6 makes it obvious).
  2. Quit Studio so all sites stop.
  3. Re-launch Studio.
  4. Start all sites at once.
  5. Before they finish booting, quit Studio.
  6. Observe: Studio reopens 4–6 times in quick succession.

Verify on this branch:

  1. Repeat steps 1–5 above.
  2. Studio should quit cleanly with no ghost windows.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
  • Manual repro on Linux (with several sites)
  • Manual repro on Windows (with several sites)

@ivan-ottinger ivan-ottinger marked this pull request as ready for review May 25, 2026 11:34
@ivan-ottinger ivan-ottinger requested a review from Copilot May 25, 2026 11:34
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 isAppQuitting gate in sendIpcEventToRenderer() plus an exported markAppQuitting() setter.
  • Call markAppQuitting() at the start of the app.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.

@ivan-ottinger ivan-ottinger requested a review from a team May 25, 2026 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants