fix(terminal): flatten overlap-trimmed restore payloads before queueing

Review of the previous commit found the queue's retention bound was still
bypassable. enqueueChunk records retainedChars as data.length, but the
restore-reconciliation paths in pty-connection hand the scheduler a raw
slice of a much larger chunk, so the queue understated what it pinned:
16,803,368 bytes retained for 524,288 queued chars (32x) on Node 24.

Flatten those two overlap-trimmed payloads at the producer, where the copy
is one bounded tail rather than work on the drain hot path, and state the
enqueue contract explicitly: a caller must hand the queue a string that owns
its storage. flattenRetainedSlice moves to src/renderer/src/lib so both
owners share one documented primitive.
This commit is contained in:
Brennan Benson
2026-08-10 19:00:16 -04:00
committed by Jinwoo-H
parent ef3fb2af88
commit d419dc1b52
4 changed files with 79 additions and 7 deletions
@@ -333,6 +333,7 @@ import {
} from './renderer-owned-agent-status-registry'
import type { DirectSshPaneRetryAttempt } from '@/store/slices/direct-ssh-terminal-recovery'
import { directSshAuthoritiesEqual } from '@/store/slices/direct-ssh-terminal-authority-ledger'
import { flattenRetainedSlice } from '@/lib/flatten-retained-slice'
const pendingSpawnByPaneKey = new Map<string, Promise<string | null>>()
const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED'
@@ -6870,7 +6871,9 @@ export function connectPanePty(
if (rawLength !== chunk.data.length) {
return null
}
return chunk.data.slice(offset)
// Why flatten: this tail outlives the chunk it was cut from once queued, and a raw slice
// would pin the whole original chunk in the output queue (STA-3567).
return flattenRetainedSlice(chunk.data.slice(offset))
}
type RestoredSnapshotReconciliation =
@@ -6920,7 +6923,9 @@ export function connectPanePty(
// Why: renderer-only OSC stripping makes raw seq offsets unmappable onto cleaned text; refetch instead of risking duplicate output.
return { action: 'force-fresh-restore' }
}
const sliced = data.slice(restoredSnapshotBaselineSeq - startSeq)
// Why flatten: same as above - the trimmed tail is handed to the output queue, and a raw
// slice would keep the entire pre-trim payload alive behind it (STA-3567).
const sliced = flattenRetainedSlice(data.slice(restoredSnapshotBaselineSeq - startSeq))
return {
action: 'write',
data: sliced,
@@ -0,0 +1,57 @@
import { describe, expect, it } from 'vitest'
import { flattenRetainedSlice } from './flatten-retained-slice'
// Why 1 MB: comfortably above V8's SlicedString threshold, so a raw slice really does keep the
// parent alive and the difference between flattened and not is unmistakable in heapUsed.
const PARENT_CHARS = 1024 * 1024
const TAIL_CHARS = 512
const PARENTS = 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
}
// Distinct leading chars stop V8 sharing one backing store across the parents.
function makeParent(index: number, filler: string): string {
return String.fromCharCode(0x41 + index) + filler.repeat(PARENT_CHARS - 1)
}
function retainedBytesForTails(filler: string, transform: (value: string) => string): number {
const parents = Array.from({ length: PARENTS }, (_unused, index) => makeParent(index, filler))
const baseline = collect()
const tails = parents.map((parent) => transform(parent.slice(parent.length - TAIL_CHARS)))
// Drop the parents; only the tails stay reachable.
parents.length = 0
const retained = collect() - baseline
expect(tails).toHaveLength(PARENTS)
expect(tails.every((tail) => tail.length === TAIL_CHARS)).toBe(true)
return retained
}
describe('flattenRetainedSlice', () => {
it('preserves content exactly', () => {
expect(flattenRetainedSlice('')).toBe('')
expect(flattenRetainedSlice('a')).toBe('a')
const source = 'hello world, 漢字, , \u{1f600}'
expect(flattenRetainedSlice(source.slice(3))).toBe(source.slice(3))
})
it.each([
['one-byte', 'a'],
['two-byte', '漢']
])('drops the %s parent a raw slice would pin', (_label, filler) => {
const raw = retainedBytesForTails(filler, (value) => value)
const flattened = retainedBytesForTails(filler, flattenRetainedSlice)
// A raw slice pins all 8 parents; flattening must keep only the tails.
expect(raw).toBeGreaterThan(PARENTS * PARENT_CHARS * 0.5)
expect(flattened).toBeLessThan(PARENTS * PARENT_CHARS * 0.05)
})
})
@@ -0,0 +1,11 @@
/**
* V8 backs `String.prototype.slice` with a SlicedString that points at the whole parent, so a
* short tail of a large chunk keeps that whole chunk alive. Concatenating forces a ConsString
* whose flattening allocates a fresh sequential string of just this length, dropping the parent.
*
* Use this whenever a slice outlives the value it was cut from — a queued remainder, or an
* overlap-trimmed payload handed to a component that will hold it.
*/
export function flattenRetainedSlice(value: string): string {
return value.length === 0 ? value : `${value} `.slice(0, -1)
}
@@ -7,6 +7,7 @@ import {
} from './pane-terminal-foreground-render-settle'
import { runGuardedWriteCompletionStep } from './xterm-write-callback-guard'
import { recordRendererCrashBreadcrumb } from '@/lib/crash-breadcrumb-recorder'
import { flattenRetainedSlice } from '@/lib/flatten-retained-slice'
import {
discardInFlightTerminalOutputAckCredits,
registerTerminalOutputAckCredits
@@ -584,11 +585,6 @@ 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 = ''
@@ -744,6 +740,9 @@ function enqueueChunk(
): void {
entry.chunks.push({
data,
// Why data.length is the truth here: callers must hand the queue a string that owns its own
// storage. A caller passing a slice of a much larger buffer would understate what the queue
// pins, so producers flatten overlap-trimmed payloads before they reach the scheduler.
retainedChars: data.length,
foreground: options?.foreground === true,
forceForegroundRefresh: options?.forceForegroundRefresh === true,