Skip to content

Avoid replaying parsed writes when resize flushes the write queue - #6201

Open
nedtwigg wants to merge 2 commits into
xtermjs:masterfrom
diffplug:fix-resize-write-replay
Open

nedtwigg wants to merge 2 commits into
xtermjs:masterfrom
diffplug:fix-resize-write-replay

Conversation

@nedtwigg

@nedtwigg nedtwigg commented Oct 2, 2026

Copy link
Copy Markdown

resize() can repeat terminal output and invoke write callbacks twice, either between write-processing slices or when called from a write callback. For example, a queued git status echo followed by its output can render as git statusgit status.

This is a regression from #5599 ("Flush writes on resize"), introduced after stable 6.0.0. _innerWrite() retains parsed chunks before _bufferOffset when it yields, but flushSync() drains from index zero with shift(). A resize inside a write callback also sees the current chunk as pending because _innerWrite() has not advanced the offset yet.

This PR drains from _bufferOffset and makes _innerWrite() update the offset and pending-data count before invoking the callback. Both resize paths then avoid replaying chunks or invoking their callbacks twice.

The length-based loop also fixes data loss: an empty string currently stops the flush, permanently discarding all later queued output and outstanding callbacks. write('', callback) is used to wait for queued writes to finish, including by Dormouse's production flushTerminal(). A resize while that empty write is queued can leave its promise unresolved.

The investigation started with an intermittent Dormouse snapshot failure. The headless regression reproduces the same doubled command by forcing a write-processing yield.

Related to #6154: this fixes its consumed-prefix replay, but leaves the separate async-parser failure unresolved. The maintainer discussion proposes removing resize's flush or making it stop at the first async handler; another contributor has been invited to work on that issue. Offset-aware draining is also needed for the latter approach; removing resize's flush would make this change unnecessary for that path. This PR is limited to queue bookkeeping and does not implement that broader change.

Test plan

  • Regression tests fail before their fixes: yielded writes replay, callback-triggered flushing replays the current chunk, and an empty string discards the remaining queue.
  • Queue coverage includes the scheduled continuation after a flush, subsequent writes, empty string/byte chunks, and writes appended by callbacks. A headless test reproduces the visible doubled echo.
  • npm run build && npm run esbuild, npm run test-unit, npm run lint, and npm run lint-api.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant