Replace the shared-global-context design with one where chansrv's
thread never touches libibus directly. All IBus objects (bus, engine,
factory, component) are now owned exclusively by a private
GMainContext/GMainLoop created inside the IBus thread; chansrv hands
off unicode codepoints through a thread-safe GAsyncQueue and wakes the
loop, instead of calling ibus_engine_commit_text() from the wrong
thread or relying on ibus_main()/ibus_quit(), which turned out to
operate on a single loop shared by the whole process rather than one
per connection.
Fixes along the way, found by testing against a live ibus-daemon and
a real focused GTK app rather than just reading the diff:
- xrdp_engine_enable()/disable() now take a real reference on g_engine
(g_object_ref/unref) instead of caching a bare pointer. The old code
unreffed g_engine on teardown without ever having reffed it - IBusEngine
derives from GInitiallyUnowned, so that was releasing a reference it
never owned.
- A custom GSource drains the unicode queue, gated on the engine
actually existing (engine creation is async once XrdpIme is
selected), so a character queued before the engine is ready isn't
lost - it commits as soon as the "enable" signal fires, instead of
racing a one-shot commit against that async setup.
- ibus_engine_commit_text() releases its IBusText argument itself
(it's floating, documented behavior for that specific call) - an
earlier draft of this rewrite added an extra g_object_unref() after
it, which would have been a double-release.
- bus (and anything else that opens a GDBusConnection) must be created
after the private GMainContext is pushed as thread-default, not
before - otherwise its async I/O silently binds to the global
default context, which nothing here iterates, and signals like
"disconnected" simply never fire.
- xrdp_input_unicode_init() now blocks on a condvar until the IBus
thread actually signals ready (or exits), instead of returning
immediately and hoping the engine shows up in time.
- unicode_init()/destroy() no longer touch bus/g_engine from chansrv's
own thread while the IBus thread may still be running against them;
destroy() schedules real teardown on the IBus thread and joins it via
a condvar before returning.
Known limitation, confirmed empirically rather than assumed: this does
NOT make reconnect-after-daemon-restart (#3230) actually work.
ibus_bus_new() returns a process-wide singleton in this libibus
version - two calls in the same process with no disconnect involved
return the identical pointer - so once its connection dies, every
later call just hands back the same dead object; ibus_bus_is_connected()
on it stays permanently false, confirmed not to be a transient state
via repeated retries with delay. The "disconnected" handler still
tears the thread down cleanly so a later xrdp_input_unicode_init()
fails fast instead of hanging or operating on stale state, but a real
fix would mean bypassing IBusBus for a raw GDBusConnection to
ibus-daemon, which isn't justified given how rarely the daemon
actually restarts mid-session.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
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.
The initial implementation of Uinicode input via IBus used a startup delay
of 3 seconds to wait for the daemon to be ready before connecting to it.
This commit introduces a poll-wait loop which can remove the delay
entirely if the daemon is up when chansrv starts the interface.
Liyi Meng