diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index f6a276d566e..9b843723a2b 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -334,7 +334,6 @@ 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>() const SSH_SESSION_EXPIRED_ERROR = 'SSH_SESSION_EXPIRED' @@ -6777,9 +6776,7 @@ export function connectPanePty( if (rawLength !== chunk.data.length) { return null } - // 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)) + return chunk.data.slice(offset) } type RestoredSnapshotReconciliation = @@ -6829,9 +6826,7 @@ 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' } } - // 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)) + const sliced = data.slice(restoredSnapshotBaselineSeq - startSeq) return { action: 'write', data: sliced, diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts index 667b3f6555a..0faa13255e7 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.test.ts @@ -1693,6 +1693,34 @@ describe('pane terminal output scheduler', () => { return process.memoryUsage().heapUsed } + it('does not pin the parent when a producer enqueues a slice', async () => { + vi.useFakeTimers() + const { writeTerminalOutput } = await loadScheduler() + + // Why a slice: agent-status-osc.ts and the restore-overlap trims hand the scheduler + // strings cut from a much larger buffer. The queue must own its copy rather than + // trusting every producer to flatten first (STA-3567 review round 2). + const KEEP_CHARS = 64 * 1024 + const sinks = Array.from({ length: TERMINALS }, () => ({ + write: (_data: string, callback?: () => void) => callback?.() + })) + + const baseline = collect() + + for (const [index, sink] of sinks.entries()) { + const parent = String.fromCharCode(65 + index) + 'q'.repeat(CHUNK_CHARS - 1) + writeTerminalOutput(sink, parent.slice(parent.length - KEEP_CHARS), { + foreground: false, + latencySensitive: false + }) + } + + const retainedBytes = collect() - baseline + + // 8 x 64 KB of real payload must not keep 8 x 2 MB of parents alive. + expect(retainedBytes).toBeLessThan(4 * 1024 * 1024) + }) + it('drops the parent chunk once only a small tail is still queued', async () => { vi.useFakeTimers() const { writeTerminalOutput } = await loadScheduler() diff --git a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts index 3ba46995f69..f0c6b94ffc8 100644 --- a/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts +++ b/src/renderer/src/lib/pane-manager/pane-terminal-output-scheduler.ts @@ -698,12 +698,14 @@ function enqueueChunk( ackCredit?: () => void } ): void { + // Why copy at the boundary: producers hand us slices and concatenations that pin far more than + // they measure - restore-overlap trims and OSC status stripping both do - and the queue holds + // this string long after the parent is otherwise dead. Owning our copy is what makes + // retainedChars true, and it costs ~0.1ms per 512KB against a ~30MB/s sustained drain. + const owned = flattenRetainedSlice(data) 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, + data: owned, + retainedChars: owned.length, foreground: options?.foreground === true, forceForegroundRefresh: options?.forceForegroundRefresh === true, followupForegroundRefresh: options?.followupForegroundRefresh === true,