Skip to content

Fast IME typing drops a committed composition: _isSendingComposition cancels every pending send, not just one #6089

Description

@joonhoekim

CompositionHelper defers each finished composition to a setTimeout(…, 0) and guards every
pending send with a single shared boolean, _isSendingComposition. The code assumes that timer
runs before the next composition finishes, so that at most one send is ever in flight.
Nothing guarantees that. Under load, input events arrive in bursts and two compositions can
complete before the timer gets a turn, leaving two sends pending at once.

The next non-composition keystroke then calls _finalizeComposition(false), which sets that
boolean to false intending to cancel the pending send — and cancels all of them,
including one that has not run yet. The composition it was carrying is dropped silently: no
error, no partial glyph, the text simply never reaches the pty.

This is a scheduling bug, not an off-by-one in the offset arithmetic. The dropped composition's
slice would have been computed correctly had its callback been allowed to run. It is invisible
on an idle page, which is why a bare demo looks fine while a real application drops characters.

Details

  • Browser and browser version: Google Chrome 151.0.7922.71 (native Wayland, --enable-wayland-ime --wayland-text-input-version=3); originally found in VS Code 1.130.0 / Electron
  • OS version: NixOS, Wayland session (Hyprland 0.56.1), fcitx5 5.1.21 + fcitx5-hangul, Korean Dubeolsik
  • xterm.js version: reproduced on 6.0.0 (latest), 6.1.0-beta.292 (beta) and 6.1.0-beta.288; the code is unchanged on current master

Steps to reproduce

Standalone xterm.js in a plain page — no Electron, no pty, no framework. A Terminal, an
onData handler recording what the library emits, and a timer that blocks the main thread.

const term = new Terminal({ rows: 6 });
term.open(document.getElementById('term'));

const chunks = [];
term.onData(d => { chunks.push(d); term.write(d); render(chunks); });

// Stand in for a busy application. Without this the bug does not appear.
// Both 100ms-every-40ms and 80ms-every-100ms reproduce it; what matters is that
// input events queue during the block and are then delivered back to back.
setInterval(() => {
  const until = performance.now() + 100;
  while (performance.now() < until) { /* make input events queue up */ }
}, 40);
  1. Focus the terminal and switch to a Korean IME (fcitx5-hangul, Dubeolsik).
  2. Type 알겠습니다. — including the trailing period — at a normal-to-fast pace.

Expected: ["알", "겠", "습", "니", "다", "."]

Actual: ["알", "겠", "습", "다", "."] — the syllable is gone.

Remove the load and the same typing is correct every time. In the VS Code integrated terminal
it happens with no artificial load at all — the renderer is busy enough on its own; cat was
running, so nothing but xterm.js sat between the keyboard and the pty.

Korean makes this easy to hit because a syllable commits the instant the next one starts, so a
fast run produces back-to-back compositionendcompositionstart pairs. Any IME with that
commit pattern should reproduce it.

The invariant that breaks

The deferred send is only safe if the 0 ms timer runs between one compositionend and the
next
. If it does, at most one send is ever pending and the single boolean is sufficient.
Nothing enforces that ordering, and it fails in at least two distinct ways.

The setTimeout(0) fired after … lines below are an independent 0 ms timer scheduled from
each compositionend, standing in for xterm.js's own flush.

Route 1 — the timer is starved (blocking 100 ms every 40 ms):

4963ms compositionend    data="니"      ← 니 finishes, flush T_니 scheduled
4963ms compositionstart  data=""        ← 다 begins
5065ms compositionupdate data="다"
5066ms compositionend    data="다"      ← 다 finishes — T_니 STILL PENDING
5068ms   └ setTimeout(0) fired after 104.2ms    ← T_니's turn, 100ms too late
5170ms   └ setTimeout(0) fired after 103.6ms

Route 2 — the timer is barely delayed at all, but the next composition is faster
(blocking 80 ms every 100 ms):

2500ms compositionend    data="니"      ← 니 finishes, flush T_니 scheduled
2501ms compositionstart  data=""        ← 다 begins
2501ms compositionupdate data="다"
2501ms compositionupdate data="다"
2501ms compositionend    data="다"      ← the whole 다 composition, inside 1ms
2503ms   └ setTimeout(0) fired after 2.3ms     ← T_니 was only 2ms late — still too late
2503ms   └ setTimeout(0) fired after 1.7ms

Route 2 is the interesting one, and it is why "make the timer faster" is not a fix. The
flush was delayed by 2.3 ms. It still lost, because started and finished in the
millisecond after ended — the input events had queued up behind the blocked main
thread and were delivered back to back once it freed up, several of them inside one task,
before the timer queue was serviced at all.

So load is the trigger, not the mechanism. Load causes input events to arrive in bursts; a
burst lets two compositions complete before one setTimeout(…, 0) gets a turn; and from
there the shared boolean does the damage. In both runs the . keypress that follows takes
the synchronous path, emits from _compositionPosition, and clears
_isSendingComposition — after which both pending timers find it false and return without
sending. is lost.

