client/x11: Fix Key typing order

ibus-x11 now also uses the hybrid process key events with
IBUS_ENABLE_SYNC_MODE=2 and it waits for the async API
with GSource and g_main_context_iteration() in xim_forward_event().

But g_main_context_iteration() calls gdk_event_source_dispatch()
and it can call another xim_forward_event() and the callbacks
of ibus_input_context_process_key_event_async() can be nested.
So if the forwarding API is called out of the callbacks of
ibus_input_context_process_key_event_async(), the key events
order is swapped due to the delayed return of
g_main_context_iteration().

To resolve this issue, the forwarding API should be called in
the callbacks of ibus_input_context_process_key_event_async().

Fixes: https://github.com/ibus/ibus/commit/506ac99

BUG=https://github.com/ibus/ibus/issues/2480

1 files changed, 83 insertions, 77 deletions

diff --git a/client/x11/main.c b/client/x11/main.c
index 905fd251..83d95cb7 100644
--- a/client/x11/main.c
+++ b/client/x11/main.c
@@ -2,7 +2,7 @@
 /* vim:set et sts=4: */
 /* ibus
  * Copyright (C) 2007-2015 Peng Huang <shawn.p.huang@gmail.com>
- * Copyright (C) 2015-2022 Takao Fujiwara <takao.fujiwara1@gmail.com>
+ * Copyright (C) 2015-2023 Takao Fujiwara <takao.fujiwara1@gmail.com>
  * Copyright (C) 2007-2015 Red Hat, Inc.
  *
  * main.c:
@@ -49,6 +49,8 @@
 #include <getopt.h>
 
 #define ESC_SEQUENCE_ISO10646_1 "\033%G"
+/* Wait for about 120 secs to return a key from async process-key-event. */
+#define MAX_WAIT_KEY_TIME       120000
 
 #define LOG(level, fmt_args...) \
     if (g_debug_level >= (level)) { \
@@ -461,11 +463,39 @@ xim_unset_ic_focus (XIMS xims, IMChangeFocusStruct *call_data)
 
 }
 
+static void
+_xim_forward_key_event_done (X11IC   *x11ic,
+                             XEvent  *event,
+                             gboolean processed)
+{
+    IMForwardEventStruct fe;
+    if (processed) {
+        if (!x11ic->has_preedit_area) {
+            _xim_set_cursor_location (x11ic);
+        }
+        return;
+    }
+    g_assert (x11ic);
+    g_assert (event);
+
+    memset (&fe, 0, sizeof (fe));
+    fe.major_code = XIM_FORWARD_EVENT;
+    fe.icid = x11ic->icid;
+    fe.connect_id = x11ic->connect_id;
+    fe.sync_bit = 0;
+    fe.serial_number = 0L;
+    fe.event = *event;
+    IMForwardEvent (_xims, (XPointer) &fe);
+}
+
+
 typedef struct {
-    IMForwardEventStruct *pfe;
     int                   count;
     guint                 count_cb_id;
     gboolean              retval;
+    X11IC                *x11ic;
+    CARD16                connect_id;
+    XEvent                event;
 } ProcessKeyEventReplyData;
 
 static void
@@ -474,7 +504,7 @@ _process_key_event_done (GObject      *object,
                          gpointer      user_data)
 {
     IBusInputContext *context = (IBusInputContext *)object;
-    IMForwardEventStruct *pfe = (IMForwardEventStruct*) user_data;
+    ProcessKeyEventReplyData *data = (ProcessKeyEventReplyData *)user_data;
 
     GError *error = NULL;
     gboolean retval = ibus_input_context_process_key_event_async_finish (
@@ -488,16 +518,15 @@ _process_key_event_done (GObject      *object,
     }
 
     if (g_hash_table_lookup (_connections,
-                             GINT_TO_POINTER ((gint) pfe->connect_id))
+                             GINT_TO_POINTER ((gint)data->connect_id))
         == NULL) {
-        g_slice_free (IMForwardEventStruct, pfe);
+        g_slice_free (ProcessKeyEventReplyData, data);
         return;
     }
 
-    if (retval == FALSE) {
-        IMForwardEvent (_xims, (XPointer) pfe);
-    }
-    g_slice_free (IMForwardEventStruct, pfe);
+    if (retval == FALSE)
+        _xim_forward_key_event_done (data->x11ic, &data->event, retval);
+    g_slice_free (ProcessKeyEventReplyData, data);
 }
 
 static void
