From 760fac19ea64bd0cdc3008a19188ec52d878fdc3 Mon Sep 17 00:00:00 2001 From: m4air Date: Tue, 15 Sep 2026 21:57:06 -0700 Subject: [PATCH] fix(memory): detach retained CI and terminal tails from oversized strings --- docs/audits/retained-text-slices/README.md | 53 ++++++++ .../audits/retained-text-slices/reproduce.mjs | 127 ++++++++++++++++++ docs/audits/retained-text-slices/results.json | 92 +++++++++++++ .../terminal-pane/pty-eager-buffer-clamp.ts | 6 +- .../terminal-capped-buffer-retention.test.ts | 50 +++++++ .../src/lib/flatten-retained-slice.ts | 5 +- src/shared/check-job-log-retention.test.ts | 45 +++++++ src/shared/check-job-log-tail-slice.ts | 8 +- src/shared/flatten-retained-slice.ts | 4 + .../workspace-session-terminal-buffers.ts | 5 +- 10 files changed, 388 insertions(+), 7 deletions(-) create mode 100644 docs/audits/retained-text-slices/README.md create mode 100644 docs/audits/retained-text-slices/reproduce.mjs create mode 100644 docs/audits/retained-text-slices/results.json create mode 100644 src/renderer/src/components/terminal-pane/terminal-capped-buffer-retention.test.ts create mode 100644 src/shared/check-job-log-retention.test.ts create mode 100644 src/shared/flatten-retained-slice.ts diff --git a/docs/audits/retained-text-slices/README.md b/docs/audits/retained-text-slices/README.md new file mode 100644 index 00000000000..160ad20e081 --- /dev/null +++ b/docs/audits/retained-text-slices/README.md @@ -0,0 +1,53 @@ +# Retained CI and terminal text tails + +`sliceCheckLogTail` limits its returned text to 16 KiB, but a V8 sliced string can +keep the entire downloaded log alive. Long single lines and oversized earlier +error context reproduce this. The GitHub job-tail cache accepts 128 entries, with +downloads up to 64 MiB each; its logical tail cap therefore did not bound retained +backing storage. GitLab's raw-tail clamp can produce the same parent-retaining +slice before the shared excerpt function runs. + +The fix reuses Orca's existing `flattenRetainedSlice` implementation, moving it to +shared code and preserving its renderer import through a re-export. The public +excerpt function copies its final, already-capped result. Content, Unicode, +earlier-error selection, cache count, and transport payloads stay identical. + +The same defect exists at terminal retention boundaries. Session scrollback caps +and eager/pre-handler/shutdown queues keep 512 KiB tails of oversized strings, +but their byte ledgers miss the retained parent. The fix copies only truncated +tails, preserving the existing path for ordinary chunks. Persisted local +scrollback is already pruned; the session-buffer fix primarily covers remote or +not-yet-classified owners, while the queue fix covers local and remote output. + +## Reproduce + +```sh +ORCA_BACKGROUND_LAUNCH=1 node --expose-gc docs/audits/retained-text-slices/reproduce.mjs +``` + +The script bundles the actual shared GitHub/GitLab excerpt functions. Its baseline +removes only the three new copy boundaries in memory; production files are not changed. It retains +eight excerpts from distinct 2 MiB-character CI inputs, or eight 512 KiB +terminal tails from 4 MiB-character inputs, and measures heap after GC. A small regex operation clears V8's independent last-input reference. Bundle +hashes and all measurements are in [results.json](./results.json). + +| Case | Returned bytes, all eight | Retained heap before | After | +| --- | ---: | ---: | ---: | +| GitHub long line | 131,072 | 16,774,216 | 143,160 | +| GitHub earlier Unicode error | 131,064 | 33,577,840 | 106,064 | +| GitLab long line | 131,072 | 16,793,640 | 147,488 | +| Persisted terminal buffers | 4,194,304 | 33,555,624 | 4,195,352 | +| Eager/pre-handler/shutdown tails | 4,194,304 | 33,556,040 | 4,195,600 | + +Captured on macOS with Node v26.6.0. Heap samples include allocator/GC variation; +the order-of-magnitude separation is the relevant result. The focused six-suite +run passed 41 tests, including actual retained-heap checks, existing byte/content +contracts, GitLab normalization, and GitHub check-detail integration. A further +six-suite terminal run passed 69 tests, including actual shutdown queue storage, +scrollback ownership, pre-handler buffering, reattach, and UTF-8 boundaries. + +The cap/slice path also exists in `v1.4.198`. Neither #19831 nor #19768 establishes +repeated CI-log viewing, so this is another demonstrated main-process retention +mechanism, not an attribution of either incident. Copying costs at most the final +16 KiB CI excerpt or truncated 512 KiB terminal tail and does not lower the +temporary allocation needed to download or parse the original input. diff --git a/docs/audits/retained-text-slices/reproduce.mjs b/docs/audits/retained-text-slices/reproduce.mjs new file mode 100644 index 00000000000..87578c4c2c8 --- /dev/null +++ b/docs/audits/retained-text-slices/reproduce.mjs @@ -0,0 +1,127 @@ +import { createHash } from 'node:crypto' +import { readFile } from 'node:fs/promises' +import { fileURLToPath } from 'node:url' +import { build } from 'esbuild' + +if (process.env.ORCA_BACKGROUND_LAUNCH !== '1' || typeof global.gc !== 'function') { + throw new Error('Run with ORCA_BACKGROUND_LAUNCH=1 node --expose-gc') +} +const root = fileURLToPath(new URL('../../../', import.meta.url)) +const replacements = { + 'check-job-log-tail-slice.ts': [ + 'return flattenRetainedSlice(buildCheckLogTail(logText))', + 'return buildCheckLogTail(logText)' + ], + 'workspace-session-terminal-buffers.ts': [ + 'return flattenRetainedSlice(\n clampUtf8TextTail(buffer, TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT).text\n )', + 'return clampUtf8TextTail(buffer, TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT).text' + ], + 'pty-eager-buffer-clamp.ts': [ + 'data: tail.text.length < data.length ? flattenRetainedSlice(tail.text) : tail.text', + 'data: tail.text' + ] +} +const results = [] +const bundles = {} +const parentChars = 2 * 1024 * 1024 +const count = 8 + +function measure(excerpt, makeLog) { + global.gc() + const before = process.memoryUsage().heapUsed + const retained = Array.from({ length: count }, (_, index) => excerpt(makeLog(index))) + // Clear V8's independent legacy RegExp input reference before measuring our retained values. + void /probe/.test('probe') + global.gc() + global.gc() + const heapDelta = process.memoryUsage().heapUsed - before + return { + entries: retained.length, + logicalChars: retained.reduce((total, text) => total + text.length, 0), + logicalBytes: retained.reduce((total, text) => total + Buffer.byteLength(text), 0), + heapDelta + } +} + +for (const fixed of [false, true]) { + const result = await build({ + stdin: { + contents: ` + export { sliceCheckLogTail } from './src/shared/check-job-log-tail-slice' + export { gitLabJobTraceToLogExcerpt } from './src/shared/gitlab-job-log-excerpt' + export { capTerminalScrollbackSessionBuffer } from './src/shared/workspace-session-terminal-buffers' + export { clampUtf8Tail } from './src/renderer/src/components/terminal-pane/pty-eager-buffer-clamp' + `, + resolveDir: root, + loader: 'ts' + }, + bundle: true, + platform: 'node', + format: 'esm', + write: false, + plugins: fixed + ? [] + : [ + { + name: 'baseline-without-retained-tail-copy', + setup(builder) { + builder.onLoad( + { + filter: + /(?:check-job-log-tail-slice|workspace-session-terminal-buffers|pty-eager-buffer-clamp)\.ts$/ + }, + async ({ path }) => { + const source = await readFile(path, 'utf8') + const replacement = Object.entries(replacements).find(([name]) => + path.endsWith(name) + )?.[1] + if (!replacement || !source.includes(replacement[0])) { + throw new Error('The copy boundary changed; update the baseline transform') + } + return { + contents: source.replace(...replacement), + loader: 'ts' + } + } + ) + } + } + ] + }) + const bundle = result.outputFiles[0].text + bundles[fixed ? 'after' : 'before'] = createHash('sha256').update(bundle).digest('hex') + const { + sliceCheckLogTail, + gitLabJobTraceToLogExcerpt, + capTerminalScrollbackSessionBuffer, + clampUtf8Tail + } = await import(`data:text/javascript;base64,${Buffer.from(bundle).toString('base64')}`) + for (const [kind, makeLog, excerpt] of [ + ['github-long-line', (i) => `${i}:${'x'.repeat(parentChars)}`, sliceCheckLogTail], + [ + 'github-earlier-error', + (i) => `error: ${i}:${'界'.repeat(parentChars)}\n${'recent\n'.repeat(100)}`, + sliceCheckLogTail + ], + ['gitlab-long-line', (i) => `${i}:${'x'.repeat(parentChars)}`, gitLabJobTraceToLogExcerpt], + [ + 'terminal-session-buffer', + (i) => `${i}:${'x'.repeat(parentChars * 2)}`, + capTerminalScrollbackSessionBuffer + ], + [ + 'terminal-eager-buffer', + (i) => `${i}:${'x'.repeat(parentChars * 2)}`, + (text) => clampUtf8Tail(text, 512 * 1024).data + ] + ]) { + results.push({ kind, fixed, ...measure(excerpt, makeLog) }) + } +} +console.log( + JSON.stringify( + { node: process.version, platform: process.platform, parentChars, count, bundles, results }, + null, + 2 + ) +) diff --git a/docs/audits/retained-text-slices/results.json b/docs/audits/retained-text-slices/results.json new file mode 100644 index 00000000000..3d9f6d3e150 --- /dev/null +++ b/docs/audits/retained-text-slices/results.json @@ -0,0 +1,92 @@ +{ + "node": "v26.6.0", + "platform": "darwin", + "parentChars": 2097152, + "count": 8, + "bundles": { + "before": "e8c11e93f529e8e8ff8b379a4f6e9bc1dedf37717ae086c6b019ab2fa39b3c1f", + "after": "3bb39c435d59f8d8e07e533a7eb2fe54c1afd39e27825e64e73d852b74f547a4" + }, + "results": [ + { + "kind": "github-long-line", + "fixed": false, + "entries": 8, + "logicalChars": 131072, + "logicalBytes": 131072, + "heapDelta": 16774216 + }, + { + "kind": "github-earlier-error", + "fixed": false, + "entries": 8, + "logicalChars": 43816, + "logicalBytes": 131064, + "heapDelta": 33577840 + }, + { + "kind": "gitlab-long-line", + "fixed": false, + "entries": 8, + "logicalChars": 131072, + "logicalBytes": 131072, + "heapDelta": 16793640 + }, + { + "kind": "terminal-session-buffer", + "fixed": false, + "entries": 8, + "logicalChars": 4194304, + "logicalBytes": 4194304, + "heapDelta": 33555624 + }, + { + "kind": "terminal-eager-buffer", + "fixed": false, + "entries": 8, + "logicalChars": 4194304, + "logicalBytes": 4194304, + "heapDelta": 33556040 + }, + { + "kind": "github-long-line", + "fixed": true, + "entries": 8, + "logicalChars": 131072, + "logicalBytes": 131072, + "heapDelta": 143160 + }, + { + "kind": "github-earlier-error", + "fixed": true, + "entries": 8, + "logicalChars": 43816, + "logicalBytes": 131064, + "heapDelta": 106064 + }, + { + "kind": "gitlab-long-line", + "fixed": true, + "entries": 8, + "logicalChars": 131072, + "logicalBytes": 131072, + "heapDelta": 147488 + }, + { + "kind": "terminal-session-buffer", + "fixed": true, + "entries": 8, + "logicalChars": 4194304, + "logicalBytes": 4194304, + "heapDelta": 4195352 + }, + { + "kind": "terminal-eager-buffer", + "fixed": true, + "entries": 8, + "logicalChars": 4194304, + "logicalBytes": 4194304, + "heapDelta": 4195600 + } + ] +} diff --git a/src/renderer/src/components/terminal-pane/pty-eager-buffer-clamp.ts b/src/renderer/src/components/terminal-pane/pty-eager-buffer-clamp.ts index 0d4087e8288..90a8e122303 100644 --- a/src/renderer/src/components/terminal-pane/pty-eager-buffer-clamp.ts +++ b/src/renderer/src/components/terminal-pane/pty-eager-buffer-clamp.ts @@ -1,4 +1,5 @@ import { clampUtf8TextTail } from '../../../../shared/utf8-byte-limits' +import { flattenRetainedSlice } from '../../lib/flatten-retained-slice' export type EagerBufferChunk = { data: string @@ -7,5 +8,8 @@ export type EagerBufferChunk = { export function clampUtf8Tail(data: string, maxBytes: number): EagerBufferChunk { const tail = clampUtf8TextTail(data, maxBytes) - return { data: tail.text, bytes: tail.bytes } + return { + data: tail.text.length < data.length ? flattenRetainedSlice(tail.text) : tail.text, + bytes: tail.bytes + } } diff --git a/src/renderer/src/components/terminal-pane/terminal-capped-buffer-retention.test.ts b/src/renderer/src/components/terminal-pane/terminal-capped-buffer-retention.test.ts new file mode 100644 index 00000000000..00a9d5d96c8 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-capped-buffer-retention.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from 'vitest' +import { capTerminalScrollbackSessionBuffer } from '../../../../shared/workspace-session-terminal-buffers' +import { TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT } from '../../../../shared/terminal-scrollback-limits' +import { clampUtf8Tail } from './pty-eager-buffer-clamp' +import { PtyShutdownOutputQueue } from './pty-shutdown-output-queue' + +const LIMIT = TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT +const PARENT_CHARS = 4 * 1024 * 1024 +const COUNT = 8 + +function heapAfterGc(): number { + if (!('gc' in globalThis) || typeof globalThis.gc !== 'function') { + throw new Error('The test runner must enable --expose-gc') + } + globalThis.gc() + globalThis.gc() + return process.memoryUsage().heapUsed +} + +describe('capped terminal buffer retention', () => { + it.each([ + ['persisted scrollback', capTerminalScrollbackSessionBuffer], + ['eager/pre-handler output', (text: string) => clampUtf8Tail(text, LIMIT).data] + ] as const)('detaches %s from oversized incoming strings', (_label, cap) => { + const before = heapAfterGc() + const retained = Array.from({ length: COUNT }, (_value, index) => + cap(`${index}:${'x'.repeat(PARENT_CHARS)}`) + ) + const growth = heapAfterGc() - before + + expect(retained.every((text) => text === 'x'.repeat(LIMIT))).toBe(true) + expect(growth).toBeLessThan(COUNT * LIMIT * 2) + }) + + it('keeps shutdown queue heap storage near its byte ledger after clamping', () => { + const before = heapAfterGc() + const queues = Array.from({ length: COUNT }, (_value, index) => { + const queue = new PtyShutdownOutputQueue() + queue.enqueue({ kind: 'replay', data: `${index}:${'x'.repeat(PARENT_CHARS)}` }) + return queue + }) + const growth = heapAfterGc() - before + + expect(queues.every((queue) => queue.getStorageForTest().retainedBytes === LIMIT)).toBe(true) + expect(growth).toBeLessThan(COUNT * LIMIT * 2) + for (const queue of queues) { + expect(queue.takeAll()).toEqual([{ kind: 'replay', data: 'x'.repeat(LIMIT) }]) + } + }) +}) diff --git a/src/renderer/src/lib/flatten-retained-slice.ts b/src/renderer/src/lib/flatten-retained-slice.ts index 45585c3a75e..d48e020b393 100644 --- a/src/renderer/src/lib/flatten-retained-slice.ts +++ b/src/renderer/src/lib/flatten-retained-slice.ts @@ -1,4 +1 @@ -// V8 slices can retain their full parent; force a standalone copy for values that outlive it. -export function flattenRetainedSlice(value: string): string { - return value.length === 0 ? value : `${value} `.slice(0, -1) -} +export { flattenRetainedSlice } from '../../../shared/flatten-retained-slice' diff --git a/src/shared/check-job-log-retention.test.ts b/src/shared/check-job-log-retention.test.ts new file mode 100644 index 00000000000..a1ca9505140 --- /dev/null +++ b/src/shared/check-job-log-retention.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from 'vitest' +import { PR_CHECK_LOG_TAIL_BYTES, sliceCheckLogTail } from './check-job-log-tail-slice' +import { gitLabJobTraceToLogExcerpt } from './gitlab-job-log-excerpt' + +function heapAfterGc(): number { + if (!('gc' in globalThis) || typeof globalThis.gc !== 'function') { + throw new Error('The test runner must enable --expose-gc') + } + globalThis.gc() + globalThis.gc() + return process.memoryUsage().heapUsed +} + +const PARENT_CHARS = 2 * 1024 * 1024 +const COUNT = 8 + +describe('retained CI log excerpts', () => { + it.each([ + [ + 'GitHub long line', + (index: number) => `${index}:${'x'.repeat(PARENT_CHARS)}`, + sliceCheckLogTail + ], + [ + 'GitHub earlier error', + (index: number) => `error: ${index}:${'界'.repeat(PARENT_CHARS)}\n${'recent\n'.repeat(100)}`, + sliceCheckLogTail + ], + [ + 'GitLab raw trace', + (index: number) => `${index}:${'x'.repeat(PARENT_CHARS)}`, + gitLabJobTraceToLogExcerpt + ] + ] as const)('releases the parent of a %s', (_label, makeLog, excerpt) => { + const before = heapAfterGc() + const retained = Array.from({ length: COUNT }, (_value, index) => excerpt(makeLog(index))) + // V8's legacy RegExp statics can otherwise keep the final input independently of our cache. + void /probe/.test('probe') + const growth = heapAfterGc() - before + + expect(retained).toHaveLength(COUNT) + expect(retained.every((text) => Buffer.byteLength(text) <= PR_CHECK_LOG_TAIL_BYTES)).toBe(true) + expect(growth).toBeLessThan(PARENT_CHARS * 2) + }) +}) diff --git a/src/shared/check-job-log-tail-slice.ts b/src/shared/check-job-log-tail-slice.ts index 19afcdfc75d..738596c9be9 100644 --- a/src/shared/check-job-log-tail-slice.ts +++ b/src/shared/check-job-log-tail-slice.ts @@ -3,6 +3,7 @@ import { getUtf8ByteLength, isUtf8ByteLengthWithinLimit } from './utf8-byte-limits' +import { flattenRetainedSlice } from './flatten-retained-slice' export const PR_CHECK_LOG_TAIL_LINES = 200 export const PR_CHECK_LOG_TAIL_RECENT_LINES = 100 @@ -57,7 +58,7 @@ function collectEarlierErrorLineIndexes(lines: string[], recentStart: number): n return [...indexes].sort((left, right) => left - right) } -export function sliceCheckLogTail(logText: string): string { +function buildCheckLogTail(logText: string): string { const lines = logText.split(/\r?\n/) const recentStart = Math.max(0, lines.length - PR_CHECK_LOG_TAIL_RECENT_LINES) const recentLines = lines.slice(recentStart) @@ -80,3 +81,8 @@ export function sliceCheckLogTail(logText: string): string { recentLines ) } + +export function sliceCheckLogTail(logText: string): string { + // Cached excerpts must not pin the downloaded log behind a small V8 slice. + return flattenRetainedSlice(buildCheckLogTail(logText)) +} diff --git a/src/shared/flatten-retained-slice.ts b/src/shared/flatten-retained-slice.ts new file mode 100644 index 00000000000..45585c3a75e --- /dev/null +++ b/src/shared/flatten-retained-slice.ts @@ -0,0 +1,4 @@ +// V8 slices can retain their full parent; force a standalone copy for values that outlive it. +export function flattenRetainedSlice(value: string): string { + return value.length === 0 ? value : `${value} `.slice(0, -1) +} diff --git a/src/shared/workspace-session-terminal-buffers.ts b/src/shared/workspace-session-terminal-buffers.ts index 706dbedc2d7..974a11d3845 100644 --- a/src/shared/workspace-session-terminal-buffers.ts +++ b/src/shared/workspace-session-terminal-buffers.ts @@ -5,6 +5,7 @@ import { getRepoIdFromWorktreeId } from './worktree/id' import { TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT } from './terminal-scrollback-limits' import { clampUtf8TextTail, isUtf8ByteLengthWithinLimit } from './utf8-byte-limits' import { parseExecutionHostId } from './execution-host' +import { flattenRetainedSlice } from './flatten-retained-slice' export type RepoConnection = Pick @@ -53,7 +54,9 @@ export function capTerminalScrollbackSessionBuffer(buffer: string): string { if (isUtf8ByteLengthWithinLimit(buffer, TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT)) { return buffer } - return clampUtf8TextTail(buffer, TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT).text + return flattenRetainedSlice( + clampUtf8TextTail(buffer, TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT).text + ) } function capTerminalScrollbackLeafBuffers(buffers: Record | undefined): {