With the load off, the same page and the same typing produce the correct
["알","겠","습","니","다","."] every time. That is the entire difference between "works"
and "drops characters", and it is why this survives in a bare demo while showing up
immediately in a real application.

Root cause

// _finalizeComposition(true) — one boolean for however many sends are in flight
this._isSendingComposition = true;
setTimeout(() => {
  if (this._isSendingComposition) {   // ← "is *any* send live?", not "is *mine* live?"
    this._isSendingComposition = false;
    
  }
}, 0);
// _finalizeComposition(false) — reached from keydown for a non-composition key
this._isSendingComposition = false;   // ← intends to cancel one; cancels all
const input = this._textarea.value.substring(
  this._compositionPosition.start, this._compositionPosition.end);
this._coreService.triggerDataEvent(input, true);

The field's own comment says it plainly — "Whether a composition is in the process of being
sent, setting this to false will cancel any in-progress composition"
— and the singular "a
composition" is the assumption that breaks. Nothing serialises the sends or prevents two
from overlapping.

Reproduced on 6.0.0, 6.1.0-beta.288 and 6.1.0-beta.292 — the structure is unchanged
across all of them, and unchanged on master as of today.

Suggested fix

Replace the shared boolean with an ordered queue, and have the synchronous path drain it
rather than cancel it. A send watermark then makes double-sending impossible regardless of
which path emits first:

private _pendingSends: (() => void)[] = [];
private _sentUpTo: number = 0;

private _finalizeComposition(waitForPropagation: boolean): void {
  this._compositionView.classList.remove('active');
  this._isComposing = false;

  if (waitForPropagation) {
    const pos = { start: this._compositionPosition.start, end: this._compositionPosition.end };
    const suffix = this._compositionSuffix;
    const alreadySent = this._dataAlreadySent;

    const send = (): void => {
      this._isSendingComposition = false;
      pos.start += alreadySent.length;
      const value = this._textarea.value;
      // Not `_isComposing` — that is already false when this runs as part of a
      // drain. Ask what the check actually meant: did a newer composition start
      // past my start offset? If so, its start is my end boundary.
      const end = this._compositionPosition.start > pos.start
        ? this._compositionPosition.start
        : (suffix.length > 0 && value.endsWith(suffix)
            ? value.length - suffix.length
            : value.length);
      const from = Math.max(pos.start, this._sentUpTo);
      const to = Math.max(from, end);
      const input = value.substring(from, to);
      this._sentUpTo = Math.max(this._sentUpTo, to);
      if (input.length > 0) {
        this._coreService.triggerDataEvent(input, true);
      }
    };

    this._pendingSends.push(send);
    this._isSendingComposition = true;
    setTimeout(() => {
      const i = this._pendingSends.indexOf(send);
      if (i >= 0) {
        this._pendingSends.splice(i, 1);
        send();                      // still mine to run — nobody else cancelled it
      }
    }, 0);
  } else {
    // Flush what is already queued, in order, before emitting our own slice.
    for (const send of this._pendingSends.splice(0, this._pendingSends.length)) {
      send();
    }
    this._isSendingComposition = false;
    const value = this._textarea.value;
    const end = Math.max(this._compositionPosition.end,
      this._textarea.selectionEnd ?? this._compositionPosition.end);
    const from = Math.max(this._compositionPosition.start, this._sentUpTo);
    const to = Math.max(from, end);
    const input = value.substring(from, to);
    this._sentUpTo = Math.max(this._sentUpTo, to);
    if (input.length > 0) {
      this._coreService.triggerDataEvent(input, true);
    }
  }
}

_sentUpTo has to be lowered again when the textarea is rewritten underneath the helper
(_syncTextArea), otherwise a stale high watermark suppresses later sends. Doing it in
compositionstart is enough:

this._sentUpTo = Math.min(this._sentUpTo, this._compositionPosition.start);

Two details worth flagging for review:

  • The end boundary in the deferred path can no longer branch on this._isComposing. When
    the callback runs as part of a drain, _finalizeComposition has already set it to
    false, so the old check would take the wrong branch and over-read into the next
    composition's preedit. Comparing _compositionPosition.start against the captured
    pos.start expresses the original intent without depending on timing.
  • The watermark is what lets the drain be unconditional. Without it, draining the queued
    and then emitting the synchronous slice would send it twice.

Verification

I replayed the captured event sequence — the real offsets and textarea contents from a
failing run — against both implementations:

current   chunks=["알","겠","습"]              joined="알겠습"
patched   chunks=["알","겠","습","니","다"]    joined="알겠습니다"

The patched build has since been in daily use for Korean input with no drops and no
duplicates, including the 받침 + vowel cases (국어, 옷을, 밥이, 학교에서) that the
existing comment in _finalizeComposition warns about — the fix stays offset-based, so that
concern is untouched.

Happy to open a PR with the above plus a unit test that drives the recorded event sequence,
if the approach looks right.

Related, but not the same

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions