From 82a7ff7c3cc7dcad19a40ed58b223f4b16bb2190 Mon Sep 17 00:00:00 2001 From: Yixian HU Date: Sat, 11 Jul 2026 20:47:21 +0800 Subject: [PATCH] fix: prevent renderer heap leak and white-screen crashes Three independent root causes all produce unhandled Promise rejections that accumulate in the renderer heap and eventually exhaust it: 1. node-pty Windows backend: WindowsTerminal.kill(signal) throws "Signals not supported on windows." for any signal argument. Add killPtyProcess() helper in pty-handler.ts that catches the Windows signal error and falls back to kill() with no argument. Update isPtyAlreadyGoneError() in main/ipc/pty.ts to classify the Windows signal error as an already-gone PTY so it does not re-throw and propagate to the renderer as an unhandled rejection. 2. Uncaught pty.kill() calls in renderer: three call sites in repos.ts, pty-transport.ts fire-and-forget window.api.pty.kill() without .catch(). On SSH disconnect or Windows PTY teardown these produce unhandled rejections that the crash-diagnostics breadcrumb system retains in memory without releasing. Add .catch(() => {}) to each. 3. ResizeObserver loop: TerminalPane.scheduleSync ran syncPaneTitleOverlayRects inside a single requestAnimationFrame, which fires while the browser is still dispatching ResizeObserver notifications. The resulting React state update re-triggers the observer, producing a "ResizeObserver loop completed" error on every resize and potentially freezing the renderer thread. Switch to a double-rAF so the work defers to the next paint frame after the observation cycle completes. Also suppress the ResizeObserver loop ErrorEvent in crash-diagnostics.ts so that any that slip through do not accumulate in the breadcrumb buffer. Add .catch(() => callback(undefined)) to the Promise.all in terminal-link-handlers.ts so SSH-disconnect errors don't flood the unhandled-rejection count. Co-Authored-By: Claude --- src/main/ipc/pty.ts | 6 +++- src/relay/pty-handler.ts | 34 +++++++++++++++---- .../components/terminal-pane/TerminalPane.tsx | 12 +++++-- .../components/terminal-pane/pty-transport.ts | 4 +-- .../terminal-pane/terminal-link-handlers.ts | 3 ++ src/renderer/src/lib/crash-diagnostics.ts | 9 +++++ src/renderer/src/store/slices/repos.ts | 2 +- 7 files changed, 58 insertions(+), 12 deletions(-) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 4dcc451324a..2a961ad3c2d 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -476,7 +476,11 @@ function normalizeNodePtySpawnError(err: unknown): Error { function isPtyAlreadyGoneError(err: unknown): boolean { const message = err instanceof Error ? err.message : String(err) - return isSshPtyNotFoundError(err) || /Session not found/i.test(message) + return ( + isSshPtyNotFoundError(err) || + /Session not found/i.test(message) || + /Signals not supported on windows/i.test(message) + ) } function delay(ms: number): Promise { diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index 965d8cbdc30..b4448a52c50 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -25,6 +25,28 @@ import { type ShellReadyScanState } from '../main/shell-ready-marker-scanner' +// Why: node-pty WindowsTerminal.kill(signal) throws "Signals not supported on +// windows." when called with any argument. Use this helper everywhere instead of +// calling .kill(signal) directly so the call is safe across all PTY backends. +function killPtyProcess(pty: IPty, signal: string): void { + try { + pty.kill(signal) + } catch (err) { + const message = err instanceof Error ? err.message : String(err) + if (/signals not supported/i.test(message)) { + // Windows PTY backend doesn't support signals — kill without a signal arg + // to terminate the process as best the platform allows. + try { + pty.kill() + } catch { + // Already dead or disposed; safe to ignore. + } + return + } + throw err + } +} + // Why: node-pty is a native addon that may not be installed on the remote. // Dynamic import keeps the require() lazy so loadPty() returns null gracefully // when the native module is unavailable. The static type import lets vitest @@ -710,11 +732,11 @@ export class PtyHandler { // response is discarded and no renderer can own this PTY. Shut it down // immediately so it does not linger as an unreachable remote shell. this.releaseStartupCommand(managed) - term.kill('SIGTERM') + killPtyProcess(term, 'SIGTERM') managed.killTimer = setTimeout(() => { const still = this.ptys.get(id) if (still && !still.disposed) { - still.pty.kill('SIGKILL') + killPtyProcess(still.pty, 'SIGKILL') // Why: stale-spawn cleanup has no client who will ever attach. If // SIGKILL's onExit is missed (kernel edge case, uninterruptible // sleep), the managed entry + ptmx fd would leak forever. Dispose @@ -839,7 +861,7 @@ export class PtyHandler { if (immediate) { this.releaseStartupCommand(managed) this.flushPtyOutput(id) - managed.pty.kill('SIGKILL') + killPtyProcess(managed.pty, 'SIGKILL') // Why: SIGKILL has already reaped the child; release the ptmx fd on the // same tick. Deferring to onExit leaves a window where the fd is live // with a dead child. Idempotent via the disposed guard — if onExit fires @@ -858,7 +880,7 @@ export class PtyHandler { this.clearPtyFlowState(id) } else { this.releaseStartupCommand(managed) - managed.pty.kill('SIGTERM') + killPtyProcess(managed.pty, 'SIGTERM') // Why: Some processes ignore SIGTERM (e.g. a hung child, a custom signal // handler). Without a SIGKILL fallback the PTY process would leak and the @@ -872,7 +894,7 @@ export class PtyHandler { managed.killTimer = setTimeout(() => { const still = this.ptys.get(id) if (still && !still.disposed) { - still.pty.kill('SIGKILL') + killPtyProcess(still.pty, 'SIGKILL') this.flushPtyOutput(id) // Why: emit pty.exit BEFORE disposeManagedPty sets disposed=true. // The natural onExit short-circuits on `managed.disposed`, so @@ -907,7 +929,7 @@ export class PtyHandler { if (!managed || managed.disposed) { throw new Error(`PTY "${id}" not found`) } - managed.pty.kill(signal) + killPtyProcess(managed.pty, signal) } private async getCwd(params: Record): Promise { diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index a3d0286efac..009ddd64be5 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -2387,9 +2387,17 @@ export default function TerminalPane({ if (frame !== null) { cancelAnimationFrame(frame) } + // Why: a single rAF fires while the browser may still be dispatching + // ResizeObserver notifications, which causes syncPaneTitleOverlayRects to + // trigger a React state update that re-fires the observer — producing a + // "ResizeObserver loop completed with undelivered notifications" error and + // a potential freeze. The double-rAF defers work to the next paint frame, + // after the current observation cycle is fully complete. frame = requestAnimationFrame(() => { - frame = null - syncPaneTitleOverlayRects() + frame = requestAnimationFrame(() => { + frame = null + syncPaneTitleOverlayRects() + }) }) } diff --git a/src/renderer/src/components/terminal-pane/pty-transport.ts b/src/renderer/src/components/terminal-pane/pty-transport.ts index 60656f2907f..bfe45ad5a64 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.ts @@ -732,7 +732,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra // If destroyed while spawn was in flight, kill the new pty and bail if (destroyed) { - window.api.pty.kill(spawnResult.id) + window.api.pty.kill(spawnResult.id).catch(() => {}) return } @@ -918,7 +918,7 @@ export function createIpcPtyTransport(opts: IpcPtyTransportOptions = {}): PtyTra inputWriteQueue.clear() if (ptyId) { const id = ptyId - window.api.pty.kill(id) + window.api.pty.kill(id).catch(() => {}) connected = false ptyId = null unregisterPtyHandlers(id) diff --git a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts index 462dbdeeedc..a7bb69ecd75 100644 --- a/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts +++ b/src/renderer/src/components/terminal-pane/terminal-link-handlers.ts @@ -231,6 +231,9 @@ export function createFilePathLinkProvider( } callback(links.length > 0 ? links : undefined) }) + .catch(() => { + callback(undefined) + }) } } } diff --git a/src/renderer/src/lib/crash-diagnostics.ts b/src/renderer/src/lib/crash-diagnostics.ts index bb0875f1433..7d0033763e5 100644 --- a/src/renderer/src/lib/crash-diagnostics.ts +++ b/src/renderer/src/lib/crash-diagnostics.ts @@ -64,6 +64,15 @@ if (typeof import.meta !== 'undefined' && import.meta.hot) { } function recordRendererError(event: ErrorEvent): void { + // Why: ResizeObserver loop errors are a Chromium/Electron browser quirk fired + // when a ResizeObserver callback causes layout changes mid-dispatch. They are + // benign and self-resolving; recording them fills the breadcrumb buffer and + // inflates the error count without adding diagnostic value. Suppress them + // here so they don't contribute to memory growth or false-positive crash traces. + if (/ResizeObserver loop/i.test(event.message)) { + event.preventDefault() + return + } recordRendererCrashBreadcrumb( 'renderer_error', compactBreadcrumbData({ diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 4538b91212e..8265419a952 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -2814,7 +2814,7 @@ export const createRepoSlice: StateCreator = (set, for (const ptyId of get().ptyIdsByTabId[tab.id] ?? []) { killedPtyIds.add(ptyId) if (!ptyId.startsWith('remote:')) { - window.api.pty.kill(ptyId) + window.api.pty.kill(ptyId).catch(() => {}) } } }