fix(terminal): release parent chunks pinned by drained output slices

takeQueuedChunk stored the unconsumed remainder back into the queue as
chunk.data.slice(remaining). V8 backs that with a SlicedString pointing at
the whole parent, so a nearly drained 2 MB chunk kept all 2 MB alive to serve
a few KB tail while queuedChars was charged only for the remainder. Fully
consumed chunks were also left referenced until the 64-chunk compaction, so
they retained their strings and callbacks while charging nothing at all.

Track what each chunk really pins in retainedChars and copy the residual flat
once it pins twice its own length. Halving bounds retention to ~2x the honest
queued chars and keeps total copying linear in the original chunk, and the
drain path measured no slower. Drained slots are replaced with an empty
placeholder, which nothing below chunkIndex ever reads.

Measured with 8 background terminals drained to a <=64 KB tail:
8,710,264 bytes retained before, 342,776 after.
This commit is contained in:
Brennan Benson
2026-08-07 06:15:14 -07:00
parent 02a1251c2d
commit c4db330225
3 changed files with 103 additions and 2 deletions
+2 -1
View File
@@ -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: [
@@ -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)
})
})
})
@@ -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,