Skip to content

Fix data races in preeditor access across D-Bus/worker threads#605

Open
Narcolepsyy wants to merge 1 commit into
BambooEngine:masterfrom
Narcolepsyy:fix/preedit-concurrency-races
Open

Fix data races in preeditor access across D-Bus/worker threads#605
Narcolepsyy wants to merge 1 commit into
BambooEngine:masterfrom
Narcolepsyy:fix/preedit-concurrency-races

Conversation

@Narcolepsyy

@Narcolepsyy Narcolepsyy commented Jun 17, 2026

Copy link
Copy Markdown

Problem

IBus dispatches each incoming D-Bus method on its own goroutine (go conn.handleCall in godbus). That means Reset, FocusIn, and FocusOut run concurrently with the keyPressChan worker goroutine (keyPressCapturing). Both paths mutate the shared bamboo preeditor, which is not safe for concurrent use — a data race that can corrupt the buffer and produce wrong output (e.g. oofôf instead of , stale words after focus changes).

Changes

  • Guard preeditor mutations with the engine mutex in FocusOut, Reset, bsProcessKeyEvent, and keyPressHandler.
  • resetBuffer: move the getRawKeyLen() empty-buffer guard inside the lock in both branches. It previously read the preeditor unlocked on the FocusIn → checkWmClass → resetBuffer path, racing the worker goroutine.
  • Reset: drain keyPressChan with a non-blocking select/default loop instead of len()>0 + <-. Other goroutines also receive from the channel, so the blocking form could hang the D-Bus thread on an empty channel (TOCTOU).
  • SendBackSpace: clamp n to [1, maxBackSpacePerCycle] and reset the fake-backspace counter after each send cycle, so a dropped synthetic backspace can't leave a permanent offset.
  • Discard the ephemeral preedit buffer on Reset/FocusOut in PreeditIM only; backspace modes preserve the buffer (apps call Reset aggressively mid-word).

Testing

CGO_ENABLED=1 go test . -mod=vendor -race -run "TestReset|TestFocus|TestFakeBackspace|TestGetOffset|TestConcurrent" -v
  • TestConcurrentResetAndKeyPress stresses Reset/FocusIn/FocusOut against the worker goroutine under -race.
  • Buffer-semantics tests per input mode (PreeditIM clears, backspace modes preserve), keyPressChan drain, getOffsetRunes, and fake-backspace reset.
  • Full suite passes clean under -race.

IBus dispatches each D-Bus method on its own goroutine (go conn.handleCall),
so Reset/FocusIn/FocusOut run concurrently with the keyPressChan worker
goroutine. Both mutate the shared bamboo preeditor, producing data races.

- Guard preeditor mutations in FocusOut, Reset, bsProcessKeyEvent and
  keyPressHandler with the engine mutex.
- resetBuffer: move the getRawKeyLen() empty-buffer guard inside the lock
  in both branches. It previously read the preeditor unlocked on the
  FocusIn -> checkWmClass -> resetBuffer path, racing the worker (caught
  by TestConcurrentResetAndKeyPress under -race).
- Reset: drain keyPressChan with a non-blocking select instead of
  len()>0 + <-. Other goroutines also receive from the channel, so the
  blocking form could hang the D-Bus thread on an empty channel.
- SendBackSpace: clamp n to [1, maxBackSpacePerCycle] and reset the
  fake-backspace counter after each cycle so a dropped synthetic
  backspace can't leave a permanent offset.
- Add tests covering Reset/FocusIn/FocusOut buffer semantics per input
  mode, getOffsetRunes, and a -race concurrency stress test.
@Narcolepsyy Narcolepsyy force-pushed the fix/preedit-concurrency-races branch from ec60076 to 2ed93ff Compare June 17, 2026 18:39
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