mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-30 11:52:04 +00:00
When the previous frame's stdout.write has not drained (the outer terminal parser is overwhelmed by a wide CR+LF burst — CJK + ANSI tool output on a high-context session), the renderer kept writing a new frame every tick. That piled writes onto an already-backed-up pipe and kept the macrotask queue hot, starving the stdin 'readable' callback — the observed stdin freeze where the agent loop keeps running but keystrokes/Ctrl-C are dead. onRender now coalesces: while pendingWriteStart is non-null (prior write's drain callback hasn't fired) it skips the frame and retries on the drain tick instead of writing. A MAX_COALESCED_BACKPRESSURE_FRAMES ceiling forces a write through after N skips so a terminal whose drain callback never fires (OSError EIO on flush) self-heals once the pipe recovers rather than wedging forever. TTY-only; piped stdout has no flow control. Coalesce counter resets on every real write. This is the stdout-backpressure strand left open after #54046 fixed the swallowed-exception strand.
This commit is contained in:
parent
a06d0198cd
commit
5e7bca95d9
3 changed files with 243 additions and 1 deletions
|
|
@ -4,3 +4,16 @@ export const FRAME_INTERVAL_MS = 16
|
|||
// Keep clock-driven animations at full speed when terminal focus changes.
|
||||
// We still pause entirely when there are no keepAlive subscribers.
|
||||
export const BLURRED_FRAME_INTERVAL_MS = FRAME_INTERVAL_MS
|
||||
|
||||
// Issue #31486 (stdout-backpressure strand): when the previous frame's
|
||||
// stdout.write has NOT drained yet (terminal parser overwhelmed by a wide
|
||||
// CR+LF burst — CJK + ANSI tool output on a high-context session), piling
|
||||
// another write on the backed-up pipe both wastes the frame and keeps the
|
||||
// macrotask queue churning, starving the stdin 'readable' callback. We
|
||||
// instead COALESCE: skip the frame and retry on the drain tick. This ceiling
|
||||
// caps how many consecutive frames we'll coalesce before forcing a write
|
||||
// through, so a terminal whose drain callback never fires (e.g. EIO on
|
||||
// flush) can't wedge the renderer permanently — it self-heals once the pipe
|
||||
// recovers. ~10 frames at the drain-tick cadence is a few hundred ms of
|
||||
// breathing room, well under any human-perceptible render stall.
|
||||
export const MAX_COALESCED_BACKPRESSURE_FRAMES = 10
|
||||
|
|
|
|||
194
ui-tui/packages/hermes-ink/src/ink/ink-backpressure.test.ts
Normal file
194
ui-tui/packages/hermes-ink/src/ink/ink-backpressure.test.ts
Normal file
|
|
@ -0,0 +1,194 @@
|
|||
import { EventEmitter } from 'events'
|
||||
|
||||
import React from 'react'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import Text from './components/Text.js'
|
||||
import { MAX_COALESCED_BACKPRESSURE_FRAMES } from './constants.js'
|
||||
import Ink from './ink.js'
|
||||
|
||||
// Regression for issue #31486 (stdout-backpressure strand): when the
|
||||
// previous frame's stdout.write has not drained (the terminal parser is
|
||||
// overwhelmed — a wide CR+LF burst on a high-context session), the renderer
|
||||
// must COALESCE rather than pile another write on the backed-up pipe. Piling
|
||||
// writes keeps the macrotask queue hot and starves the stdin 'readable'
|
||||
// callback, which is the observed freeze. The coalesce must be bounded: after
|
||||
// MAX_COALESCED_BACKPRESSURE_FRAMES skipped frames it forces a write through
|
||||
// so a terminal whose drain callback never fires can't wedge the renderer.
|
||||
|
||||
/**
|
||||
* A TTY whose write() reports backpressure (returns false) and WITHHOLDS the
|
||||
* drain callback until fireDrain() is called — simulating a wedged terminal
|
||||
* parser. Each write records its drain callback so the test controls timing.
|
||||
*/
|
||||
class WedgedTty extends EventEmitter {
|
||||
chunks: string[] = []
|
||||
columns = 20
|
||||
rows = 5
|
||||
isTTY = true
|
||||
private pendingDrains: Array<(err?: Error | null) => void> = []
|
||||
|
||||
write(chunk: string | Uint8Array, cb?: (err?: Error | null) => void): boolean {
|
||||
this.chunks.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8'))
|
||||
|
||||
if (cb) {
|
||||
// Hold the callback — do NOT fire it. This leaves the renderer's
|
||||
// pendingWriteStart non-null, the backpressure signal it coalesces on.
|
||||
this.pendingDrains.push(cb)
|
||||
}
|
||||
|
||||
// Report backpressure.
|
||||
return false
|
||||
}
|
||||
|
||||
/** Fire all withheld drain callbacks, simulating the pipe recovering. */
|
||||
fireDrain(): void {
|
||||
const drains = this.pendingDrains
|
||||
this.pendingDrains = []
|
||||
|
||||
for (const cb of drains) {
|
||||
cb()
|
||||
}
|
||||
}
|
||||
|
||||
get pendingDrainCount(): number {
|
||||
return this.pendingDrains.length
|
||||
}
|
||||
}
|
||||
|
||||
/** A normal fast TTY: write succeeds and drains synchronously. */
|
||||
class FastTty extends EventEmitter {
|
||||
chunks: string[] = []
|
||||
columns = 20
|
||||
rows = 5
|
||||
isTTY = true
|
||||
|
||||
write(chunk: string | Uint8Array, cb?: (err?: Error | null) => void): boolean {
|
||||
this.chunks.push(typeof chunk === 'string' ? chunk : Buffer.from(chunk).toString('utf8'))
|
||||
cb?.()
|
||||
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
const makeInk = (stdout: WedgedTty | FastTty) => {
|
||||
const stdin = new EventEmitter() as unknown as NodeJS.ReadStream
|
||||
const stderr = new FastTty()
|
||||
|
||||
return new Ink({
|
||||
exitOnCtrlC: false,
|
||||
patchConsole: false,
|
||||
stderr: stderr as unknown as NodeJS.WriteStream,
|
||||
stdin,
|
||||
stdout: stdout as unknown as NodeJS.WriteStream
|
||||
})
|
||||
}
|
||||
|
||||
describe('Ink stdout backpressure coalescing (issue #31486)', () => {
|
||||
it('coalesces frames while the previous write has not drained', () => {
|
||||
vi.useFakeTimers()
|
||||
|
||||
try {
|
||||
const stdout = new WedgedTty()
|
||||
const ink = makeInk(stdout)
|
||||
|
||||
ink.render(React.createElement(Text, null, 'hello'))
|
||||
ink.onRender()
|
||||
|
||||
// First frame wrote (and reported backpressure; drain withheld).
|
||||
const writesAfterFirst = stdout.chunks.length
|
||||
expect(writesAfterFirst).toBeGreaterThan(0)
|
||||
expect(stdout.pendingDrainCount).toBe(1)
|
||||
|
||||
// Subsequent renders while the write is still pending must coalesce —
|
||||
// no new bytes written, a retry timer scheduled instead.
|
||||
ink.render(React.createElement(Text, null, 'world'))
|
||||
ink.onRender()
|
||||
expect(stdout.chunks.length).toBe(writesAfterFirst)
|
||||
|
||||
ink.onRender()
|
||||
expect(stdout.chunks.length).toBe(writesAfterFirst)
|
||||
|
||||
ink.unmount()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('resumes writing once the wedged pipe drains', () => {
|
||||
vi.useFakeTimers()
|
||||
|
||||
try {
|
||||
const stdout = new WedgedTty()
|
||||
const ink = makeInk(stdout)
|
||||
|
||||
ink.render(React.createElement(Text, null, 'hello'))
|
||||
ink.onRender()
|
||||
const writesAfterFirst = stdout.chunks.length
|
||||
|
||||
// Backed up: this render coalesces.
|
||||
ink.render(React.createElement(Text, null, 'changed'))
|
||||
ink.onRender()
|
||||
expect(stdout.chunks.length).toBe(writesAfterFirst)
|
||||
|
||||
// Pipe recovers — drain callback fires, clearing pendingWriteStart.
|
||||
stdout.fireDrain()
|
||||
|
||||
// The retry tick now finds the pipe drained and writes the pending frame.
|
||||
vi.runAllTimers()
|
||||
expect(stdout.chunks.length).toBeGreaterThan(writesAfterFirst)
|
||||
|
||||
ink.unmount()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('forces a write through after the coalesce ceiling so it never wedges forever', () => {
|
||||
vi.useFakeTimers()
|
||||
|
||||
try {
|
||||
const stdout = new WedgedTty()
|
||||
const ink = makeInk(stdout)
|
||||
|
||||
ink.render(React.createElement(Text, null, 'hello'))
|
||||
ink.onRender()
|
||||
const writesAfterFirst = stdout.chunks.length
|
||||
|
||||
// Mark content dirty and drive renders. The drain callback NEVER fires
|
||||
// (pendingDrainCount stays > 0). After MAX_COALESCED_BACKPRESSURE_FRAMES
|
||||
// coalesced retries, the renderer must force a write through.
|
||||
ink.render(React.createElement(Text, null, 'forced'))
|
||||
|
||||
// Drive enough retry ticks to exceed the ceiling.
|
||||
for (let i = 0; i <= MAX_COALESCED_BACKPRESSURE_FRAMES + 2; i++) {
|
||||
vi.advanceTimersByTime(4)
|
||||
}
|
||||
|
||||
// A write was forced through despite the never-firing drain callback.
|
||||
expect(stdout.chunks.length).toBeGreaterThan(writesAfterFirst)
|
||||
|
||||
ink.unmount()
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('never coalesces on a fast terminal that drains synchronously', () => {
|
||||
const stdout = new FastTty()
|
||||
const ink = makeInk(stdout)
|
||||
|
||||
ink.render(React.createElement(Text, null, 'a'))
|
||||
ink.onRender()
|
||||
const afterA = stdout.chunks.length
|
||||
expect(afterA).toBeGreaterThan(0)
|
||||
|
||||
// Each changed render writes immediately — synchronous drain clears the
|
||||
// backpressure signal before the next frame, so nothing is coalesced.
|
||||
ink.render(React.createElement(Text, null, 'b'))
|
||||
ink.onRender()
|
||||
expect(stdout.chunks.length).toBeGreaterThan(afterA)
|
||||
|
||||
ink.unmount()
|
||||
})
|
||||
})
|
||||
|
|
@ -18,7 +18,7 @@ import { colorize } from './colorize.js'
|
|||
import App from './components/App.js'
|
||||
import type { CursorAdvanceNotifier } from './components/CursorAdvanceContext.js'
|
||||
import type { CursorDeclaration, CursorDeclarationSetter } from './components/CursorDeclarationContext.js'
|
||||
import { FRAME_INTERVAL_MS } from './constants.js'
|
||||
import { FRAME_INTERVAL_MS, MAX_COALESCED_BACKPRESSURE_FRAMES } from './constants.js'
|
||||
import * as dom from './dom.js'
|
||||
import { markDirty } from './dom.js'
|
||||
import { KeyboardEvent } from './events/keyboard-event.js'
|
||||
|
|
@ -205,6 +205,11 @@ export default class Ink {
|
|||
// (callback fired).
|
||||
private pendingWriteStart: number | null = null
|
||||
private lastDrainMs = 0
|
||||
// Issue #31486: count of consecutive frames skipped because the previous
|
||||
// write hadn't drained. Reset to 0 whenever a frame actually writes (or the
|
||||
// pipe has drained). Capped by MAX_COALESCED_BACKPRESSURE_FRAMES so a
|
||||
// never-firing drain callback can't coalesce forever.
|
||||
private coalescedBackpressureFrames = 0
|
||||
private lastYogaCounters: {
|
||||
ms: number
|
||||
visited: number
|
||||
|
|
@ -703,6 +708,36 @@ export default class Ink {
|
|||
this.drainTimer = null
|
||||
}
|
||||
|
||||
// Issue #31486 (stdout-backpressure strand): if the PREVIOUS frame's
|
||||
// stdout.write still hasn't drained (callback hasn't fired —
|
||||
// pendingWriteStart is non-null), the outer terminal is consuming bytes
|
||||
// slower than we're producing them. Piling another write on the backed-up
|
||||
// pipe is wasted work AND keeps the macrotask queue hot, which is what
|
||||
// starves the stdin 'readable' callback and wedges input. Coalesce:
|
||||
// skip this frame's render+write entirely and retry on the drain tick.
|
||||
// The ceiling guarantees forward progress — after N coalesced frames we
|
||||
// force the write through, so a terminal whose drain callback NEVER fires
|
||||
// (e.g. OSError EIO on flush) self-heals once the pipe recovers instead of
|
||||
// coalescing forever. Only on a TTY; piped stdout has no flow control and
|
||||
// pendingWriteStart is never set there.
|
||||
if (
|
||||
this.options.stdout.isTTY &&
|
||||
this.pendingWriteStart !== null &&
|
||||
this.coalescedBackpressureFrames < MAX_COALESCED_BACKPRESSURE_FRAMES
|
||||
) {
|
||||
this.coalescedBackpressureFrames += 1
|
||||
this.isRendering = false
|
||||
// Retry at the same cadence as a scroll drain tick. Don't use
|
||||
// scheduleRender — lodash throttle's leading edge would re-enter here.
|
||||
this.drainTimer = setTimeout(() => this.onRender(), FRAME_INTERVAL_MS >> 2)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// Either we wrote, or we hit the ceiling and are forcing a write through.
|
||||
// Reset the coalesce counter so the next backpressure episode starts fresh.
|
||||
this.coalescedBackpressureFrames = 0
|
||||
|
||||
// Flush deferred interaction-time update before rendering so we call
|
||||
// Date.now() at most once per frame instead of once per keypress.
|
||||
// Done before the render to avoid dirtying state that would trigger
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue