chansrv: Make XrdpIme survive reconnects and stop native-client input being eaten #1
Reference in New Issue
Block a user
Delete Branch "fix"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Three fixes in input_ibus.c, found by capturing the IBus session bus while a
native Mac RDP client and a Guacamole (browser) client typed Chinese.
Keep the IBus thread alive across client disconnects.
xrdp_input_unicode_destroy() used to stop the IBus thread and its private
GMainContext on every disconnect, and unicode_init() started a new pair on
every reconnect. ibus_bus_new() returns a process-wide singleton whose
GDBusConnection stays bound to the context that was thread-default when it
was first created, so a reconnect got the old connection back, bound to a
context nobody iterates. ibus-daemon's CreateEngine call on our factory was
never dispatched and every ibus_bus_set_global_engine() timed out ("failed
to switch global engine to XrdpIme", 15 s each), losing all Unicode input
until the session was fully logged out. (Same connection name on both
connects in the bus trace.) destroy() now only restores the user's engine
and drains the queue on the IBus thread (xrdp_input_reset_cb) and waits,
bounded, for that; init() takes its existing "already ready" path.
Do not re-request XrdpIme while an instance is being created.
Two characters arriving ~30 us apart each called set_global_engine(): the
second found the name already XrdpIme but no instance yet, fell through,
and made ibus destroy the instance under construction and build another
("IM enabled / IM disabled / IM enabled"), dropping what was queued for the
first. A request made within XRDP_INPUT_ENGINE_CREATE_WAIT_US (2 s) is now
treated as in flight and left alone; an older one is still treated as a
stale name and re-set.
Measure the commit debounce from when the text was queued.
"Quiet" was measured from the last raw key only. A user who pauses between
typing the pinyin and confirming it (270 ms in the capture) makes the raw
keys already quiet when the commit arrives, so it was committed at once and
the client's cleanup Backspaces, one per leaked key and arriving 1-2 ms
later, deleted the committed text and kept going into what was there
before. Quiet is now measured from the later of the last raw key and the
queue time, so every commit waits at least XRDP_INPUT_COMMIT_QUIET_US and
any Backspace inside the window extends it (still capped by MAX_WAIT).
Tested against a real xrdp session in the agent-cn image, through guacd with a
script that mimics the native client (raw keys, pause, Unicode commit,
Backspaces after a variable delay): with the old code any delay >= 8 ms lost
the commit ("我是" became "wo"); now delays up to 90 ms pass, 110 ms and up
still fail. A fresh session and a reconnect both commit correctly, and the
"failed to switch global engine" timeouts are gone. Confirmed working with the
native client by hand. Costs every Unicode commit 100 ms of latency.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Three fixes in input_ibus.c, found by capturing the IBus session bus while a native Mac RDP client and a Guacamole (browser) client typed Chinese. 1. Keep the IBus thread alive across client disconnects. xrdp_input_unicode_destroy() used to stop the IBus thread and its private GMainContext on every disconnect, and unicode_init() started a new pair on every reconnect. ibus_bus_new() returns a process-wide singleton whose GDBusConnection stays bound to the context that was thread-default when it was first created, so a reconnect got the old connection back, bound to a context nobody iterates. ibus-daemon's CreateEngine call on our factory was never dispatched and every ibus_bus_set_global_engine() timed out ("failed to switch global engine to XrdpIme", 15 s each), losing all Unicode input until the session was fully logged out. (Same connection name on both connects in the bus trace.) destroy() now only restores the user's engine and drains the queue on the IBus thread (xrdp_input_reset_cb) and waits, bounded, for that; init() takes its existing "already ready" path. 2. Do not re-request XrdpIme while an instance is being created. Two characters arriving ~30 us apart each called set_global_engine(): the second found the name already XrdpIme but no instance yet, fell through, and made ibus destroy the instance under construction and build another ("IM enabled / IM disabled / IM enabled"), dropping what was queued for the first. A request made within XRDP_INPUT_ENGINE_CREATE_WAIT_US (2 s) is now treated as in flight and left alone; an older one is still treated as a stale name and re-set. 3. Measure the commit debounce from when the text was queued. "Quiet" was measured from the last raw key only. A user who pauses between typing the pinyin and confirming it (270 ms in the capture) makes the raw keys already quiet when the commit arrives, so it was committed at once and the client's cleanup Backspaces, one per leaked key and arriving 1-2 ms later, deleted the committed text and kept going into what was there before. Quiet is now measured from the later of the last raw key and the queue time, so every commit waits at least XRDP_INPUT_COMMIT_QUIET_US and any Backspace inside the window extends it (still capped by MAX_WAIT). Tested against a real xrdp session in the agent-cn image, through guacd with a script that mimics the native client (raw keys, pause, Unicode commit, Backspaces after a variable delay): with the old code any delay >= 8 ms lost the commit ("我是" became "wo"); now delays up to 90 ms pass, 110 ms and up still fail. A fresh session and a reconnect both commit correctly, and the "failed to switch global engine" timeouts are gone. Confirmed working with the native client by hand. Costs every Unicode commit 100 ms of latency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>