mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 00:03:45 +00:00
fix(crash): count the terminal buffer that holds the scrollback
The live-terminal census read `buffer.active`, which is the *alternate*
buffer whenever an alt-screen app is up — vim, less, htop, an agent TUI.
That buffer is viewport-sized and holds no scrollback: measured on
@xterm/headless, 24 rows against `normal`'s 5001. So the reader cleared
terminals of exactly the retention it exists to find, and did so hardest
under the workloads most likely to be leaking.
Read `buffer.normal` instead, and report `altScreenPanes` as its own
count rather than folding the two facts into one number.
Also commit `panes`/`lines`/`cells` together at the end of the read, so a
throw partway cannot leave a pane counted with its lines missing — which
reads as a pane retaining nothing. The catch stays, with a corrected
reason: a disposed xterm does not throw (measured — it still returns
`normal.length` 5001), but the pane is reached through a caller that may
be tearing it down, and one bad pane must not sink its siblings.
Tests: `pane-manager-registry.test.ts` fixtures moved from a bare
`{ buffer: { active: { length } } }` to `normal` plus an `active.type`
discriminator, and gained alt-screen cases. Three assertions moved
because they pinned the wrong buffer; two more moved on shape alone,
from the added `altScreenPanes` return key.
This commit is contained in:
@@ -252,8 +252,14 @@ describe('live terminal buffer census', () => {
|
||||
registeredManagers.push(manager)
|
||||
}
|
||||
|
||||
function terminal(cols: number, lines: number): unknown {
|
||||
return { cols, buffer: { active: { length: lines } } }
|
||||
function terminal(cols: number, lines: number, altScreen = false): unknown {
|
||||
return {
|
||||
cols,
|
||||
buffer: {
|
||||
normal: { length: lines },
|
||||
active: { type: altScreen ? 'alternate' : 'normal' }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
@@ -273,52 +279,87 @@ describe('live terminal buffer census', () => {
|
||||
expect(getLiveTerminalBufferCensus()).toEqual({
|
||||
panes: 2,
|
||||
lines: 50_024,
|
||||
cells: 6_002_880
|
||||
cells: 6_002_880,
|
||||
altScreenPanes: 0
|
||||
})
|
||||
})
|
||||
|
||||
// Why: `buffer.active` is the alternate buffer while vim/less/an agent TUI is up,
|
||||
// and that buffer is viewport-sized — measured on @xterm/headless at 24 rows while
|
||||
// `normal` still held 5001. Reading `active` reports a pane retaining 50k lines as
|
||||
// retaining a screenful, which clears terminals of a leak they are causing.
|
||||
it('counts scrollback held behind an alt-screen app, not its viewport', () => {
|
||||
registerPanes(
|
||||
{ id: 1, terminal: terminal(120, 50_000, true) },
|
||||
{ id: 2, terminal: terminal(120, 24) }
|
||||
)
|
||||
|
||||
expect(getLiveTerminalBufferCensus()).toEqual({
|
||||
panes: 2,
|
||||
lines: 50_024,
|
||||
cells: 6_002_880,
|
||||
altScreenPanes: 1
|
||||
})
|
||||
})
|
||||
|
||||
it('reports into the renderer memory profile', () => {
|
||||
registerPanes({ id: 1, terminal: terminal(80, 5_000) })
|
||||
registerPanes({ id: 1, terminal: terminal(80, 5_000, true) })
|
||||
|
||||
expect(collectRendererMemoryProfileCounts()).toMatchObject({
|
||||
'liveTerminalBuffers.panes': 1,
|
||||
'liveTerminalBuffers.lines': 5_000,
|
||||
'liveTerminalBuffers.cells': 400_000
|
||||
'liveTerminalBuffers.cells': 400_000,
|
||||
'liveTerminalBuffers.altScreenPanes': 1
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps counting siblings when a disposed terminal throws on buffer access', () => {
|
||||
it('keeps counting siblings when a terminal throws on buffer access', () => {
|
||||
registerPanes(
|
||||
{
|
||||
id: 1,
|
||||
terminal: {
|
||||
get buffer(): never {
|
||||
throw new Error('disposed')
|
||||
throw new Error('torn down')
|
||||
}
|
||||
}
|
||||
},
|
||||
{ id: 2, terminal: terminal(100, 10) }
|
||||
)
|
||||
|
||||
expect(getLiveTerminalBufferCensus()).toEqual({ panes: 1, lines: 10, cells: 1_000 })
|
||||
expect(getLiveTerminalBufferCensus()).toEqual({
|
||||
panes: 1,
|
||||
lines: 10,
|
||||
cells: 1_000,
|
||||
altScreenPanes: 0
|
||||
})
|
||||
})
|
||||
|
||||
it('prefers a manager census over materializing public pane views', () => {
|
||||
const counted = {
|
||||
resetWebglTextureAtlases: vi.fn<() => void>(),
|
||||
getPaneBufferCensus: () => ({ panes: 2, lines: 30, cells: 3_000 }),
|
||||
getPaneBufferCensus: () => ({ panes: 2, lines: 30, cells: 3_000, altScreenPanes: 1 }),
|
||||
getPanes: vi.fn(() => [{ id: 1, terminal: terminal(80, 10) }])
|
||||
}
|
||||
registerLivePaneManager(counted)
|
||||
registeredManagers.push(counted)
|
||||
|
||||
expect(getLiveTerminalBufferCensus()).toEqual({ panes: 2, lines: 30, cells: 3_000 })
|
||||
expect(getLiveTerminalBufferCensus()).toEqual({
|
||||
panes: 2,
|
||||
lines: 30,
|
||||
cells: 3_000,
|
||||
altScreenPanes: 1
|
||||
})
|
||||
expect(counted.getPanes).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('tolerates a terminal with no buffer rather than emitting NaN', () => {
|
||||
registerPanes({ id: 1, terminal: {} })
|
||||
|
||||
expect(getLiveTerminalBufferCensus()).toEqual({ panes: 1, lines: 0, cells: 0 })
|
||||
expect(getLiveTerminalBufferCensus()).toEqual({
|
||||
panes: 1,
|
||||
lines: 0,
|
||||
cells: 0,
|
||||
altScreenPanes: 0
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -122,11 +122,15 @@ export function getLivePaneCensus(): { managers: number; panes: number } {
|
||||
* live terminals is therefore consistent with anything from megabytes to gigabytes
|
||||
* — the same ambiguity the scrollback census resolves for stored buffers, for the
|
||||
* larger live copy. Safe to run inside a near-OOM highwater breadcrumb.
|
||||
*
|
||||
* `lines`/`cells` count the normal buffer only; `altScreenPanes` says how many panes
|
||||
* were additionally holding a viewport-sized alternate buffer at collection time.
|
||||
*/
|
||||
export function getLiveTerminalBufferCensus(): TerminalBufferCensus {
|
||||
let panes = 0
|
||||
let lines = 0
|
||||
let cells = 0
|
||||
let altScreenPanes = 0
|
||||
for (const manager of liveManagers) {
|
||||
let census: TerminalBufferCensus
|
||||
try {
|
||||
@@ -139,8 +143,9 @@ export function getLiveTerminalBufferCensus(): TerminalBufferCensus {
|
||||
panes += census.panes
|
||||
lines += census.lines
|
||||
cells += census.cells
|
||||
altScreenPanes += census.altScreenPanes
|
||||
}
|
||||
return { panes, lines, cells }
|
||||
return { panes, lines, cells, altScreenPanes }
|
||||
}
|
||||
|
||||
registerRendererMemoryProfileContributor('liveTerminalBuffers', getLiveTerminalBufferCensus)
|
||||
|
||||
@@ -6,9 +6,17 @@
|
||||
* do not expose one. Both funnel through here so the two paths cannot drift.
|
||||
*/
|
||||
|
||||
type XtermBufferShape = { cols?: number; buffer?: { active?: { length?: number } } }
|
||||
type XtermBufferShape = {
|
||||
cols?: number
|
||||
buffer?: { active?: { type?: string }; normal?: { length?: number } }
|
||||
}
|
||||
|
||||
export type TerminalBufferCensus = { panes: number; lines: number; cells: number }
|
||||
export type TerminalBufferCensus = {
|
||||
panes: number
|
||||
lines: number
|
||||
cells: number
|
||||
altScreenPanes: number
|
||||
}
|
||||
|
||||
export function sumTerminalBufferSizes(
|
||||
managedPanes: Iterable<{ terminal: unknown }>
|
||||
@@ -16,17 +24,29 @@ export function sumTerminalBufferSizes(
|
||||
let panes = 0
|
||||
let lines = 0
|
||||
let cells = 0
|
||||
let altScreenPanes = 0
|
||||
for (const pane of managedPanes) {
|
||||
try {
|
||||
const terminal = pane.terminal as XtermBufferShape | undefined
|
||||
// `buffer.active.length` and `cols` are both O(1); safe on the crash path.
|
||||
const length = terminal?.buffer?.active?.length ?? 0
|
||||
// Why `normal` and not `active`: `active` is the alternate buffer whenever an
|
||||
// alt-screen app is up (vim, less, an agent TUI). That buffer is viewport-sized
|
||||
// and holds no scrollback — measured on @xterm/headless as 24 rows against
|
||||
// `normal`'s 5001 — so reading it clears terminals of a leak they are causing.
|
||||
const retained = terminal?.buffer?.normal?.length ?? 0
|
||||
// Why counted separately rather than folded in: one number cannot say both how
|
||||
// much is retained and which pane shape retained it.
|
||||
const onAltScreen = terminal?.buffer?.active?.type === 'alternate'
|
||||
// Why commit together: a throw mid-read must not leave a pane counted with its
|
||||
// lines missing, which reads as a pane retaining nothing.
|
||||
panes += 1
|
||||
lines += length
|
||||
cells += length * (terminal?.cols ?? 0)
|
||||
lines += retained
|
||||
cells += retained * (terminal?.cols ?? 0)
|
||||
altScreenPanes += onAltScreen ? 1 : 0
|
||||
} catch {
|
||||
// Why: a disposed terminal throws on buffer access; siblings still count.
|
||||
// Why kept although a disposed xterm returns values rather than throwing
|
||||
// (measured): `terminal` is reached through a pane the caller may be tearing
|
||||
// down, and one bad pane must not sink the census for its siblings.
|
||||
}
|
||||
}
|
||||
return { panes, lines, cells }
|
||||
return { panes, lines, cells, altScreenPanes }
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user