fix(terminal): detach retained error and reattach string slices

This commit is contained in:
m4air
2026-09-15 22:06:24 -07:00
parent 5c4f5535cc
commit 68c984071a
6 changed files with 135 additions and 28 deletions
+20 -11
View File
@@ -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.
+22 -3
View File
@@ -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) })
+43 -11
View File
@@ -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
}
]
}
@@ -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
@@ -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)
})
})
@@ -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(