github/xrdp
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

chansrv: Fix stale engine handling and cross-thread ibus commit

Browse Source 
A few follow-on fixes found while testing the ibus reconnect changes
in a live session:

- xrdp_input_unicode_init() failed outright if ibus had no default
  global engine yet at connect time, which is a normal state on a
  fresh session (nothing has chosen one yet), not an error. Since
  nothing else used the return value, this permanently killed unicode
  input for the whole session over a spurious check.

- ibus can disable our engine instance (e.g. on a focus change) while
  leaving the global engine name as "XrdpIme", since those are tracked
  separately. xrdp_input_enable()'s fast path only checked the name,
  so it could skip re-asserting and leave xrdp_input_send_unicode()
  committing text through a disabled g_engine. Clear g_engine on
  disable and require it to be set for the fast path to apply.

- ibus_engine_commit_text() was being called from chansrv's own
  thread, not the thread pumping the glib main loop that owns the
  engine's D-Bus connection (xrdp_input_main_loop). Marshal the actual
  commit through g_main_context_invoke() onto the correct thread.

None of these are the full fix for intermittent dropped commits during
real pinyin input testing - that's still open, with the current lead
being ibus Reset calls interleaved with commit bursts, likely from the
FocusOut/FocusIn churn caused by passing every raw keystroke through
engine_process_key_event_cb. To be continued.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Liyi Meng
2026-08-17 20:38:20 +00:00
parent 38c8db8293
commit 92fdb09959
1 changed files with 66 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sesman/chansrv/input_ibus.c
+66 -11
View File
@@ -53,8 +53,14 @@ xrdp_input_enable(void)
* committing text to an engine that's no longer active. */
desc = ibus_bus_get_global_engine(bus);
name = desc ? ibus_engine_desc_get_name(desc) : NULL;
if (name && !g_ascii_strcasecmp(name, "XrdpIme"))
if (name && !g_ascii_strcasecmp(name, "XrdpIme") && g_engine)
{
/* XrdpIme is the global engine AND our cached engine instance is
* still the enabled one - genuinely nothing to do. If g_engine
* is NULL here, ibus disabled our instance (see
* xrdp_input_ibus_engine_disable) without changing the global
* engine name, so fall through and reassert below to get a
* fresh, enabled instance. */
g_object_unref(desc);
return 0;
}
@@ -83,6 +89,40 @@ xrdp_input_enable(void)
return 0;
}
static gboolean
xrdp_input_commit_text_cb(gpointer data)
{
gunichar chr = GPOINTER_TO_UINT(data);
/* Runs on the same thread/main context as xrdp_input_main_loop()
* (see the g_main_context_invoke() call below), so this re-check of
* g_engine is race-free against xrdp_input_ibus_engine_enable() /
* xrdp_input_ibus_engine_disable(), which are dispatched on that
* same thread.
*
* NOTE: engine->has_focus was tried here as a gate (skip/retry the
* commit until the engine reports focus) on the theory that ibus
* drops commits sent while unfocused. That made things strictly
* worse: has_focus read FALSE on every single attempt, even ones
* that would otherwise have succeeded, because IBusEngine does not
* appear to auto-maintain that field from the FocusIn/FocusOut
* D-Bus calls in this setup - nothing in this file ever writes to
* it, so it's not a trustworthy signal without our own focus-in/
* focus-out handlers actually tracking it. Do not reintroduce a
* has_focus check without first confirming it actually flips to
* TRUE (e.g. by logging it from a real focus-in signal handler). */
if (g_engine)
{
ibus_engine_commit_text(g_engine, ibus_text_new_from_unichar(chr));
}
else
{
LOG(LOG_LEVEL_ERROR, "xrdp_input_send_unicode: no active ibus engine to commit to");
}
return G_SOURCE_REMOVE;
}
int
xrdp_input_send_unicode(char32_t unicode)
{
@@ -93,8 +133,16 @@ xrdp_input_send_unicode(char32_t unicode)
return 1;
}
gunichar chr = unicode;
ibus_engine_commit_text(g_engine, ibus_text_new_from_unichar(chr));
/* ibus_engine_commit_text() must run on the same thread that's
* pumping the glib main loop the engine's D-Bus connection belongs
* to (the xrdp_input_main_loop thread) - that's also the thread
* that processes FocusIn/FocusOut for the engine. Calling it
* directly from here (chansrv's own thread) lets the commit race
* against that focus churn with no ordering guarantee, so ibus can
* silently drop it. g_main_context_invoke() marshals the actual
* commit onto the correct thread instead. */
g_main_context_invoke(NULL, xrdp_input_commit_text_cb,
GUINT_TO_POINTER((guint)unicode));
return 0;
}
@@ -110,6 +158,16 @@ static void
xrdp_input_ibus_engine_disable(IBusEngine *engine)
{
LOG(LOG_LEVEL_INFO, "xrdp_ibus_engine_disable: IM disabled");
/* ibus can disable our engine instance (e.g. it loses focus) while
* still reporting "XrdpIme" as the global engine name, since that's
* a separate piece of bookkeeping. Clear g_engine so xrdp_input_enable()
* notices the mismatch instead of committing text to a disabled
* engine on the next keypress. */
if (engine == g_engine)
{
g_engine = NULL;
}
}
static void
@@ -314,14 +372,11 @@ xrdp_input_unicode_init(void)
tc_thread_create(xrdp_input_main_loop, NULL);
if (!ibus_bus_get_global_engine(bus))
{
/* The bus connection itself is fine (and is now owned by the
* ibus main loop thread we just started), so leave it in place
* rather than tearing it down here. */
LOG(LOG_LEVEL_ERROR, "xrdp_ibus_init: failed to get origin global engine");
return 1;
}
/* No global engine may be set yet at this point (e.g. ibus hasn't
* picked a default on this fresh session, or the user simply never
* had one configured) - that's a normal state, not a failure. There
* is nothing to remember here; xrdp_input_enable() already handles
* a NULL/absent current engine correctly when it's actually needed. */
return 0;
}