mirror of
https://github.com/stablyai/orca.git
synced 2026-10-30 16:47:34 +00:00
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 <neil@stably.ai>
This commit is contained in:
@@ -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<string | null>((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 })
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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<string>((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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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<typeof vi.fn>
|
||||
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()
|
||||
}
|
||||
})
|
||||
})
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user