diff --git a/src/web/public/terminal-tile.js b/src/web/public/terminal-tile.js index e7d6f8a7..ff4e07c8 100644 --- a/src/web/public/terminal-tile.js +++ b/src/web/public/terminal-tile.js @@ -24,42 +24,58 @@ // How long a scroll-to-top history pull may hold Pane B's live output. const HISTORY_PULL_TIMEOUT_MS = 10000; + // How much of a replay is queued in xterm at once: a 1 MiB load goes in one + // window, and xterm's write queue throws past 50 MB, which an unbounded + // `full=1` capture (up to the server's 32 MB) would otherwise come near. + const REPLAY_WINDOW_BYTES = 1024 * 1024; + /** - * Minimal chunked write for Pane B's own xterm instance — write() in - * TERMINAL_CHUNK_SIZE slices, yielding a frame between each, instead of one - * giant synchronous write that blocks the main thread while parsing a long - * scrollback. Deliberately NOT the primary pane's chunkedTerminalWrite - * (terminal-ui.js): that one is wired into session-switch generation - * counters and the live-output gate this simpler, independently - * created/destroyed pane has no equivalent of. + * Replays a capture into a pane's own xterm: TERMINAL_CHUNK_SIZE slices, all + * of a window queued at once. xterm 6 parses its write queue in 12 ms slices + * and yields between them, so a long scrollback never becomes a long task, + * and it is not held to one slice per animation frame either (that pacing + * took about a second per 1 MiB, with the grid's load queue waiting behind + * it). Queued up front, the capture also stays in one piece: live output + * written during the parse lands after it, not between two of its slices. + * Deliberately NOT the primary pane's chunkedTerminalWrite (terminal-ui.js): + * that one is wired into session-switch generation counters and the + * live-output gate this simpler, independently created/destroyed pane has no + * equivalent of. + * + * Resolves once xterm has parsed the last slice (a write's callback runs once + * everything queued before it is parsed), so _loadBuffer() below holds its + * single-flight flag across the whole replay. A disposed xterm never runs its + * callbacks, so `setCancel` hands the owner a function that settles the + * replay at once: destroy() calls it, or the pane's flag and the grid's load + * queue would wait forever. */ - function writeChunked(terminal, buffer, isDestroyed) { - if (!buffer) return Promise.resolve(); - if (buffer.length <= TERMINAL_CHUNK_SIZE) { - terminal.write(buffer); - return Promise.resolve(); - } - // Resolves once the LAST chunk is written (or the pane was destroyed - // mid-replay), so _loadBuffer() below can hold its single-flight flag - // across the whole replay rather than just the fetch that precedes it. + function writeChunked(terminal, buffer, isDestroyed, setCancel) { + if (!buffer || !terminal) return Promise.resolve(); return new Promise((resolve) => { let offset = 0; - const writeNext = () => { - if (isDestroyed() || !terminal) { - resolve(); + let settled = false; + const settle = () => { + if (settled) return; + settled = true; + setCancel?.(null); + resolve(); + }; + const writeWindow = () => { + if (settled) return; + if (isDestroyed()) { + settle(); return; } - const chunk = buffer.slice(offset, offset + TERMINAL_CHUNK_SIZE); - offset += chunk.length; - terminal.write(chunk); - if (offset < buffer.length) { - if (typeof requestAnimationFrame === 'function') requestAnimationFrame(writeNext); - else setTimeout(writeNext, 16); - } else { - resolve(); + const end = Math.min(buffer.length, offset + REPLAY_WINDOW_BYTES); + while (offset < end) { + const chunk = buffer.slice(offset, Math.min(end, offset + TERMINAL_CHUNK_SIZE)); + offset += chunk.length; + terminal.write(chunk); } + terminal.write('', offset < buffer.length ? writeWindow : settle); }; - writeNext(); + setCancel?.(settle); + writeWindow(); }); } @@ -103,6 +119,9 @@ // Aborts the running load's fetch; destroy() uses it so a removed tile // does not hold the owner's queue for a whole deadline. this._loadAbort = null; + // Settles a replay xterm is still parsing (writeChunked): destroy() calls + // it, because a disposed xterm never runs the callback the replay awaits. + this._cancelReplay = null; // Scroll-to-top history pull (shell panes only), see _maybeLoadMoreHistory(). // `_liveQueue` is non-null from the pull's response until its finally // block: live frames are held there with their arrival time instead of @@ -640,7 +659,12 @@ this._loadAbort = null; } if (payload.terminalBuffer && this.terminal) { - await writeChunked(this.terminal, payload.terminalBuffer, () => this._destroyed); + await writeChunked( + this.terminal, + payload.terminalBuffer, + () => this._destroyed, + (cancel) => (this._cancelReplay = cancel) + ); } } catch { /* Best-effort: live output still arrives once the socket connects. */ @@ -825,7 +849,12 @@ term.write('\x1bc'); replayed = true; if (this._wsClosed) this._markerOwed = true; - await writeChunked(term, buffer, () => this._destroyed); + await writeChunked( + term, + buffer, + () => this._destroyed, + (cancel) => (this._cancelReplay = cancel) + ); if (this._destroyed || !this.terminal) return; // xterm parses asynchronously: an empty write's callback fires only // after everything before it, so the row count below is the settled one. @@ -964,6 +993,10 @@ /* Already settled. */ } this._loadAbort = null; + // Likewise a replay still parsing: the xterm is disposed below, so the + // write callback it waits for would never come. + this._cancelReplay?.(); + this._cancelReplay = null; if (this._onWheel) { this.mountEl?.removeEventListener('wheel', this._onWheel, { capture: true }); this._onWheel = null; diff --git a/test/terminal-tile-unit.test.ts b/test/terminal-tile-unit.test.ts index 17bb55de..07a46294 100644 --- a/test/terminal-tile-unit.test.ts +++ b/test/terminal-tile-unit.test.ts @@ -70,8 +70,6 @@ type PaneUnderTest = { }; const fetchMock = vi.fn(); -/** requestAnimationFrame stand-in: chunked writes queue here and are drained by hand. */ -const rafQueue: Array<() => void> = []; /** Recorded deadline timers (see the context's setTimeout); `fn` aborts the request. */ const deadlines: Array<{ fn: () => void; ms: number; cleared: boolean }> = []; const SOURCE = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-tile.js'), 'utf8'); @@ -105,7 +103,6 @@ function loadTerminalTile() { else clearTimeout(id as Parameters[0]); }, fetch: (...args: unknown[]) => fetchMock(...args), - requestAnimationFrame: (fn: () => void) => rafQueue.push(fn), // The constants.js globals the module reads at call time. TERMINAL_CHUNK_SIZE, TERMINAL_TAIL_SIZE, @@ -179,12 +176,36 @@ function deferred() { // Every marker variant (reconnecting, session ended, refused, taken over) starts the same way. const isMarker = (data: unknown) => typeof data === 'string' && data.includes('[disconnected'); +/** + * What reached the pane's screen. A replay also queues an empty write, only to + * hear through its callback that everything before it has been parsed + * (writeChunked); it puts nothing on screen, so it is left out. + */ +const screenWrites = (pane: { terminal: FakeTerminal }) => + pane.terminal.write.mock.calls.map((call) => call[0]).filter((data) => data !== ''); + +/** + * Holds xterm's write callbacks, as a real xterm still parsing a replay does: + * the replay stays in progress until `parse()` runs the ones held so far. + */ +function holdParses(pane: { terminal: FakeTerminal }) { + const held: Array<() => void> = []; + pane.terminal.write = vi.fn((_data: string, done?: () => void) => { + if (done) held.push(done); + }); + return { + held, + parse: () => { + for (const done of held.splice(0)) done(); + }, + }; +} + /** Lets every microtask the vm-side promise chain queued run. */ const settle = () => new Promise((r) => setTimeout(r, 0)); beforeEach(() => { fetchMock.mockReset(); - rafQueue.length = 0; deadlines.length = 0; clock = 0; }); @@ -265,40 +286,88 @@ describe('TerminalTile server-refresh single-flight', () => { second.resolve(jsonResponse('replay-2')); await settle(); - expect(pane.terminal.write).toHaveBeenLastCalledWith('replay-2'); + expect(screenWrites(pane).at(-1)).toBe('replay-2'); expect(fetchMock).toHaveBeenCalledTimes(2); expect(pane._bufferLoading).toBe(false); expect(pane._bufferRefreshPending).toBe(false); }); - it('holds the flag across the chunked write, not just the fetch', async () => { + it('queues the whole replay at once and holds the flag until xterm has parsed it', async () => { const pane = makePane(); - // Three chunks: two full ones plus a tail, so the last two are queued on - // requestAnimationFrame and the replay is mid-write after the fetch lands. + const xterm = holdParses(pane); + // Three slices: two full ones plus a tail. const big = 'x'.repeat(TERMINAL_CHUNK_SIZE * 2 + 5); fetchMock.mockResolvedValueOnce(jsonResponse(big)); pane._refreshBuffer(); await settle(); - expect(pane.terminal.write).toHaveBeenCalledTimes(1); - expect(rafQueue).toHaveLength(1); + // Every slice queued at once, then the empty write whose callback ends the + // replay: nothing waits for an animation frame. + expect(pane.terminal.write.mock.calls.map((call) => call[0].length)).toEqual([ + TERMINAL_CHUNK_SIZE, + TERMINAL_CHUNK_SIZE, + 5, + 0, + ]); + // xterm is still parsing: the replay, and with it the flag, is not done. expect(pane._bufferLoading).toBe(true); - // A refresh mid-write must not clear the terminal under the chunks still - // to come, nor start a second fetch. + // A refresh mid-parse must not clear the terminal under the replay, nor + // start a second fetch. pane._refreshBuffer(); expect(pane.terminal.clear).toHaveBeenCalledTimes(1); expect(fetchMock).toHaveBeenCalledTimes(1); fetchMock.mockResolvedValueOnce(jsonResponse('after')); - rafQueue.shift()!(); - rafQueue.shift()!(); + xterm.parse(); await settle(); - - expect(pane.terminal.write).toHaveBeenCalledTimes(4); - expect(pane.terminal.write).toHaveBeenLastCalledWith('after'); + // Parsed: the coalesced refresh runs now, once. expect(pane.terminal.clear).toHaveBeenCalledTimes(2); expect(fetchMock).toHaveBeenCalledTimes(2); + expect(pane._bufferLoading).toBe(true); + + xterm.parse(); + await settle(); + expect(screenWrites(pane).at(-1)).toBe('after'); + expect(pane._bufferLoading).toBe(false); + }); + + it('queues a replay larger than the window one window at a time', async () => { + // xterm's write queue throws past 50 MB, and an unbounded `full=1` capture + // can reach the server's 32 MB: at most 1 MiB is queued before xterm has + // parsed what came before it. + const pane = makePane(); + const xterm = holdParses(pane); + fetchMock.mockResolvedValueOnce(jsonResponse('z'.repeat(TERMINAL_TAIL_SIZE + 5))); + + pane._refreshBuffer(); + await settle(); + const queued = () => pane.terminal.write.mock.calls.reduce((n, call) => n + call[0].length, 0); + expect(queued()).toBe(TERMINAL_TAIL_SIZE); + + xterm.parse(); + await settle(); + expect(queued()).toBe(TERMINAL_TAIL_SIZE + 5); + expect(pane._bufferLoading).toBe(true); + + xterm.parse(); + await settle(); + expect(pane._bufferLoading).toBe(false); + }); + + it('a replay still parsing when the pane is destroyed settles at once', async () => { + // A disposed xterm never runs a write callback: without destroy() settling + // the replay, the flag (and in the grid the one load queue) would wait forever. + const pane = makePane(); + holdParses(pane); + fetchMock.mockResolvedValueOnce(jsonResponse('replay')); + + pane._refreshBuffer(); + await settle(); + expect(pane._bufferLoading).toBe(true); + + pane.destroy(); + await settle(); expect(pane._bufferLoading).toBe(false); }); @@ -503,6 +572,7 @@ describe('TerminalTile scroll-to-top history pull', () => { it('holds live output during the replay and replays only what arrived after the capture', async () => { const pane = makePane('shell'); const term = pane.terminal; + const xterm = holdParses(pane); const response = deferred>(); fetchMock.mockReturnValueOnce(response.promise); @@ -518,24 +588,26 @@ describe('TerminalTile scroll-to-top history pull', () => { await settle(); // 200 rows (more than the pane holds, so it replays) of 400 columns each: - // three chunks, which leaves the replay mid-write once the fetch lands. + // three chunks, still being parsed once the fetch lands. const bigReplay = Array.from({ length: 200 }, () => 'y'.repeat(400)).join('\n'); expect(bigReplay.length).toBeGreaterThan(TERMINAL_CHUNK_SIZE * 2); clock = 2; // the response arrives: this is the cutoff response.resolve(jsonResponse(bigReplay)); await settle(); - expect(rafQueue).toHaveLength(1); + expect(xterm.held).toHaveLength(1); - // Arrives while the snapshot is still being written: must not land under it. + // Arrives while the snapshot is still being parsed: must not land under it. clock = 3; pane._onLiveOutput('late'); expect(term.write).not.toHaveBeenCalledWith('late'); - rafQueue.shift()!(); - rafQueue.shift()!(); + // The replay parsed, then the pull's own settle write before it scrolls. + xterm.parse(); + await settle(); + xterm.parse(); await settle(); - const written = term.write.mock.calls.map((call) => call[0]); + const written = screenWrites(pane); // 'early' went out before the reset, so the replay wiped it and it is not repeated. expect(written.indexOf('early')).toBeLessThan(written.indexOf('\x1bc')); expect(written.filter((w) => w === 'early')).toHaveLength(1); @@ -831,7 +903,7 @@ describe('TerminalTile scroll-to-top history pull', () => { second.resolve(jsonResponse('second')); await settle(); - const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + const writes = screenWrites(pane); expect(writes.at(-1)).toSatisfy(isMarker); expect(writes.lastIndexOf('second')).toBe(writes.length - 2); expect(writes.filter(isMarker)).toHaveLength(1); @@ -887,25 +959,27 @@ describe('TerminalTile scroll-to-top history pull', () => { } ); - it('a close during the chunked replay writes exactly one marker, at the end', async () => { + it('a close during the replay writes exactly one marker, at the end', async () => { const pane = makePane('shell'); + const xterm = holdParses(pane); const response = deferred>(); fetchMock.mockReturnValueOnce(response.promise); const pull = pane._pullHistory(); - // Three chunks, so the replay is still mid-write once the fetch lands. + // Three chunks, still being parsed once the fetch lands. const bigReplay = Array.from({ length: 200 }, () => 'y'.repeat(400)).join('\n'); response.resolve(jsonResponse(bigReplay)); await settle(); - expect(rafQueue).toHaveLength(1); + expect(xterm.held).toHaveLength(1); - // Written now, the marker would land between two chunks of recovered history. + // Written now, the marker would land above the recovered history's end. pane._onSocketClosed(); - rafQueue.shift()!(); - rafQueue.shift()!(); + xterm.parse(); + await settle(); + xterm.parse(); await pull; - const writes = pane.terminal.write.mock.calls.map((c) => c[0]); + const writes = screenWrites(pane); expect(writes[0]).toBe('\x1bc'); expect(writes.filter(isMarker)).toHaveLength(1); expect(isMarker(writes.at(-1))).toBe(true); diff --git a/test/tile-grid-load-queue.test.ts b/test/tile-grid-load-queue.test.ts index ddb84fb1..59b564e0 100644 --- a/test/tile-grid-load-queue.test.ts +++ b/test/tile-grid-load-queue.test.ts @@ -86,9 +86,13 @@ class FakeTerminal { attachCustomKeyEventHandler() {} registerLinkProvider() {} textarea = { addEventListener() {}, removeEventListener() {} }; + /** Set by a test: write callbacks never run, as on a disposed xterm. */ + holdParse = false; write(data: string, cb?: () => void) { - this.writes.push(data); - cb?.(); + // An empty write puts nothing on screen; the replay queues one only to hear + // (its callback) that everything before it has been parsed. + if (data) this.writes.push(data); + if (!this.holdParse) cb?.(); } clear() { this.writes.push(''); @@ -412,6 +416,27 @@ describe('teardown', () => { await Promise.all(connecting); }); + it('destroying a tile while xterm still parses its replay settles the load, and the queue moves on', async () => { + // The replay waits for xterm's write callback (writeChunked), which a + // disposed xterm never runs: unsettled, the tile's load would hold the + // grid's one queue, and every other tile behind it, forever. + const { tiles } = makeGrid(['a', 'b']); + const connecting = tiles.map((t) => t.connect()); + await settle(); + tiles[0].terminal!.holdParse = true; + captures[0].answer('replay of a'); + await settle(); + // a's replay is still parsing: b waits its turn. + expect(captures.map((c) => c.url.split('/')[3])).toEqual(['a']); + + tiles[0].destroy(); + await settle(); + expect(captures.map((c) => c.url.split('/')[3])).toEqual(['a', 'b']); + captures[1].answer('b'); + await settle(); + await Promise.all(connecting); + }); + it('a capture that never answers is cut off by its deadline, and the next tile loads', async () => { vi.useFakeTimers(); const { tiles } = makeGrid(['a', 'b']);