chansrv: Stop reclaiming XrdpIme from a deliberately-chosen engine

xrdp_input_enable() unconditionally reasserted XrdpIme as the global
engine any time it wasn't already active, on the theory that ibus can
silently revert to the user's original engine on its own. That's true,
but the fix was too broad: it also fired whenever the user had
manually switched the session's engine to something else entirely
(e.g. libpinyin, to compose Chinese directly in the remote desktop
rather than via the client's local IME), yanking control away
mid-use. Confirmed happening live: a composition left pending in a
client-side IME and then abandoned got auto-flushed by the OS well
after the fact, silently reclaiming XrdpIme and interrupting an
unrelated libpinyin session already in progress.

Now only reclaims XrdpIme when the current engine is unset or matches
the original one remembered before ever switching away from it
(last_input_name) - the actual "ibus reverted on its own" case this
existed for. Any other named engine is treated as a deliberate choice
and left alone; the stale/deferred commit is dropped instead of
hijacking whatever the user is currently doing.

Also fixes a hang found while testing this: if xrdp_input_unicode_init()
times out waiting for ibus_get_address() (daemon unreachable), it
returned early without resetting ibus_thread_exited back to TRUE, even
though it had just set it FALSE in anticipation of a thread that was
never actually created. No thread means no broadcast ever comes, so
any later xrdp_input_unicode_destroy() or re-init() call blocks on
that condvar forever. Pre-existing in the private-loop rewrite, not
introduced by this change - just surfaced by testing this one harder.

Verified: 20 back-to-back init() calls on a healthy connection still
take the fast path with no false failures; manually switching to a
different engine and then sending a stale commit leaves that engine
untouched and fails the commit; reverting to the original engine still
correctly reasserts XrdpIme; 5 kill/restart cycles still show no
thread leak; an 18-char rapid-fire send against a real focused GTK
entry still lands every character in order.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Liyi Meng
2026-08-18 08:13:45 +00:00
parent a3934cde9a
commit 82b4eb6556
+33
View File
@@ -204,6 +204,31 @@ xrdp_input_enable(void)
return TRUE; return TRUE;
} }
/* Only reclaim XrdpIme if the current engine is either unset, or is
* the original one we remembered before ever switching away from
* it (last_input_name) - that's ibus silently reverting on its
* own, which is exactly the scenario this reassertion exists to
* fix. If it's some OTHER named engine, that's a deliberate user
* action (e.g. manually switching to a native engine like
* libpinyin to compose directly in the remote session), possibly
* well before this call - forcibly reclaiming XrdpIme would yank
* that away mid-use for the sake of a single, possibly stale or
* deferred commit (e.g. macOS auto-flushing a composition that was
* left pending after the user moved on to something else). Drop
* this commit instead and leave the user's active choice alone. */
if (last_input_name != NULL && name != NULL &&
g_ascii_strcasecmp(name, last_input_name) != 0)
{
LOG(LOG_LEVEL_WARNING,
"xrdp_input_enable: global engine is \"%s\", not XrdpIme or "
"the original \"%s\" - leaving it alone rather than "
"reclaiming it, dropping this commit",
name, last_input_name);
g_object_unref(desc);
g_object_unref(local_bus);
return FALSE;
}
/* Remember the user's engine only the first time we replace it. */ /* Remember the user's engine only the first time we replace it. */
if (last_input_name == NULL && name != NULL) if (last_input_name == NULL && name != NULL)
{ {
@@ -719,6 +744,14 @@ xrdp_input_unicode_init(void)
{ {
LOG(LOG_LEVEL_ERROR, LOG(LOG_LEVEL_ERROR,
"xrdp_input_unicode_init: timed out waiting for IBus daemon"); "xrdp_input_unicode_init: timed out waiting for IBus daemon");
/* No thread was created, so nothing will ever broadcast
* state_cond to wake a future destroy()/init() call waiting on
* ibus_thread_exited - reset it back to TRUE (set FALSE above
* in anticipation of the thread this function didn't end up
* creating), or that later wait blocks forever. */
g_mutex_lock(&state_mutex);
ibus_thread_exited = TRUE;
g_mutex_unlock(&state_mutex);
return 1; return 1;
} }