From bed99acf2720d55e2acc2a3ad4cb686af9d299c3 Mon Sep 17 00:00:00 2001 From: Liyi Meng Date: Sun, 16 Aug 2026 16:21:53 +0000 Subject: [PATCH] chansrv: Reconnect ibus when the cached connection has gone stale Fixes #3230. The static `bus` global was only ever checked for non-NULL, not for whether the underlying connection was still alive. If ibus disconnected (daemon restart, stale socket) or the initial connect attempt failed, `bus` was left set to a dead/freed connection, so every later call to xrdp_input_unicode_init() took the "already initialized" fast path and operated on it. - Null out bus/g_engine in the "disconnected" signal handler instead of leaving them dangling after g_object_unref(). - Check ibus_bus_is_connected() before trusting a cached bus, and tear down + reconnect if it's stale. - Unref and clear bus on a failed connect attempt instead of leaving it set. - Guard the unrefs in xrdp_input_unicode_destroy() now that bus/ g_engine can legitimately already be NULL. --- sesman/chansrv/input_ibus.c | 110 +++++++++++++++++++++++++++++------- 1 file changed, 89 insertions(+), 21 deletions(-) diff --git a/sesman/chansrv/input_ibus.c b/sesman/chansrv/input_ibus.c index b7ff2992..4be198cc 100644 --- a/sesman/chansrv/input_ibus.c +++ b/sesman/chansrv/input_ibus.c @@ -30,7 +30,7 @@ static IBusBus *bus; static IBusEngine *g_engine; /* This is the engine name enabled before unicode engine enabled */ -static const gchar *last_input_name; +static gchar *last_input_name; static int id = 0; static int @@ -39,27 +39,38 @@ xrdp_input_enable(void) IBusEngineDesc *desc; const gchar *name; - if (last_input_name) - { - /* already enabled */ - return 0; - } - if (!bus) { LOG(LOG_LEVEL_ERROR, "xrdp_ibus_init: input method switched failed, ibus not connected"); return 1; } + /* Re-check the current global engine on every call rather than + * trusting a one-time flag: ibus (particularly with + * use_global_engine disabled, which is common) can silently swap + * the active engine back to the user's own IME between calls, and + * we need to notice that and reassert XrdpIme rather than keep + * committing text to an engine that's no longer active. */ desc = ibus_bus_get_global_engine(bus); - name = ibus_engine_desc_get_name (desc); - if (!g_ascii_strcasecmp(name, "XrdpIme")) + name = desc ? ibus_engine_desc_get_name(desc) : NULL; + if (name && !g_ascii_strcasecmp(name, "XrdpIme")) { + g_object_unref(desc); return 0; } - /* remember user's input method, will switch back when disconnect */ - last_input_name = name; + if (!last_input_name && name) + { + /* remember user's original input method (first time only), will + * switch back when disconnected. Copy the name out since it's + * owned by desc, which we're about to unref. */ + last_input_name = g_strdup(name); + } + + if (desc) + { + g_object_unref(desc); + } if (!ibus_bus_set_global_engine(bus, "XrdpIme")) { @@ -105,8 +116,25 @@ static void xrdp_input_ibus_disconnect(IBusEngine *engine) { LOG(LOG_LEVEL_INFO, "xrdp_ibus_engine_disable: IM disabled"); - g_object_unref(g_engine); - g_object_unref(bus); + if (g_engine) + { + g_object_unref(g_engine); + g_engine = NULL; + } + if (bus) + { + g_object_unref(bus); + bus = NULL; + } + g_free(last_input_name); + last_input_name = NULL; + + /* ibus_main()/ibus_quit() operate on a single loop shared by the + * whole process, not one per IBusBus. Without this, the + * xrdp_input_main_loop thread for this (now dead) connection stays + * blocked in ibus_main() forever, and a fresh thread + loop gets + * started on the next reconnect - leaking a thread per reconnect. */ + ibus_quit(); } static gboolean @@ -125,12 +153,13 @@ xrdp_input_ibus_create_engine(IBusFactory *factory, gpointer user_data) { IBusEngine *engine; - gchar *path = g_strdup_printf("/org/freedesktop/IBus/Engine/%i", 1); + gchar *path = g_strdup_printf("/org/freedesktop/IBus/Engine/%i", ++id); engine = ibus_engine_new(engine_name, path, ibus_bus_get_connection(bus)); - LOG(LOG_LEVEL_DEBUG, "xrdp_input_ibus_create_engine: Creating IM Engine with name:%s and id:%d\n", engine_name, ++id); + LOG(LOG_LEVEL_DEBUG, "xrdp_input_ibus_create_engine: Creating IM Engine with name:%s and id:%d\n", engine_name, id); + g_free(path); g_signal_connect(engine, "process-key-event", G_CALLBACK(engine_process_key_event_cb), NULL); g_signal_connect(engine, "enable", G_CALLBACK(xrdp_input_ibus_engine_enable), NULL); @@ -192,15 +221,22 @@ int xrdp_input_unicode_destroy(void) { LOG(LOG_LEVEL_DEBUG, "xrdp_input_unicode_destory: ibus input is under destory"); - if (last_input_name) + if (last_input_name && bus) { LOG(LOG_LEVEL_INFO, "xrdp_input_unicode_destory: ibus engine rolling back to origin: %s", last_input_name); ibus_bus_set_global_engine(bus, last_input_name); } - g_object_unref(g_engine); - g_object_unref(bus); + if (g_engine) + { + g_object_unref(g_engine); + } + if (bus) + { + g_object_unref(bus); + } + g_free(last_input_name); last_input_name = NULL; bus = NULL; g_engine = NULL; @@ -213,9 +249,36 @@ xrdp_input_unicode_init(void) { if (bus) { - /* Already initialized, just re-enable it */ - xrdp_input_enable(); - return 0; + if (ibus_bus_is_connected(bus)) + { + /* Already initialized, just re-enable it */ + xrdp_input_enable(); + return 0; + } + + /* The bus is stale (e.g. the ibus daemon restarted and left a + * dead connection behind). Tear it down so we reconnect below + * instead of operating on a dead connection. */ + LOG(LOG_LEVEL_WARNING, + "xrdp_ibus_init: existing iBus connection is stale, reconnecting"); + if (g_engine) + { + g_object_unref(g_engine); + g_engine = NULL; + } + g_object_unref(bus); + bus = NULL; + g_free(last_input_name); + last_input_name = NULL; + + /* Belt-and-suspenders: normally the "disconnected" signal handler + * (xrdp_input_ibus_disconnect) already called this when the bus + * dropped. But a stale connection can be detected here without + * that signal ever having fired, and ibus_main()/ibus_quit() are + * process-global rather than per-bus, so make sure the old + * xrdp_input_main_loop thread isn't left blocked in ibus_main() + * before we start a new one below. */ + ibus_quit(); } /* Wait because the ibus daemon may not be ready on first login */ @@ -242,6 +305,8 @@ xrdp_input_unicode_init(void) if (!ibus_bus_is_connected(bus)) { LOG(LOG_LEVEL_ERROR, "xrdp_ibus_init: Connect to iBus failed"); + g_object_unref(bus); + bus = NULL; return 1; } @@ -251,6 +316,9 @@ xrdp_input_unicode_init(void) 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; }