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

chansrv: Make XrdpIme survive reconnects and stop native-client input being eaten #1

Merged
liyi merged 1 commits from fix into devel 2026-09-19 00:14:23 +00:00
Conversation 0 Commits 1 Files Changed 1
+101 -30
1 changed files with 101 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
sesman/chansrv/input_ibus.c
+101 -30
View File
@@ -75,6 +75,14 @@ static GCond state_cond;
static gboolean ibus_ready = FALSE;
static gboolean ibus_thread_exited = TRUE;
static gboolean ibus_shutting_down = FALSE;
/* TRUE from xrdp_input_unicode_destroy() until the IBus thread has run xrdp_input_reset_cb() */
static gboolean ibus_reset_pending = FALSE;
/* When xrdp_input_enable() last asked ibus to switch to XrdpIme (0 = never).
* Guarded by state_mutex. While the engine instance it asked for is still
* being created, asking again would make ibus destroy that instance and
* build another, and the characters queued for the first one are lost. */
static gint64 last_engine_request_time = 0;
#define XRDP_INPUT_ENGINE_CREATE_WAIT_US (2 * G_TIME_SPAN_SECOND)
static int engine_id = 0;
/* Debounce state for committing queued unicode text - IBus-thread only,
@@ -285,8 +293,31 @@ xrdp_input_enable(void)
* construction, so that guard would otherwise reject every
* commit that lands in this window - which, since composing
* even one phrase sends several characters in quick
* succession, was most of them). Fall through and let the
* queue/prepare-check mechanism wait for g_engine as usual. */
* succession, was most of them).
*
* If we asked for this engine moments ago, it is simply still
* being created: return and let the queue/prepare-check
* mechanism wait for g_engine. Do NOT ask again - that makes
* ibus destroy the instance under construction and create
* another (seen as "IM enabled / IM disabled / IM enabled" in
* the log), losing what was queued for the first. Only when no
* request is recent is this a stale name (ibus disabled the
* instance but left the name), which needs the re-set below. */
gboolean in_flight;
g_mutex_lock(&state_mutex);
in_flight = (last_engine_request_time != 0 &&
g_get_monotonic_time() - last_engine_request_time <
XRDP_INPUT_ENGINE_CREATE_WAIT_US);
g_mutex_unlock(&state_mutex);
if (in_flight)
{
g_free(last_input_name_snapshot);
g_object_unref(desc);
g_object_unref(local_bus);
return TRUE;
}
}
else if (last_input_name_snapshot != NULL && name != NULL &&
g_ascii_strcasecmp(name, last_input_name_snapshot) != 0)
@@ -335,6 +366,10 @@ xrdp_input_enable(void)
g_object_unref(desc);
}
g_mutex_lock(&state_mutex);
last_engine_request_time = g_get_monotonic_time();
g_mutex_unlock(&state_mutex);
result = ibus_bus_set_global_engine(local_bus, "XrdpIme");
if (!result)
{
@@ -383,7 +418,19 @@ xrdp_input_process_unicode_queue(gpointer data)
* client's own compensating keystrokes, if any, have had time to land),
* or XRDP_INPUT_COMMIT_MAX_WAIT_US has elapsed since the queue first had
* something pending, whichever comes first. If not yet ready, *wait_us
* (when non-NULL) is set to how much longer until it will be. */
* (when non-NULL) is set to how much longer until it will be.
*
* "Quiet" is measured from the later of the last raw key and the moment
* the text was queued. Measuring from the last raw key alone fails when
* the user has paused between typing the pinyin and confirming it: the
* raw keys are then already "quiet" when the commit arrives, so it is
* committed at once, and the client's cleanup Backspaces (which follow
* the commit by a few ms to a few tens of ms) delete the text just
* committed and keep going into what was there before. Measured against
* a real Mac client: a 270 ms pause before confirming, commit followed
* 2 ms later by as many Backspaces as leaked keys. Every commit therefore
* waits at least the quiet window, and any Backspace arriving inside it
* extends the wait. */
static gboolean
xrdp_input_queue_ready(gint64 *wait_us)
{
@@ -402,7 +449,8 @@ xrdp_input_queue_ready(gint64 *wait_us)
queue_wait_since = now;
}
deadline = MIN(last_raw_key_time + XRDP_INPUT_COMMIT_QUIET_US,
deadline = MIN(MAX(last_raw_key_time, queue_wait_since) +
XRDP_INPUT_COMMIT_QUIET_US,
queue_wait_since + XRDP_INPUT_COMMIT_MAX_WAIT_US);
if (now >= deadline)
@@ -476,15 +524,27 @@ xrdp_input_install_queue_source(void)
}
/*****************************************************************************/
/* Runs on the IBus thread when the RDP client disconnects. Puts back the
* user's own engine and drops anything still queued, but deliberately does
* NOT stop the loop or the thread.
*
* ibus_bus_new() returns a process-wide singleton here (see the note above
* xrdp_input_ibus_bus_disconnected), and its GDBusConnection stays bound to
* the GMainContext that was thread-default when it was first created. If
* the thread and that context were torn down on every client disconnect,
* the next connect would build a new private context but get the old
* connection back: nothing would iterate the old context, so ibus-daemon's
* CreateEngine call on our factory would never be dispatched and
* ibus_bus_set_global_engine() would time out ("failed to switch global
* engine to XrdpIme") on every reconnect. Keeping one IBus thread for the
* life of chansrv avoids the whole class of problem. */
static gboolean
xrdp_input_shutdown_cb(gpointer data)
xrdp_input_reset_cb(gpointer data)
{
XrdpUnicodeEvent *event;
gchar *original_engine_name;
LOG(LOG_LEVEL_INFO, "xrdp_input: shutting down IBus");
ibus_shutting_down = TRUE;
LOG(LOG_LEVEL_INFO, "xrdp_input: client disconnected, resetting IBus state");
/* Take ownership of last_input_name under state_mutex rather than
* reading then freeing it directly - xrdp_input_enable() on
@@ -515,10 +575,11 @@ xrdp_input_shutdown_cb(gpointer data)
g_free(event);
}
if (ibus_loop != NULL)
{
g_main_loop_quit(ibus_loop);
}
g_mutex_lock(&state_mutex);
last_engine_request_time = 0;
ibus_reset_pending = FALSE;
g_cond_broadcast(&state_cond);
g_mutex_unlock(&state_mutex);
return G_SOURCE_REMOVE;
}
@@ -881,6 +942,9 @@ xrdp_input_unicode_init(void)
if (connected)
{
/* Normal reconnect: the IBus thread was kept alive by
* xrdp_input_unicode_destroy(), just let it commit again. */
ibus_shutting_down = FALSE;
g_mutex_unlock(&state_mutex);
return 0;
}
@@ -964,11 +1028,12 @@ xrdp_input_unicode_destroy(void)
GMainContext *context;
LOG(LOG_LEVEL_DEBUG,
"xrdp_input_unicode_destroy: destroying IBus input");
"xrdp_input_unicode_destroy: client gone, resetting IBus input");
g_mutex_lock(&state_mutex);
ibus_shutting_down = TRUE;
context = ibus_context;
context = (ibus_ready && !ibus_thread_exited) ? ibus_context : NULL;
ibus_reset_pending = (context != NULL);
g_mutex_unlock(&state_mutex);
if (context != NULL)
@@ -976,29 +1041,35 @@ xrdp_input_unicode_destroy(void)
g_main_context_invoke_full(
context,
G_PRIORITY_HIGH,
xrdp_input_shutdown_cb,
xrdp_input_reset_cb,
NULL,
NULL);
g_main_context_wakeup(context);
}
g_mutex_lock(&state_mutex);
while (!ibus_thread_exited)
{
g_cond_wait(&state_cond, &state_mutex);
}
g_mutex_unlock(&state_mutex);
if (unicode_queue != NULL)
{
XrdpUnicodeEvent *event;
while ((event = g_async_queue_try_pop(unicode_queue)) != NULL)
/* Wait for the reset so a reconnect arriving right behind this
* cannot have its engine restored out from under it. Bounded,
* and also ends if the IBus thread exits (which never runs the
* callback), so this cannot hang. */
g_mutex_lock(&state_mutex);
{
g_free(event);
gint64 end = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
while (ibus_reset_pending && !ibus_thread_exited)
{
if (!g_cond_wait_until(&state_cond, &state_mutex, end))
{
LOG(LOG_LEVEL_WARNING,
"xrdp_input_unicode_destroy: timed out waiting for "
"the IBus thread to reset");
break;
}
}
ibus_reset_pending = FALSE;
}
g_async_queue_unref(unicode_queue);
unicode_queue = NULL;
g_mutex_unlock(&state_mutex);
}
/* unicode_queue is left allocated: the IBus thread's queue source
* keeps using it across client connections. */
return 0;
}