diff --git a/config/vitest.config.ts b/config/vitest.config.ts index 735e1b2a0af..d23bee128c9 100644 --- a/config/vitest.config.ts +++ b/config/vitest.config.ts @@ -16,7 +16,8 @@ export default defineConfig({ test: { environment: 'node', // Why: Node 26's undefined Web Storage globals prevent Vitest from installing happy-dom's. - execArgv: ['--no-experimental-webstorage'], + // Why --expose-gc: retention tests need a deterministic collection point to measure what a queue really holds. + execArgv: ['--no-experimental-webstorage', '--expose-gc'], // Why: happy-dom drops MutationObserver callbacks on GC; keep them alive like a browser does. setupFiles: [resolve('config/scripts/happy-dom-mutation-observer-retention.ts')], include: [ diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts index aaefe89f872..667b3f6555a 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts @@ -1677,4 +1677,79 @@ describe('pane terminal output scheduler', () => { vi.advanceTimersByTime(100) expect(throwing.write).toHaveBeenCalledTimes(1) }) + + describe('queue memory retention (STA-3567)', () => { + // Why 2 MB: comfortably above BACKGROUND_CHUNK_CHARS (16 K), so every drain leaves a residual slice. + const CHUNK_CHARS = 2 * 1024 * 1024 + const TERMINALS = 8 + + function collect(): number { + const gc = (globalThis as { gc?: () => void }).gc + if (!gc) { + throw new Error('global.gc unavailable - config/vitest.config.ts must pass --expose-gc') + } + gc() + gc() + return process.memoryUsage().heapUsed + } + + it('drops the parent chunk once only a small tail is still queued', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + + // Why a hand-rolled write: vi.fn() retains every argument in mock.calls, which would + // dominate the measurement with the very bytes the queue is supposed to have released. + let writtenChars = 0 + const makeSink = (): { write: (data: string, callback?: () => void) => void } => ({ + write: (data: string, callback?: () => void) => { + writtenChars += data.length + callback?.() + } + }) + + // Distinct leading chars keep V8 from sharing any backing store between terminals. + const terminals = Array.from({ length: TERMINALS }, (_unused, index) => { + const terminal = makeSink() + writeTerminalOutput( + terminal, + String.fromCharCode(65 + index) + 'q'.repeat(CHUNK_CHARS - 1), + { foreground: false, latencySensitive: false } + ) + return terminal + }) + + const debug = ( + globalThis as { + __terminalOutputSchedulerDebug?: { snapshot: () => { queuedChars: number } } + } + ).__terminalOutputSchedulerDebug + if (!debug) { + throw new Error('scheduler debug API unavailable') + } + + const baseline = collect() + + // Why stop early: the leak is what a PARTIALLY drained queue pins. Draining to empty + // frees the chunks either way, so a full drain cannot observe the defect. + const TAIL_CHARS = 64 * 1024 + let ticks = 0 + while (debug.snapshot().queuedChars > TAIL_CHARS && ticks < 20000) { + vi.advanceTimersByTime(4) + ticks += 1 + } + + const queuedChars = debug.snapshot().queuedChars + const retainedBytes = collect() - baseline + + // Sanity: nearly everything drained, but a real tail is still queued - otherwise + // there is no residual slice to hold a parent chunk and the measurement is meaningless. + expect(writtenChars).toBeGreaterThan(TERMINALS * CHUNK_CHARS * 0.9) + expect(queuedChars).toBeGreaterThan(0) + expect(queuedChars).toBeLessThanOrEqual(TAIL_CHARS) + + // The defect: those few queued KB pinned 8 x 2 MB of parent chunks (~16 MB). + expect(retainedBytes).toBeLessThan(4 * 1024 * 1024) + expect(terminals).toHaveLength(TERMINALS) + }) + }) }) diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts index 4272d4c0913..e05d22a1251 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts @@ -49,6 +49,8 @@ type WriteTerminalOutputOptions = { type QueueChunk = { data: string + // Why: `data` may be a V8 SlicedString pinning a much larger parent; this is what the queue really holds alive. + retainedChars: number foreground: boolean forceForegroundRefresh: boolean followupForegroundRefresh: boolean @@ -542,6 +544,11 @@ function coalescedQueuedDataNeedsCursorRestore(entry: QueueEntry): boolean { ) } +// Why: V8 backs `slice` with a SlicedString that points at the whole parent, so a small tail keeps a huge chunk alive. Concatenating forces a ConsString whose flattening allocates a fresh sequential string of just this length. +function flattenRetainedSlice(value: string): string { + return value.length === 0 ? value : `${value} `.slice(0, -1) +} + function takeQueuedChunk(entry: QueueEntry, limit: number): QueuedWrite | null { let remaining = limit let data = '' @@ -591,6 +598,18 @@ function takeQueuedChunk(entry: QueueEntry, limit: number): QueuedWrite | null { data += chunk.data remaining -= chunk.data.length entry.queuedChars -= chunk.data.length + // Why: compaction only splices every 64 chunks, so without this a fully consumed chunk keeps + // its (possibly parent-pinning) string and callbacks alive while charging nothing. Nothing + // reads below chunkIndex, so the drained slot only has to stay type-shaped. + entry.chunks[entry.chunkIndex] = { + data: '', + retainedChars: 0, + foreground: chunk.foreground, + forceForegroundRefresh: false, + followupForegroundRefresh: false, + shouldRefreshForegroundSynchronously: ALWAYS_REFRESH_FOREGROUND_SYNCHRONOUSLY, + stripTransientCursorShows: false + } entry.chunkIndex += 1 if (chunk.onParsed) { parsedCallbacks.push(chunk.onParsed) @@ -602,9 +621,13 @@ function takeQueuedChunk(entry: QueueEntry, limit: number): QueuedWrite | null { } data += chunk.data.slice(0, remaining) + const residual = chunk.data.slice(remaining) + // Why: the residual is a SlicedString still pinning `retainedChars`; copy it flat once it pins 2x its own length so a drained 2MB chunk stops holding 2MB to serve a 1KB tail. Halving keeps total copying linear in the original chunk. + const flatten = residual.length * 2 <= chunk.retainedChars entry.chunks[entry.chunkIndex] = { ...chunk, - data: chunk.data.slice(remaining) + data: flatten ? flattenRetainedSlice(residual) : residual, + retainedChars: flatten ? residual.length : chunk.retainedChars } entry.queuedChars -= remaining remaining = 0 @@ -681,6 +704,7 @@ function enqueueChunk( ): void { entry.chunks.push({ data, + retainedChars: data.length, foreground: options?.foreground === true, forceForegroundRefresh: options?.forceForegroundRefresh === true, followupForegroundRefresh: options?.followupForegroundRefresh === true, @@ -744,6 +768,7 @@ function replaceBacklogWithWarning( entry.chunks = [ { data: warning, + retainedChars: warning.length, foreground: false, forceForegroundRefresh: false, followupForegroundRefresh: false,