From 68c984071ae339f49af7030b14abbd2ca82a8e1e Mon Sep 17 00:00:00 2001 From: m4air Date: Tue, 15 Sep 2026 22:06:24 -0700 Subject: [PATCH] fix(terminal): detach retained error and reattach string slices --- docs/audits/retained-text-slices/README.md | 31 +++++++---- .../audits/retained-text-slices/reproduce.mjs | 25 +++++++-- docs/audits/retained-text-slices/results.json | 54 +++++++++++++++---- .../deferred-reattach-live-data-queue.ts | 5 +- .../terminal-capped-buffer-retention.test.ts | 42 +++++++++++++++ .../terminal-error-accumulation.ts | 6 ++- 6 files changed, 135 insertions(+), 28 deletions(-) diff --git a/docs/audits/retained-text-slices/README.md b/docs/audits/retained-text-slices/README.md index 160ad20e081..ca1cab92870 100644 --- a/docs/audits/retained-text-slices/README.md +++ b/docs/audits/retained-text-slices/README.md @@ -18,6 +18,9 @@ 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. +Deferred reattach queues have the same sliced-tail defect. Terminal error state +keeps eight messages capped at 4,000 characters each, yet those messages can also +pin oversized source strings; copying the final error surface bounds that storage. ## Reproduce @@ -26,18 +29,20 @@ ORCA_BACKGROUND_LAUNCH=1 node --expose-gc docs/audits/retained-text-slices/repro ``` 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 +removes only the five 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 | +| Case | Returned bytes, all eight | Retained heap before | After | +| -------------------------------- | ------------------------: | -------------------: | --------: | +| GitHub long line | 131,072 | 16,786,760 | 142,616 | +| GitHub earlier Unicode error | 131,064 | 33,577,472 | 106,392 | +| GitLab long line | 131,072 | 16,793,272 | 147,520 | +| Persisted terminal buffers | 4,194,304 | 33,555,624 | 4,195,736 | +| Eager/pre-handler/shutdown tails | 4,194,304 | 33,555,960 | 4,195,568 | +| Terminal error surfaces | 32,000 | 33,556,120 | 33,272 | +| Deferred reattach tails | 4,194,304 | 33,565,304 | 4,198,352 | 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 @@ -45,9 +50,13 @@ run passed 41 tests, including actual retained-heap checks, existing byte/conten 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. +After adding error and deferred-reattach coverage, another three-suite run passed +28 tests, including the actual retained error state and queue objects. Counts are +per run and overlap. 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 +repeated CI-log viewing or oversized terminal payloads, so these are demonstrated +retention mechanisms, not an attribution of either incident. Copying costs at most the final +16 KiB CI excerpt, 4,000-character error, or truncated terminal tail +(512 KiB for byte-capped buffers; 512 Ki characters for the reattach queue) 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 index 87578c4c2c8..69c6ac1fb9a 100644 --- a/docs/audits/retained-text-slices/reproduce.mjs +++ b/docs/audits/retained-text-slices/reproduce.mjs @@ -19,6 +19,11 @@ const replacements = { 'pty-eager-buffer-clamp.ts': [ 'data: tail.text.length < data.length ? flattenRetainedSlice(tail.text) : tail.text', 'data: tail.text' + ], + 'terminal-error-accumulation.ts': ['return flattenRetainedSlice(bounded)', 'return bounded'], + 'deferred-reattach-live-data-queue.ts': [ + 'flattenRetainedSlice(chunk.data.slice(-MAX_DEFERRED_REATTACH_LIVE_CHARS))', + 'chunk.data.slice(-MAX_DEFERRED_REATTACH_LIVE_CHARS)' ] } const results = [] @@ -51,6 +56,8 @@ for (const fixed of [false, true]) { 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' + export { boundTerminalErrorSurface } from './src/renderer/src/components/terminal-pane/terminal-error-accumulation' + export { DeferredReattachLiveDataQueue } from './src/renderer/src/components/terminal-pane/deferred-reattach-live-data-queue' `, resolveDir: root, loader: 'ts' @@ -68,7 +75,7 @@ for (const fixed of [false, true]) { builder.onLoad( { filter: - /(?:check-job-log-tail-slice|workspace-session-terminal-buffers|pty-eager-buffer-clamp)\.ts$/ + /(?:check-job-log-tail-slice|workspace-session-terminal-buffers|pty-eager-buffer-clamp|terminal-error-accumulation|deferred-reattach-live-data-queue)\.ts$/ }, async ({ path }) => { const source = await readFile(path, 'utf8') @@ -79,7 +86,7 @@ for (const fixed of [false, true]) { throw new Error('The copy boundary changed; update the baseline transform') } return { - contents: source.replace(...replacement), + contents: source.replaceAll(...replacement), loader: 'ts' } } @@ -94,7 +101,9 @@ for (const fixed of [false, true]) { sliceCheckLogTail, gitLabJobTraceToLogExcerpt, capTerminalScrollbackSessionBuffer, - clampUtf8Tail + clampUtf8Tail, + boundTerminalErrorSurface, + DeferredReattachLiveDataQueue } = 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], @@ -113,6 +122,16 @@ for (const fixed of [false, true]) { 'terminal-eager-buffer', (i) => `${i}:${'x'.repeat(parentChars * 2)}`, (text) => clampUtf8Tail(text, 512 * 1024).data + ], + ['terminal-error', (i) => `${'x'.repeat(parentChars * 2)}:${i}`, boundTerminalErrorSurface], + [ + 'terminal-deferred-reattach', + (i) => `${i}:${'x'.repeat(parentChars * 2)}`, + (data) => { + const queue = new DeferredReattachLiveDataQueue() + queue.enqueue({ data, ptyId: 'p', streamGeneration: 1 }) + return queue.takeAll()[0].data + } ] ]) { results.push({ kind, fixed, ...measure(excerpt, makeLog) }) diff --git a/docs/audits/retained-text-slices/results.json b/docs/audits/retained-text-slices/results.json index 3d9f6d3e150..4e097e1bd48 100644 --- a/docs/audits/retained-text-slices/results.json +++ b/docs/audits/retained-text-slices/results.json @@ -4,8 +4,8 @@ "parentChars": 2097152, "count": 8, "bundles": { - "before": "e8c11e93f529e8e8ff8b379a4f6e9bc1dedf37717ae086c6b019ab2fa39b3c1f", - "after": "3bb39c435d59f8d8e07e533a7eb2fe54c1afd39e27825e64e73d852b74f547a4" + "before": "a02e097c1c10d0c9b60c44b415c9d0eb521a56e3033fef1eecc001c41b822624", + "after": "f6a0b5d5c50b34b86b63161f16ec961b46793b408dc8916443e327ba7e623b80" }, "results": [ { @@ -14,7 +14,7 @@ "entries": 8, "logicalChars": 131072, "logicalBytes": 131072, - "heapDelta": 16774216 + "heapDelta": 16786760 }, { "kind": "github-earlier-error", @@ -22,7 +22,7 @@ "entries": 8, "logicalChars": 43816, "logicalBytes": 131064, - "heapDelta": 33577840 + "heapDelta": 33577472 }, { "kind": "gitlab-long-line", @@ -30,7 +30,7 @@ "entries": 8, "logicalChars": 131072, "logicalBytes": 131072, - "heapDelta": 16793640 + "heapDelta": 16793272 }, { "kind": "terminal-session-buffer", @@ -46,7 +46,23 @@ "entries": 8, "logicalChars": 4194304, "logicalBytes": 4194304, - "heapDelta": 33556040 + "heapDelta": 33555960 + }, + { + "kind": "terminal-error", + "fixed": false, + "entries": 8, + "logicalChars": 32000, + "logicalBytes": 32000, + "heapDelta": 33556120 + }, + { + "kind": "terminal-deferred-reattach", + "fixed": false, + "entries": 8, + "logicalChars": 4194304, + "logicalBytes": 4194304, + "heapDelta": 33565304 }, { "kind": "github-long-line", @@ -54,7 +70,7 @@ "entries": 8, "logicalChars": 131072, "logicalBytes": 131072, - "heapDelta": 143160 + "heapDelta": 142616 }, { "kind": "github-earlier-error", @@ -62,7 +78,7 @@ "entries": 8, "logicalChars": 43816, "logicalBytes": 131064, - "heapDelta": 106064 + "heapDelta": 106392 }, { "kind": "gitlab-long-line", @@ -70,7 +86,7 @@ "entries": 8, "logicalChars": 131072, "logicalBytes": 131072, - "heapDelta": 147488 + "heapDelta": 147520 }, { "kind": "terminal-session-buffer", @@ -78,7 +94,7 @@ "entries": 8, "logicalChars": 4194304, "logicalBytes": 4194304, - "heapDelta": 4195352 + "heapDelta": 4195736 }, { "kind": "terminal-eager-buffer", @@ -86,7 +102,23 @@ "entries": 8, "logicalChars": 4194304, "logicalBytes": 4194304, - "heapDelta": 4195600 + "heapDelta": 4195568 + }, + { + "kind": "terminal-error", + "fixed": true, + "entries": 8, + "logicalChars": 32000, + "logicalBytes": 32000, + "heapDelta": 33272 + }, + { + "kind": "terminal-deferred-reattach", + "fixed": true, + "entries": 8, + "logicalChars": 4194304, + "logicalBytes": 4194304, + "heapDelta": 4198352 } ] } diff --git a/src/renderer/src/components/terminal-pane/deferred-reattach-live-data-queue.ts b/src/renderer/src/components/terminal-pane/deferred-reattach-live-data-queue.ts index 3045569b1e1..86c2f6fe4fd 100644 --- a/src/renderer/src/components/terminal-pane/deferred-reattach-live-data-queue.ts +++ b/src/renderer/src/components/terminal-pane/deferred-reattach-live-data-queue.ts @@ -1,4 +1,5 @@ import type { PtyDataMeta } from './pty-dispatcher' +import { flattenRetainedSlice } from '../../lib/flatten-retained-slice' export const MAX_DEFERRED_REATTACH_LIVE_CHARS = 512 * 1024 export const MAX_DEFERRED_REATTACH_LIVE_CHUNKS = 1_024 @@ -35,7 +36,9 @@ export class DeferredReattachLiveDataQueue { const oversized = chunk.data.length > MAX_DEFERRED_REATTACH_LIVE_CHARS const queuedChunk = { ...chunk, - data: oversized ? chunk.data.slice(-MAX_DEFERRED_REATTACH_LIVE_CHARS) : chunk.data + data: oversized + ? flattenRetainedSlice(chunk.data.slice(-MAX_DEFERRED_REATTACH_LIVE_CHARS)) + : chunk.data } this.chunks.push(queuedChunk) this.retainedChars += queuedChunk.data.length 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 index 00a9d5d96c8..5d16aeb6979 100644 --- 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 @@ -3,6 +3,8 @@ import { capTerminalScrollbackSessionBuffer } from '../../../../shared/workspace 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' +import { DeferredReattachLiveDataQueue } from './deferred-reattach-live-data-queue' +import { appendPaneTerminalError, type TerminalErrorsByPaneId } from './terminal-error-accumulation' const LIMIT = TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT const PARENT_CHARS = 4 * 1024 * 1024 @@ -17,6 +19,14 @@ function heapAfterGc(): number { return process.memoryUsage().heapUsed } +function createPaneErrors(): TerminalErrorsByPaneId { + let errors: TerminalErrorsByPaneId = {} + for (let index = 0; index < COUNT; index++) { + errors = appendPaneTerminalError(errors, 0, `${'x'.repeat(PARENT_CHARS)}:${index}`) + } + return errors +} + describe('capped terminal buffer retention', () => { it.each([ ['persisted scrollback', capTerminalScrollbackSessionBuffer], @@ -47,4 +57,36 @@ describe('capped terminal buffer retention', () => { expect(queue.takeAll()).toEqual([{ kind: 'replay', data: 'x'.repeat(LIMIT) }]) } }) + + it('detaches oversized chunks while a reattach queue waits for its consumer', () => { + const before = heapAfterGc() + const queues = Array.from({ length: COUNT }, (_value, index) => { + const queue = new DeferredReattachLiveDataQueue() + queue.enqueue({ + data: `${index}:${'x'.repeat(PARENT_CHARS)}`, + ptyId: 'p', + streamGeneration: 1 + }) + return queue + }) + const growth = heapAfterGc() - before + + expect(queues.every((queue) => queue.getStorageForTest().retainedChars === LIMIT)).toBe(true) + expect(growth).toBeLessThan(COUNT * LIMIT * 2) + for (const queue of queues) { + expect(queue.takeAll()[0]?.data).toBe('x'.repeat(LIMIT)) + } + }) + + it('keeps capped pane errors without retaining the original error payloads', () => { + const before = heapAfterGc() + const errors = createPaneErrors() + const growth = heapAfterGc() - before + + expect(errors[0]).toHaveLength(COUNT) + expect( + errors[0].every((text, index) => text.length === 4000 && text.endsWith(`:${index}`)) + ).toBe(true) + expect(growth).toBeLessThan(PARENT_CHARS) + }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts index 63c5de8423e..d22364e74ad 100644 --- a/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts +++ b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts @@ -1,3 +1,5 @@ +import { flattenRetainedSlice } from '../../lib/flatten-retained-slice' + // The toast still consumes newline-joined copy, so legacy tab-wide messages need // whole-run dedup even though pane errors remain structurally separate until render. function containsWholeLineRun(accumulated: string, message: string): boolean { @@ -22,12 +24,12 @@ export function boundTerminalErrorSurface( const lines = surface.split('\n') let bounded = lines.length > maxLines ? lines.slice(-maxLines).join('\n') : surface if (bounded.length <= maxChars) { - return bounded + return flattenRetainedSlice(bounded) } const suffix = bounded.slice(-maxChars) const firstNewline = suffix.indexOf('\n') bounded = firstNewline === -1 ? suffix : suffix.slice(firstNewline + 1) || suffix - return bounded + return flattenRetainedSlice(bounded) } export function appendPaneTerminalError(