@@ -518,6 +547,21 @@ _process_key_event_reply_done (GObject      *object,
     }
     g_return_if_fail (data);
     data->retval = retval;
+    if (g_hash_table_lookup (_connections,
+                             GINT_TO_POINTER ((gint)data->connect_id))
+        == NULL) {
+        return;
+    }
+    /* _xim_forward_key_event_done() should be called in
+     * _process_key_event_reply_done() because g_main_context_iteration()
+     * can call another xim_forward_event() and xim_forward_event() can be
+     * nested and the first _process_key_event_reply_done() is returned
+     * at last with g_main_context_iteration() so
+     * if _xim_forward_key_event_done() is called out of
+     * _process_key_event_reply_done(), the key events order
+     * can be swapped.
+     */
+    _xim_forward_key_event_done (data->x11ic, &data->event, retval);
     data->count = 0;
     g_source_remove (data->count_cb_id);
 }
@@ -529,9 +573,8 @@ _process_key_event_count_cb (gpointer user_data)
     g_return_val_if_fail (data, G_SOURCE_REMOVE);
     if (!data->count)
         return G_SOURCE_REMOVE;
-    /* Wait for about 10 secs. */
-    if (data->count++ == 10000) {
-        data->count = 0;
+    if (data->count++ == MAX_WAIT_KEY_TIME) {
+        g_warning ("Key event is not returned for %usecs.", MAX_WAIT_KEY_TIME);
         return G_SOURCE_REMOVE;
     }
     return G_SOURCE_CONTINUE;
@@ -571,32 +614,13 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
                                       event.keyval,
                                       event.hardware_keycode - 8,
                                       event.state);
-        if (retval) {
-            if (!x11ic->has_preedit_area) {
-                _xim_set_cursor_location (x11ic);
-            }
-            return 1;
-        }
-
-        IMForwardEventStruct fe;
-        memset (&fe, 0, sizeof (fe));
-
-        fe.major_code = XIM_FORWARD_EVENT;
-        fe.icid = x11ic->icid;
-        fe.connect_id = x11ic->connect_id;
-        fe.sync_bit = 0;
-        fe.serial_number = 0L;
-        fe.event = call_data->event;
-
-        IMForwardEvent (_xims, (XPointer) &fe);
-
+        _xim_forward_key_event_done (x11ic, &call_data->event, retval);
         retval = 1;
         break;
     }
     case 2: {
         GSource *source = g_timeout_source_new (1);
         ProcessKeyEventReplyData *data = NULL;
-        IMForwardEventStruct fe;
 
         if (source)
             data = g_slice_new0 (ProcessKeyEventReplyData);
@@ -610,11 +634,13 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
             if (source)
                 g_source_destroy (source);
         } else {
-            CARD16 connect_id = x11ic->connect_id;
             data->count = 1;
             g_source_attach (source, NULL);
             g_source_unref (source);
             data->count_cb_id = g_source_get_id (source);
+            data->connect_id = call_data->connect_id;
+            data->x11ic = x11ic;
+            data->event = *((XEvent*)xevent);
             ibus_input_context_process_key_event_async (
                     x11ic->context,
                     event.keyval,
@@ -626,7 +652,7 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
                     data);
             g_source_set_callback (source, _process_key_event_count_cb,
                                    data, NULL);
-            while (data->count)
+            while (data->count > 0 && data->count < MAX_WAIT_KEY_TIME)
                 g_main_context_iteration (NULL, TRUE);
             if (source->ref_count > 0) {
                 /* g_source_get_id() could causes a SEGV */
@@ -634,46 +660,33 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
                         "issue in %p.", source);
             }
             retval = data->retval;
