From 88fc2eb4fc41bab82501d7ebb1ce9e795c52462d Mon Sep 17 00:00:00 2001 From: CIS2042 <3021399+cis2042@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:22:57 +0800 Subject: [PATCH] fix(window): stop "Object has been destroyed" crash from stale renderer-reply IPC after window close (#21581) Electron's BrowserWindow.webContents getter throws "Object has been destroyed" once the native window is gone. Several ipcMain reply listeners (terminal reveal, CLI/mobile terminal create, browser tab create, remote CLI scope) and the main window focus lifecycle read mainWindow.webContents on every event, so a late or replacement-window reply after close threw inside ipcMain.emit, crashing main and starving the new window's listener. Capture the renderer webContents once at setup and compare event.sender against that reference instead. Adds a regression test for a pending reveal on a closed window while a replacement window answers. Co-authored-by: Neil --- .../desktop-browser-bridge-renderer-scope.ts | 5 +- .../browser/browser-tab-renderer-create.ts | 6 +- .../orca-runtime-create-terminal-desktop.ts | 8 +- .../launch/mobile-terminal-renderer-create.ts | 9 +- .../window/main-window-focus-lifecycle.ts | 25 ++--- ...e-window-lifecycle-terminal-reveal.test.ts | 95 +++++++++++++++++++ src/main/window/runtime-window-lifecycle.ts | 2 +- 7 files changed, 125 insertions(+), 25 deletions(-) create mode 100644 src/main/window/runtime-window-lifecycle-terminal-reveal.test.ts diff --git a/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-renderer-scope.ts b/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-renderer-scope.ts index 0db6fb0b2ea..e0ce9de3d9c 100644 --- a/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-renderer-scope.ts +++ b/src/main/browser/desktop-browser-bridge/desktop-browser-bridge-renderer-scope.ts @@ -12,6 +12,7 @@ export function rendererResolveRemoteCliWorkspace( if (!win || win.isDestroyed()) { return Promise.resolve(null) } + const rendererWebContents = win.webContents const requestId = randomUUID() return new Promise((resolve) => { const finish = (workspaceId: string | null): void => { @@ -21,7 +22,7 @@ export function rendererResolveRemoteCliWorkspace( } const handler = (event: Electron.IpcMainEvent, reply: unknown): void => { if ( - event.sender !== win.webContents || + event.sender !== rendererWebContents || !reply || typeof reply !== 'object' || !('requestId' in reply) || @@ -37,6 +38,6 @@ export function rendererResolveRemoteCliWorkspace( } const timer = setTimeout(() => finish(null), SCOPE_REPLY_TIMEOUT_MS) ipcMain.on('browser:remoteCliScopeReply', handler) - win.webContents.send('browser:requestRemoteCliScope', { requestId, ...query }) + rendererWebContents.send('browser:requestRemoteCliScope', { requestId, ...query }) }) } diff --git a/src/main/runtime/browser/browser-tab-renderer-create.ts b/src/main/runtime/browser/browser-tab-renderer-create.ts index d466fce2429..9fadb1eae9d 100644 --- a/src/main/runtime/browser/browser-tab-renderer-create.ts +++ b/src/main/runtime/browser/browser-tab-renderer-create.ts @@ -11,7 +11,7 @@ export async function createBrowserTabInRenderer( activate?: boolean, requestedPageId?: string ): Promise<{ browserPageId: string }> { - const win = host.getAuthoritativeWindow() + const rendererWebContents = host.getAuthoritativeWindow().webContents const requestId = randomUUID() const browserPageId = await new Promise((resolve, reject) => { @@ -24,7 +24,7 @@ export async function createBrowserTabInRenderer( event: Electron.IpcMainEvent, reply: { requestId: string; browserPageId?: string; error?: string } ): void => { - if (event.sender !== win.webContents || reply.requestId !== requestId) { + if (event.sender !== rendererWebContents || reply.requestId !== requestId) { return } clearTimeout(timer) @@ -36,7 +36,7 @@ export async function createBrowserTabInRenderer( } } ipcMain.on('browser:tabCreateReply', handler) - win.webContents.send('browser:requestTabCreate', { + rendererWebContents.send('browser:requestTabCreate', { requestId, url, worktreeId, diff --git a/src/main/runtime/orca-runtime-create-terminal-desktop.ts b/src/main/runtime/orca-runtime-create-terminal-desktop.ts index 294918bc539..1cd5dde7ba2 100644 --- a/src/main/runtime/orca-runtime-create-terminal-desktop.ts +++ b/src/main/runtime/orca-runtime-create-terminal-desktop.ts @@ -21,7 +21,9 @@ export async function createDesktopTerminal( } opts = applyRemoteViewerLaunchEnv(opts, opts.terminalImageViewerSupported) runtime.assertGraphReady() - const win = runtime.getAuthoritativeWindow(NO_WORKSPACE_OR_WINDOW_MESSAGE) + const rendererWebContents = runtime.getAuthoritativeWindow( + NO_WORKSPACE_OR_WINDOW_MESSAGE + ).webContents const requestId = dependencies.randomUUID() const reply = await new Promise<{ tabId: string; title: string }>((resolve, reject) => { const timer = setTimeout(() => { @@ -37,7 +39,7 @@ export async function createDesktopTerminal( error?: string } ): void => { - if (event.sender !== win.webContents || response.requestId !== requestId) { + if (event.sender !== rendererWebContents || response.requestId !== requestId) { return } clearTimeout(timer) @@ -49,7 +51,7 @@ export async function createDesktopTerminal( } } dependencies.getRuntimeDesktopSurface().onIpc('terminal:tabCreateReply', handler) - win.webContents.send('terminal:requestTabCreate', { + rendererWebContents.send('terminal:requestTabCreate', { requestId, command: opts.command, cwd: opts.cwd, diff --git a/src/main/runtime/terminal/launch/mobile-terminal-renderer-create.ts b/src/main/runtime/terminal/launch/mobile-terminal-renderer-create.ts index 1de50d423fc..35f60a749ac 100644 --- a/src/main/runtime/terminal/launch/mobile-terminal-renderer-create.ts +++ b/src/main/runtime/terminal/launch/mobile-terminal-renderer-create.ts @@ -171,11 +171,12 @@ export abstract class OrcaRuntimeWithRunCreateMobileSessionTerminal extends Orca } ) } - if (win.webContents.isDestroyed?.()) { + const rendererWebContents = win.webContents + if (rendererWebContents.isDestroyed?.()) { throw new Error('runtime_unavailable') } const releasePublicationThrottle = pairedCreate - ? rendererPublicationThrottle.acquire(win.webContents) + ? rendererPublicationThrottle.acquire(rendererWebContents) : () => {} try { const requestId = randomUUID() @@ -196,7 +197,7 @@ export abstract class OrcaRuntimeWithRunCreateMobileSessionTerminal extends Orca event: IpcMainEvent, r: { requestId: string; tabId?: string; title?: string; error?: string } ): void => { - if (event.sender !== win.webContents || r.requestId !== requestId) { + if (event.sender !== rendererWebContents || r.requestId !== requestId) { return } clearTimeout(timer) @@ -210,7 +211,7 @@ export abstract class OrcaRuntimeWithRunCreateMobileSessionTerminal extends Orca } opts.signal?.addEventListener('abort', onAbort, { once: true }) getRuntimeDesktopSurface().onIpc('terminal:tabCreateReply', handler) - win.webContents.send('terminal:requestTabCreate', { + rendererWebContents.send('terminal:requestTabCreate', { requestId, worktreeId, afterTabId: afterDesktopTabId, diff --git a/src/main/window/main-window-focus-lifecycle.ts b/src/main/window/main-window-focus-lifecycle.ts index 17561906bcf..66127f9fefc 100644 --- a/src/main/window/main-window-focus-lifecycle.ts +++ b/src/main/window/main-window-focus-lifecycle.ts @@ -47,6 +47,8 @@ export function installMainWindowFocusLifecycle(args: { rendererWebContentsId: number }): MainWindowFocusLifecycle { const { isWindowClosing, mainWindow, opts, reloadMainWindow, rendererWebContentsId } = args + // Capture once: the BrowserWindow getter throws after close, but late IPC and webContents events can still arrive. + const rendererWebContents = mainWindow.webContents // Mirror TipTap focus before main shortcut resolution so bare Cmd/Ctrl+B remains bold (docs/markdown-cmd-b-bold-design.md). let markdownEditorFocused = false let terminalInputFocused = false @@ -58,7 +60,7 @@ export function installMainWindowFocusLifecycle(args: { const markdownFocusChannel = 'ui:setMarkdownEditorFocused' // Check sender and strict boolean so guests/malformed payloads cannot disable sidebar shortcuts. const onMarkdownEditorFocused = (event: Electron.IpcMainEvent, focused: unknown): void => { - if (event.sender !== mainWindow.webContents) { + if (event.sender !== rendererWebContents) { return } markdownEditorFocused = focused === true @@ -67,7 +69,7 @@ export function installMainWindowFocusLifecycle(args: { const terminalInputFocusChannel = 'ui:setTerminalInputFocused' // Main resolves before renderer keydown; mirror xterm focus so Terminal-first leaves shell chords alone. const onTerminalInputFocused = (event: Electron.IpcMainEvent, focused: unknown): void => { - if (event.sender !== mainWindow.webContents) { + if (event.sender !== rendererWebContents) { return } terminalInputFocused = focused === true @@ -76,7 +78,7 @@ export function installMainWindowFocusLifecycle(args: { const floatingFocusChannel = 'ui:setFloatingFocus' // Publish both focus bits atomically; terminal selects keybinding context and implies panel routing ownership. const onFloatingFocus = (event: Electron.IpcMainEvent, state: unknown): void => { - if (event.sender !== mainWindow.webContents) { + if (event.sender !== rendererWebContents) { return } const payload = (state ?? {}) as { panelFocused?: unknown; terminalFocused?: unknown } @@ -89,7 +91,7 @@ export function installMainWindowFocusLifecycle(args: { const shortcutRecorderFocusChannel = 'ui:setShortcutRecorderFocused' // Shortcut recording must receive the keys before main consumes their existing bindings. const onShortcutRecorderFocused = (event: Electron.IpcMainEvent, focused: unknown): void => { - if (event.sender !== mainWindow.webContents) { + if (event.sender !== rendererWebContents) { return } shortcutRecorderFocused = focused === true @@ -98,7 +100,7 @@ export function installMainWindowFocusLifecycle(args: { let pendingRichMarkdownContextMenuTableTarget: RichMarkdownContextMenuTableTarget | null = null const onRichMarkdownContextMenuTarget = (event: Electron.IpcMainEvent, value: unknown): void => { - if (event.sender !== mainWindow.webContents) { + if (event.sender !== rendererWebContents) { return } pendingRichMarkdownContextMenuTableTarget = parseRichMarkdownContextMenuTableTarget(value) @@ -107,7 +109,7 @@ export function installMainWindowFocusLifecycle(args: { // Renderer sends this on the same frame pipe immediately before the next Chromium context menu. let pendingPlainTextContextMenuTarget = false const onPlainTextContextMenuTarget = (event: Electron.IpcMainEvent): void => { - if (event.sender === mainWindow.webContents) { + if (event.sender === rendererWebContents) { pendingPlainTextContextMenuTarget = true } } @@ -120,7 +122,7 @@ export function installMainWindowFocusLifecycle(args: { const plainText = pendingPlainTextContextMenuTarget pendingRichMarkdownContextMenuTableTarget = null pendingPlainTextContextMenuTarget = false - const template = buildEditableContextMenuTemplate(params, mainWindow.webContents, { + const template = buildEditableContextMenuTemplate(params, rendererWebContents, { tableTarget, plainText }) @@ -130,7 +132,7 @@ export function installMainWindowFocusLifecycle(args: { // Context menu may precede focus mirroring; trust Electron's editable params. Menu.buildFromTemplate(template).popup({ window: mainWindow, x: params.x, y: params.y }) } - mainWindow.webContents.on('context-menu', onMainContextMenu) + rendererWebContents.on('context-menu', onMainContextMenu) // Dead renderers cannot clear focus mirrors; reset carve-outs so later lifecycles retain app shortcuts. const resetMarkdownEditorFocus = (): void => { @@ -149,7 +151,6 @@ export function installMainWindowFocusLifecycle(args: { shortcutRecorderFocused = false } let rendererProcessGone = false - const rendererWebContents = mainWindow.webContents registerRendererDocumentNavigation(rendererWebContents, () => { const frame = rendererWebContents.mainFrame retireBrowserClientPageRenderer(rendererWebContents) @@ -256,7 +257,7 @@ export function installMainWindowFocusLifecycle(args: { }, launchRetryDelayMs ?? 250) } const rendererLiveness = trackRendererLivenessAtDeath(mainWindow) - mainWindow.webContents.on('render-process-gone', (_event, details) => { + rendererWebContents.on('render-process-gone', (_event, details) => { const liveness = rendererLiveness.takeAtRendererGone() rendererProcessGone = true retireBrowserClientPageRenderer(rendererWebContents) @@ -275,14 +276,14 @@ export function installMainWindowFocusLifecycle(args: { } scheduleRendererRecovery(details) }) - mainWindow.webContents.on('destroyed', () => { + rendererWebContents.on('destroyed', () => { retireBrowserClientPageRenderer(rendererWebContents) resetMarkdownEditorFocus() resetTerminalInputFocus() resetFloatingTerminalInputFocus() resetShortcutRecorderFocus() }) - mainWindow.webContents.on('did-finish-load', () => { + rendererWebContents.on('did-finish-load', () => { rendererProcessGone = false attachBrowserClientPageRenderer(rendererWebContents) clearRendererRecoveryTimer() diff --git a/src/main/window/runtime-window-lifecycle-terminal-reveal.test.ts b/src/main/window/runtime-window-lifecycle-terminal-reveal.test.ts new file mode 100644 index 00000000000..66bf50fce85 --- /dev/null +++ b/src/main/window/runtime-window-lifecycle-terminal-reveal.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it, vi } from 'vitest' +import type { BrowserWindow } from 'electron' +import type { OrcaRuntimeService } from '../runtime/orca-runtime' +import type { RuntimeNotifier } from '../runtime/runtime-notifier-contract' + +const { ipcMain } = await vi.hoisted(async () => { + const { EventEmitter } = await import('node:events') + return { ipcMain: new EventEmitter() } +}) +vi.mock('electron', () => ({ ipcMain })) +vi.mock('../ipc/worktree-change-invalidators', () => ({ runWorktreeChangeInvalidators: vi.fn() })) +vi.mock('./renderer-document-navigation', () => ({ registerRendererDocumentNavigation: vi.fn() })) + +import { registerRuntimeWindowLifecycle } from './runtime-window-lifecycle' + +function attachWindow(id: number): { + destroy: () => void + notifier: RuntimeNotifier + send: ReturnType + webContents: object +} { + let destroyed = false + const send = vi.fn() + const webContents = { isDestroyed: () => false, send, on: vi.fn() } + const attached: { notifier: RuntimeNotifier | null } = { notifier: null } + const mainWindow = { + id, + isDestroyed: () => destroyed, + on: vi.fn(), + // Electron's BrowserWindow getter throws once the native window is gone. + get webContents() { + if (destroyed) { + throw new TypeError('Object has been destroyed') + } + return webContents + } + } + const runtime = { + attachWindow: vi.fn(), + markGraphReloadFailed: vi.fn(), + setNotifier: (next: RuntimeNotifier | null) => { + attached.notifier = next + } + } + registerRuntimeWindowLifecycle( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: registration only reads id, isDestroyed, on and webContents. + mainWindow as unknown as BrowserWindow, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: registration only calls the stubbed runtime members. + runtime as unknown as OrcaRuntimeService + ) + if (!attached.notifier) { + throw new Error('runtime notifier was not attached') + } + return { + destroy: () => { + destroyed = true + }, + notifier: attached.notifier, + send, + webContents + } +} + +describe('runtime window terminal reveal', () => { + it('lets a replacement window answer while a closed window still has a pending reveal', async () => { + vi.useFakeTimers() + try { + const closedWindow = attachWindow(1) + const stale = Promise.resolve( + closedWindow.notifier.revealTerminalSession?.('wt-1', { ptyId: 'pty-1' }) + ) + const staleSettled = expect(stale).rejects.toThrow('Terminal reveal timed out') + closedWindow.destroy() + + const nextWindow = attachWindow(2) + const reveal = nextWindow.notifier.revealTerminalSession?.('wt-1', { ptyId: 'pty-2' }) + const [, payload] = nextWindow.send.mock.calls[0] + + expect(() => + ipcMain.emit( + 'terminal:tabCreateReply', + { sender: nextWindow.webContents }, + { requestId: payload.requestId, tabId: 'tab-2' } + ) + ).not.toThrow() + await expect(reveal).resolves.toEqual({ tabId: 'tab-2', title: undefined }) + + await vi.advanceTimersByTimeAsync(10_000) + await staleSettled + expect(ipcMain.listenerCount('terminal:tabCreateReply')).toBe(0) + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/main/window/runtime-window-lifecycle.ts b/src/main/window/runtime-window-lifecycle.ts index 6b0a516baf9..942fa1b64b3 100644 --- a/src/main/window/runtime-window-lifecycle.ts +++ b/src/main/window/runtime-window-lifecycle.ts @@ -90,7 +90,7 @@ export function registerRuntimeWindowLifecycle( }, 10_000) const handler = (event: Electron.IpcMainEvent, reply: TerminalTabCreateReply): void => { // Renderer-supplied request IDs confer no authority; only this window can satisfy reveal. - if (event.sender !== mainWindow.webContents || reply.requestId !== requestId) { + if (event.sender !== mainWebContents || reply.requestId !== requestId) { return } clearTimeout(timer)