From f68001da18a8c25965e4937512e30ad057a0a28a Mon Sep 17 00:00:00 2001 From: ciregenz Date: Mon, 22 Jun 2026 04:07:06 -0700 Subject: [PATCH] [eric] browser: clean CDP detach before card teardown + drop child-iframe Network (DevTools SIGSEGV) --- electron/main.js | 46 ++++++++++++++++++- electron/preload.js | 1 + .../app/pages/Dashboard/cards/BrowserCard.tsx | 6 +-- .../interaction/useDashboardShortcuts.ts | 5 +- frontend/src/shared/browserRegistry.ts | 8 ++++ frontend/src/shared/browserTeardown.ts | 44 ++++++++++++++++++ 6 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 frontend/src/shared/browserTeardown.ts diff --git a/electron/main.js b/electron/main.js index f2d8bc03..9b8405e2 100644 --- a/electron/main.js +++ b/electron/main.js @@ -2109,6 +2109,7 @@ app.on('web-contents-created', (_event, contents) => { cdpAutoAttachWired.delete(contents.id); cdpRoutesByWcId.delete(contents.id); webviewConsoleErrors.delete(contents.id); + cdpTearingDown.delete(contents.id); }); contents.on('render-process-gone', () => { @@ -2118,6 +2119,7 @@ app.on('web-contents-created', (_event, contents) => { cdpAutoAttachWired.delete(contents.id); cdpRoutesByWcId.delete(contents.id); webviewConsoleErrors.delete(contents.id); + cdpTearingDown.delete(contents.id); }); // A heavy SPA can HANG the renderer without crashing it (a render-process-gone @@ -2614,6 +2616,10 @@ const cdpChildSessions = new Map(); // wcId -> Map Map (tier-2 shadow-API capture) const webviewConsoleErrors = new Map(); // wcId -> [{level,message,source,line}] capped warn+error, read via BrowserGetConsole +// wcIds whose CDP is being cleanly detached on the way to destruction. Blocks a +// late agent command from RE-attaching (and re-enabling Network/auto-attach) as +// the webview tears down, which would re-arm the freed-DevToolsSession SIGSEGV. +const cdpTearingDown = new Set(); function wireChildSessions(wc) { const wcId = wc.id; @@ -2643,11 +2649,14 @@ function wireChildSessions(wc) { parentSessionId: sessionId || null, url: info.url || '', }); - // Enable perception + network domains and propagate auto-attach into nested OOPIF. + // Enable perception domains on the child + propagate auto-attach into nested OOPIF. + // Deliberately NO Network.enable here: a child iframe churns constantly, and a + // Network notification arriving after the child detaches lands on a freed session + // and SIGSEGVs the browser process (the mid-browse crash). Root Network still + // captures the page's own routes; we only forgo transient child-iframe routes. const sid = params.sessionId; wc.debugger.sendCommand('Accessibility.enable', {}, sid).catch(() => {}); wc.debugger.sendCommand('DOM.enable', {}, sid).catch(() => {}); - wc.debugger.sendCommand('Network.enable', {}, sid).catch(() => {}); wc.debugger.sendCommand('Target.setAutoAttach', { autoAttach: true, waitForDebuggerOnStart: false, flatten: true }, sid).catch(() => {}); } else if (method === 'Target.detachedFromTarget') { @@ -2666,6 +2675,11 @@ async function ensureDebuggerAttached(wc) { if (!wc || wc.isDestroyed()) { throw new Error('webContents is destroyed'); } + // Once a clean teardown has started, never re-attach: a re-attach here would + // re-enable Network + auto-attach right as the session is being freed. + if (cdpTearingDown.has(wc.id)) { + throw new Error('webContents is tearing down'); + } if (wc.debugger.isAttached()) return; try { wc.debugger.attach('1.3'); @@ -2687,6 +2701,25 @@ async function ensureDebuggerAttached(wc) { } catch (_) {} } +// Cleanly tear the DevTools session down BEFORE the webContents is destroyed: +// turn off the two churn-prone domains (auto-attach, Network) so no child +// sessions or network observers are live when Chromium frees the session, then +// detach. Without this, a notification in the mojo pipe lands on a freed +// DevToolsSession on the browser main thread and SIGSEGVs the whole app. +// Bounded + fail-open: a wedged pipe must never block the card from closing. +async function detachCdpCleanly(wc) { + if (!wc || wc.isDestroyed()) return; + cdpTearingDown.add(wc.id); + let attached = false; + try { attached = wc.debugger.isAttached(); } catch (_) { return; } + if (!attached) return; + const drain = (method, params) => + raceCdp(wc.debugger.sendCommand(method, params || {}), 1200, method).catch(() => {}); + await drain('Target.setAutoAttach', { autoAttach: false, waitForDebuggerOnStart: false, flatten: true }); + await drain('Network.disable', {}); + try { wc.debugger.detach(); } catch (_) { /* already detached / gone */ } +} + // debugger.sendCommand can hang FOREVER when the target's pipe breaks without // a detach event (renderer process swap, wedged guest). Unraced, one hung call // poisons the per-card queue and every later command "times out" while @@ -2751,6 +2784,15 @@ ipcMain.handle('send-cdp-command', async (_event, wcId, method, params, sessionI } }); +// Called by the renderer right before a browser card unmounts, so its CDP +// session is drained + detached while the webContents is still alive. +ipcMain.handle('cdp-detach-clean', async (_event, wcId) => { + try { + await detachCdpCleanly(getWebContentsById(wcId)); + } catch (_) { /* fail-open: never block the card's teardown */ } + return { ok: true }; +}); + // Renderer-side AX index cache helpers — the renderer stores its own copy // keyed by (browser_id, tab_id). The main process only stores per-wcId for // invalidation purposes. diff --git a/electron/preload.js b/electron/preload.js index ac4a3e13..9bb2010c 100644 --- a/electron/preload.js +++ b/electron/preload.js @@ -59,6 +59,7 @@ contextBridge.exposeInMainWorld('openswarm', { hardReset: () => ipcRenderer.invoke('hard-reset'), connectSlack: () => ipcRenderer.invoke('connect-slack'), sendCdpCommand: (wcId, method, params, sessionId) => ipcRenderer.invoke('send-cdp-command', wcId, method, params, sessionId), + cdpDetachClean: (wcId) => ipcRenderer.invoke('cdp-detach-clean', wcId), cdpCacheSet: (wcId, indexMap) => ipcRenderer.invoke('cdp-cache-set', wcId, indexMap), cdpCacheGet: (wcId) => ipcRenderer.invoke('cdp-cache-get', wcId), cdpCacheClear: (wcId) => ipcRenderer.invoke('cdp-cache-clear', wcId), diff --git a/frontend/src/app/pages/Dashboard/cards/BrowserCard.tsx b/frontend/src/app/pages/Dashboard/cards/BrowserCard.tsx index 76b655b1..807cfab9 100644 --- a/frontend/src/app/pages/Dashboard/cards/BrowserCard.tsx +++ b/frontend/src/app/pages/Dashboard/cards/BrowserCard.tsx @@ -24,7 +24,6 @@ import SmartToyOutlinedIcon from '@mui/icons-material/SmartToyOutlined'; import { setBrowserCardPosition, setBrowserCardSize, - removeBrowserCard, resumeBrowserCard, cancelBrowserCardEnding, addBrowserTab, @@ -36,6 +35,7 @@ import { reorderBrowserTab, type BrowserTab, } from '@/shared/state/dashboardLayoutSlice'; +import { removeBrowserCardCleanly } from '@/shared/browserTeardown'; import { createSelector } from '@reduxjs/toolkit'; import { useAppDispatch, useAppSelector } from '@/shared/hooks'; import { handleApproval } from '@/shared/state/agentsSlice'; @@ -278,7 +278,7 @@ const BrowserCard: React.FC = ({ useEffect(() => { if (!endingState) return; const timer = setTimeout(() => { - dispatch(removeBrowserCard(browserId)); + removeBrowserCardCleanly(browserId, dispatch); }, 3000); return () => clearTimeout(timer); }, [endingState, browserId, dispatch]); @@ -464,7 +464,7 @@ const BrowserCard: React.FC = ({ const handleRemove = useCallback((e: React.MouseEvent) => { e.stopPropagation(); - dispatch(removeBrowserCard(browserId)); + removeBrowserCardCleanly(browserId, dispatch); }, [dispatch, browserId]); const handleAddTab = useCallback((e: React.MouseEvent) => { diff --git a/frontend/src/app/pages/Dashboard/hooks/interaction/useDashboardShortcuts.ts b/frontend/src/app/pages/Dashboard/hooks/interaction/useDashboardShortcuts.ts index 401c8b35..a195674d 100644 --- a/frontend/src/app/pages/Dashboard/hooks/interaction/useDashboardShortcuts.ts +++ b/frontend/src/app/pages/Dashboard/hooks/interaction/useDashboardShortcuts.ts @@ -2,7 +2,8 @@ import { useEffect, type Dispatch, type SetStateAction } from 'react'; import { report } from '@/shared/serviceClient'; import { useAppDispatch } from '@/shared/hooks'; import { closeSession, toggleExpandSession } from '@/shared/state/agentsSlice'; -import { removeViewCard, removeBrowserCard, removeNote } from '@/shared/state/dashboardLayoutSlice'; +import { removeViewCard, removeNote } from '@/shared/state/dashboardLayoutSlice'; +import { removeBrowserCardCleanly } from '@/shared/browserTeardown'; import type { useDashboardSelection } from '../state/useDashboardSelection'; type Selection = ReturnType; @@ -76,7 +77,7 @@ export function useDashboardShortcuts({ } else if (type === 'view') { dispatch(removeViewCard(id)); } else if (type === 'browser') { - dispatch(removeBrowserCard(id)); + removeBrowserCardCleanly(id, dispatch); } else if (type === 'note') { dispatch(removeNote(id)); } diff --git a/frontend/src/shared/browserRegistry.ts b/frontend/src/shared/browserRegistry.ts index 8a03e481..8e162770 100644 --- a/frontend/src/shared/browserRegistry.ts +++ b/frontend/src/shared/browserRegistry.ts @@ -57,6 +57,14 @@ export function getWebview(browserId: string, tabId?: string): BrowserWebview | return registry.get(makeKey(browserId, resolvedTabId)); } +export function getBrowserWebviews(browserId: string): BrowserWebview[] { + const out: BrowserWebview[] = []; + for (const [key, wv] of registry.entries()) { + if (key.split(':')[0] === browserId) out.push(wv); + } + return out; +} + export function findBrowserByWebContentsId(wcId: number): string | undefined { for (const [key, wv] of registry.entries()) { if ((wv as any).getWebContentsId?.() === wcId) { diff --git a/frontend/src/shared/browserTeardown.ts b/frontend/src/shared/browserTeardown.ts new file mode 100644 index 00000000..61f6000f --- /dev/null +++ b/frontend/src/shared/browserTeardown.ts @@ -0,0 +1,44 @@ +import type { Dispatch } from '@reduxjs/toolkit'; +import { removeBrowserCard } from '@/shared/state/dashboardLayoutSlice'; +import { getBrowserWebviews } from '@/shared/browserRegistry'; + +interface CdpBridge { + cdpDetachClean?: (wcId: number) => Promise; +} + +// A wedged CDP pipe must never hold a card open; cap the whole detach round-trip. +const DETACH_BUDGET_MS = 600; + +// Detach the CDP debugger from every webview of a browser card BEFORE React +// unmounts them. Otherwise a late DevTools notification (a Target child-session +// message or Network.responseReceivedExtraInfo) lands on a session Chromium has +// already freed and SIGSEGVs the whole browser process. Bounded + fail-open. +export async function detachBrowserCdp(browserId: string): Promise { + const ow = (window as unknown as { openswarm?: CdpBridge }).openswarm; + if (!ow?.cdpDetachClean) return; + const detaches: Promise[] = []; + for (const wv of getBrowserWebviews(browserId)) { + try { + detaches.push(ow.cdpDetachClean(wv.getWebContentsId()).catch(() => {})); + } catch { + // webview already torn down; nothing to detach + } + } + if (!detaches.length) return; + await Promise.race([ + Promise.allSettled(detaches), + new Promise((resolve) => setTimeout(resolve, DETACH_BUDGET_MS)), + ]); +} + +// Clean-detach a browser card's CDP, THEN remove it. All three card-removal +// paths (the X button, the agent-finish timer, and keyboard delete) route +// through here so none of them tears the webview down with the debugger still +// attached. The detach is bounded, so removal is never blocked by a dead pipe. +export async function removeBrowserCardCleanly( + browserId: string, + dispatch: Dispatch, +): Promise { + await detachBrowserCdp(browserId); + dispatch(removeBrowserCard(browserId)); +}