-            g_slice_free (ProcessKeyEventReplyData, data);
-
-            if (g_hash_table_lookup (_connections,
-                                     GINT_TO_POINTER ((gint)connect_id))
-                == NULL) {
+            if (data->count == 0) {
+                g_slice_free (ProcessKeyEventReplyData, data);
                 return 1;
             }
         }
 
-        if (retval) {
-            if (! x11ic->has_preedit_area) {
-                _xim_set_cursor_location (x11ic);
-            }
-            return 1;
+        g_slice_free (ProcessKeyEventReplyData, data);
+        if (g_hash_table_lookup (_connections,
+                                 GINT_TO_POINTER ((gint)call_data->connect_id))
+                == NULL) {
+                return 1;
         }
-
-        memset (&fe, 0, sizeof (fe));
-
-        fe.major_code = XIM_FORWARD_EVENT;
-        fe.icid = x11ic->icid;
-        fe.connect_id = x11ic->connect_id;
-        fe.sync_bit = 0;
-        fe.serial_number = 0L;
-        fe.event = call_data->event;
-
-        IMForwardEvent (_xims, (XPointer) &fe);
-
+        _xim_forward_key_event_done (x11ic, &call_data->event, retval);
         retval = 1;
         break;
     }
     default: {
-        IMForwardEventStruct *pfe;
+        ProcessKeyEventReplyData *data;
 
-        pfe = g_slice_new0 (IMForwardEventStruct);
-        pfe->major_code = XIM_FORWARD_EVENT;
-        pfe->icid = x11ic->icid;
-        pfe->connect_id = x11ic->connect_id;
-        pfe->sync_bit = 0;
-        pfe->serial_number = 0L;
-        pfe->event = call_data->event;
+        if (!(data = g_slice_new0 (ProcessKeyEventReplyData))) {
+            g_warning ("Cannot allocate async data");
+            _xim_forward_key_event_done (x11ic, &call_data->event, 0);
+            return 1;
+        }
+        data->connect_id = call_data->connect_id;
+        data->x11ic = x11ic;
+        data->event = call_data->event;
 
         ibus_input_context_process_key_event_async (
                                       x11ic->context,
@@ -683,7 +696,7 @@ xim_forward_event (XIMS xims, IMForwardEventStruct *call_data)
                                       -1,
                                       NULL,
                                       _process_key_event_done,
-                                      pfe);
+                                      data);
         retval = 1;
     }
     }
@@ -962,11 +975,10 @@ _xim_forward_key_event (X11IC   *x11ic,
                         guint    keycode,
                         guint    state)
 {
-    g_return_if_fail (x11ic != NULL);
-
-    IMForwardEventStruct fe = {0};
     XEvent xkp = {0};
 
+    g_return_if_fail (x11ic != NULL);
+
     xkp.xkey.type = (state & IBUS_RELEASE_MASK) ? KeyRelease : KeyPress;
     xkp.xkey.serial = 0L;
     xkp.xkey.send_event = False;
@@ -975,20 +987,14 @@ _xim_forward_key_event (X11IC   *x11ic,
     xkp.xkey.window =
         x11ic->focus_window ? x11ic->focus_window : x11ic->client_window;
     xkp.xkey.subwindow = None;
-    xkp.xkey.root = DefaultRootWindow (GDK_DISPLAY_XDISPLAY (gdk_display_get_default ()));
+    xkp.xkey.root = DefaultRootWindow (
+            GDK_DISPLAY_XDISPLAY (gdk_display_get_default ()));
 
     xkp.xkey.time = 0;
     xkp.xkey.state = state;
     xkp.xkey.keycode = (keycode == 0) ? 0 : keycode + 8;
 
-    fe.major_code = XIM_FORWARD_EVENT;
-    fe.icid = x11ic->icid;
-    fe.connect_id = x11ic->connect_id;
-    fe.sync_bit = 0;
-    fe.serial_number = 0L;
-    fe.event = xkp;
-
-    IMForwardEvent (_xims, (XPointer) & fe);
+    _xim_forward_key_event_done (x11ic, &xkp, FALSE);
 }
 
 static void