From 115706aa539378e61f42511d056bb0488810cf82 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Tue, 15 Sep 2026 12:22:56 -0700 Subject: [PATCH] fix(web): release xterm WebGL contexts on reconnect instead of dropping the renderer Follow-up to the salvaged #111918. That commit fixed the WebGL context pile-up by removing the WebGL renderer altogether. With @xterm/xterm 6 the fallback is the DOM renderer, so wide layouts would have lost the crisp, fast rendering the renderer split in 63975aa deliberately gave them, and the salvaged comment ("default canvas renderer") described a renderer that no longer exists. The leak itself is one missing call: @xterm/addon-webgl 0.19 removes its canvas on dispose but never calls WEBGL_lose_context.loseContext(), so every PTY reconnect (which rebuilds the Terminal) leaves a live GL context until GC. Browsers cap live contexts at ~16 and force-lose the oldest, so a reconnect storm eventually blanks the terminal the user is looking at. `loseWebglContexts(host)` runs in the terminal effect's cleanup before `term.dispose()` and releases every WebGL context under the host; WebGL stays on for wide layouts exactly as before. The keepalive is no longer gated on tab visibility / chat activity: an open PTY socket on a hidden or backgrounded tab still owns its PTY and is the one most likely to sit idle through a proxy timeout, and the frame is ~20 bytes. That removes the `shouldSendPtyKeepalive` helper and its two tests; the socket-open check already lives in `sendTerminalResize`. The ChatPage test drops its "no WebglAddon constructed" assertion, which passed on base too (jsdom hosts are 0px wide, so the wide-layout WebGL branch never ran). --- web/src/lib/pty-reconnect.test.ts | 32 ----------------- web/src/lib/pty-reconnect.ts | 10 ------ web/src/lib/xterm-webgl-release.test.ts | 26 ++++++++++++++ web/src/lib/xterm-webgl-release.ts | 27 +++++++++++++++ web/src/pages/ChatPage.test.tsx | 9 +---- web/src/pages/ChatPage.tsx | 46 ++++++++++++++++--------- 6 files changed, 84 insertions(+), 66 deletions(-) create mode 100644 web/src/lib/xterm-webgl-release.test.ts create mode 100644 web/src/lib/xterm-webgl-release.ts diff --git a/web/src/lib/pty-reconnect.test.ts b/web/src/lib/pty-reconnect.test.ts index 56202eb4b3..995076d41b 100644 --- a/web/src/lib/pty-reconnect.test.ts +++ b/web/src/lib/pty-reconnect.test.ts @@ -2,12 +2,10 @@ import { describe, expect, it } from 'vitest' import { PTY_RECONNECT_BASE_MS, - PTY_KEEPALIVE_INTERVAL_MS, PTY_RECONNECT_MAX_ATTEMPTS, PTY_RECONNECT_MAX_MS, ptyReconnectDelayMs, shouldBlockPtyInput, - shouldSendPtyKeepalive, shouldReconnectPtyOnPageResume } from './pty-reconnect' @@ -141,36 +139,6 @@ describe('shouldBlockPtyInput', () => { }) }) -describe('shouldSendPtyKeepalive', () => { - it('keeps an active, visible PTY socket alive every 20 seconds', () => { - expect(PTY_KEEPALIVE_INTERVAL_MS).toBe(20_000) - expect( - shouldSendPtyKeepalive({ - isActive: true, - visibilityState: 'visible', - socketReadyState: 1 - }) - ).toBe(true) - }) - - it('does not send control traffic from hidden tabs or closed sockets', () => { - expect( - shouldSendPtyKeepalive({ - isActive: true, - visibilityState: 'hidden', - socketReadyState: 1 - }) - ).toBe(false) - expect( - shouldSendPtyKeepalive({ - isActive: true, - visibilityState: 'visible', - socketReadyState: 3 - }) - ).toBe(false) - }) -}) - describe('ptyReconnectDelayMs', () => { it('doubles from the base on each attempt and clamps at the cap', () => { expect(Array.from({ length: PTY_RECONNECT_MAX_ATTEMPTS }, (_, i) => ptyReconnectDelayMs(i + 1))).toEqual([ diff --git a/web/src/lib/pty-reconnect.ts b/web/src/lib/pty-reconnect.ts index da592461ba..61f205d0f4 100644 --- a/web/src/lib/pty-reconnect.ts +++ b/web/src/lib/pty-reconnect.ts @@ -69,16 +69,6 @@ const WS_OPEN = 1 const WS_CLOSING = 2 const WS_CLOSED = 3 -export interface PtyKeepaliveInput { - isActive: boolean - visibilityState?: DocumentVisibilityState - socketReadyState?: number | null -} - -export function shouldSendPtyKeepalive({ isActive, visibilityState, socketReadyState }: PtyKeepaliveInput): boolean { - return isActive && visibilityState !== 'hidden' && socketReadyState === WS_OPEN -} - export function shouldReconnectPtyOnPageResume({ isActive, visibilityState, diff --git a/web/src/lib/xterm-webgl-release.test.ts b/web/src/lib/xterm-webgl-release.test.ts new file mode 100644 index 0000000000..9d0ddc3379 --- /dev/null +++ b/web/src/lib/xterm-webgl-release.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it, vi } from 'vitest' + +import { loseWebglContexts } from './xterm-webgl-release' + +function canvasWith(gl: unknown) { + return { + getContext: vi.fn((type: string) => (type === 'webgl2' ? gl : null)) + } +} + +describe('loseWebglContexts', () => { + it('loses every WebGL context under the host and leaves 2D canvases alone', () => { + // Dashboard reconnect churn (#111909): xterm disposes its WebGL canvas + // without losing the context, so the browser's context budget fills up. + const loseContext = vi.fn() + const gl = { getExtension: vi.fn((name: string) => (name === 'WEBGL_lose_context' ? { loseContext } : null)) } + const twoD = canvasWith(null) + const host = { querySelectorAll: () => [canvasWith(gl), twoD, canvasWith(gl)] } as unknown as ParentNode + + expect(loseWebglContexts(host)).toBe(2) + expect(loseContext).toHaveBeenCalledTimes(2) + // A canvas that holds no WebGL context is never handed one. + expect(twoD.getContext).toHaveBeenCalledWith('webgl2') + expect(twoD.getContext).toHaveBeenCalledWith('webgl') + }) +}) diff --git a/web/src/lib/xterm-webgl-release.ts b/web/src/lib/xterm-webgl-release.ts new file mode 100644 index 0000000000..6a357f427f --- /dev/null +++ b/web/src/lib/xterm-webgl-release.ts @@ -0,0 +1,27 @@ +/** + * Release the WebGL contexts an xterm WebglAddon left behind. + * + * `@xterm/addon-webgl` (0.19) removes its canvas on dispose but never calls + * `WEBGL_lose_context.loseContext()`, so the GL context stays alive until the + * browser garbage-collects the canvas. Browsers cap live contexts (~16) and + * force-lose the oldest when the cap is hit; ChatPage rebuilds the terminal on + * every PTY reconnect, so a reconnect storm walks straight into that cap and + * eventually loses the context of the terminal the user is looking at. + * + * Call it on the terminal host BEFORE `term.dispose()` removes the canvases. + * Returns the number of contexts released. + */ +export function loseWebglContexts(host: ParentNode): number { + let released = 0 + for (const canvas of Array.from(host.querySelectorAll('canvas'))) { + // getContext() with the type a canvas already holds returns that context; + // for a 2D canvas both WebGL lookups return null without creating one. + const gl = canvas.getContext('webgl2') ?? canvas.getContext('webgl') + const lose = gl?.getExtension('WEBGL_lose_context') + if (lose) { + lose.loseContext() + released += 1 + } + } + return released +} diff --git a/web/src/pages/ChatPage.test.tsx b/web/src/pages/ChatPage.test.tsx index 6fc28d2166..9c080dae69 100644 --- a/web/src/pages/ChatPage.test.tsx +++ b/web/src/pages/ChatPage.test.tsx @@ -15,10 +15,6 @@ class FakeFitAddon { } class FakeWebglAddon { - constructor() { - webglAddonConstructed += 1; - } - onContextLoss() { return { dispose() {} }; } @@ -165,7 +161,6 @@ type CloseEventLike = { let container: HTMLDivElement; let root: Root; -let webglAddonConstructed = 0; // jsdom runs without an origin here (per-file @vitest-environment jsdom on a // node-default config), so localStorage is undefined. Stub it so components @@ -200,7 +195,6 @@ async function render(ui: ReactNode) { beforeEach(() => { FakeWebSocket.instances = []; - webglAddonConstructed = 0; maybeReloadForLoopbackWsAuthFailure.mockClear(); apiMocks.buildWsUrl.mockReset(); apiMocks.buildWsUrl.mockResolvedValue("ws://localhost/api/pty?channel=chat-1"); @@ -265,7 +259,7 @@ afterEach(async () => { }); describe("ChatPage", () => { - it("uses the canvas renderer and sends a visible PTY keepalive", async () => { + it("sends a PTY keepalive frame every 20 seconds while the socket is open", async () => { vi.useFakeTimers(); try { const { default: ChatPage } = await import("./ChatPage"); @@ -278,7 +272,6 @@ describe("ChatPage", () => { await Promise.resolve(); }); expect(FakeWebSocket.instances).toHaveLength(1); - expect(webglAddonConstructed).toBe(0); const socket = FakeWebSocket.instances[0]; await act(async () => socket.onopen?.()); diff --git a/web/src/pages/ChatPage.tsx b/web/src/pages/ChatPage.tsx index ca74a9eac9..039621169a 100644 --- a/web/src/pages/ChatPage.tsx +++ b/web/src/pages/ChatPage.tsx @@ -4,7 +4,7 @@ *
(dashboard chrome) . * └─
(rounded, dark bg, padded — the "terminal window" . * look that gives the page a distinct visual identity) . - * └─ @xterm/xterm Terminal (canvas renderer, Unicode 11 widths) . + * └─ @xterm/xterm Terminal (WebGL renderer, Unicode 11 widths) . * │ onData keystrokes → WebSocket → PTY master . * │ onResize terminal resize → `\x1b[RESIZE:cols;rows]` . * │ write(data) PTY output bytes → VT100 parser . @@ -19,6 +19,7 @@ import { FitAddon } from "@xterm/addon-fit"; import { Unicode11Addon } from "@xterm/addon-unicode11"; import { WebLinksAddon } from "@xterm/addon-web-links"; +import { WebglAddon } from "@xterm/addon-webgl"; import { Terminal } from "@xterm/xterm"; import "@xterm/xterm/css/xterm.css"; import { Button } from "@nous-research/ui/ui/components/button"; @@ -51,7 +52,6 @@ import { type PtyConnectionState, ptyReconnectDelayMs, shouldBlockPtyInput, - shouldSendPtyKeepalive, shouldReconnectPtyOnPageResume, } from "@/lib/pty-reconnect"; import { @@ -92,6 +92,7 @@ import { ptyRejectionBanner, type PtyBannerAction, } from "@/lib/pty-close-copy"; +import { loseWebglContexts } from "@/lib/xterm-webgl-release"; import { PluginSlot } from "@/plugins"; import { useTheme } from "@/themes"; import { useProfileScope } from "@/contexts/useProfileScope"; @@ -929,9 +930,24 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { }; } - // Keep the default canvas renderer. Reconnects recreate the xterm - // instance; avoiding WebGL prevents those short-lived instances from - // exhausting the browser's limited WebGL context budget. + // WebGL draws from a texture atlas sized with device pixels. On phones and + // in DevTools device mode that often produces *visually* much larger cells + // than `fontSize` suggests — users see "huge" text even at 7–9px settings. + // The canvas/DOM renderer tracks `fontSize` faithfully; use it for narrow + // hosts. Wide layouts still get WebGL for crisp box-drawing. + const useWebgl = terminalTierWidthPx(host) >= 768; + if (useWebgl) { + try { + const webgl = new WebglAddon(); + webgl.onContextLoss(() => webgl.dispose()); + term.loadAddon(webgl); + } catch (err) { + console.warn( + "[hermes-chat] WebGL renderer unavailable; falling back to default", + err, + ); + } + } // Initial fit + resize observer. fit.fit() reads the container's // current bounding box and resizes the terminal grid to match. @@ -1304,17 +1320,11 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { } }; sendTerminalResize(); - keepaliveTimer = setInterval(() => { - if ( - shouldSendPtyKeepalive({ - isActive: isActiveRef.current, - visibilityState: document.visibilityState, - socketReadyState: ws.readyState, - }) - ) { - sendTerminalResize(); - } - }, PTY_KEEPALIVE_INTERVAL_MS); + // Application-level keepalive: browsers cannot send WS ping frames, and a + // loopback-bound dashboard behind a reverse proxy gets no server pings + // either, so a quiet PTY socket is idle traffic to any proxy timeout. + // Runs whenever the socket is open — a hidden tab still owns its PTY. + keepaliveTimer = setInterval(sendTerminalResize, PTY_KEEPALIVE_INTERVAL_MS); // Resumed sessions replay scrollback over the socket. Start pinned to // the bottom so the latest output is in view; released once the user // scrolls up (#59591). @@ -1599,6 +1609,10 @@ export default function ChatPage({ isActive = true }: { isActive?: boolean }) { wsRef.current?.close(); wsRef.current = null; host.removeEventListener("keydown", _imeCompositionGuard, true); + // Every reconnect rebuilds this terminal; the WebGL addon leaves its GL + // context alive on dispose, so a reconnect storm hits the browser's + // context cap and blanks the live terminal (#111909). + loseWebglContexts(host); term.dispose(); termRef.current = null; fitRef.current = null;