[rpms/ibus] autotool: Resolves: #1671286 wrong mutex

[Takao Fujiwara <tfujiwar@redhat.com>]

A new commit has been pushed.

Repo   : rpms/ibus
Branch : autotool
Commit : 769eef0b906750959c3ba0bf98d9bd5e2fdc551e
Author : Takao Fujiwara <tfujiwar@redhat.com>
Date   : 2019-02-05T19:40:33+09:00
Stats  : +59/-294 in 2 file(s)
URL    : https://src.fedoraproject.org/rpms/ibus/c/769eef0b906750959c3ba0bf98d9bd5e2fdc551e?branch=autotool

Log:
Resolves: #1671286 wrong mutex

---
diff --git a/ibus-1385349-segv-bus-proxy.patch b/ibus-1385349-segv-bus-proxy.patch
index b2b54ea..bb904e0 100644
--- a/ibus-1385349-segv-bus-proxy.patch
+++ b/ibus-1385349-segv-bus-proxy.patch
@@ -1,6 +1,6 @@
-From 988059d40c9b6cffc3039a6d7623dc73672b0ad5 Mon Sep 17 00:00:00 2001
+From 018a0f889d18c41e314f0b1297d1dc559603142b Mon Sep 17 00:00:00 2001
 From: fujiwarat <takao.fujiwara1@gmail.com>
-Date: Tue, 29 Jan 2019 17:49:47 +0900
+Date: Tue, 5 Feb 2019 18:36:04 +0900
 Subject: [PATCH] Fix SEGV in bus_panel_proxy_focus_in()
 
 rhbz#1349148, rhbz#1385349
@@ -36,13 +36,13 @@ BUG=rhbz#1432252
 BUG=rhbz#1601577
 BUG=rhbz#1663528
 ---
- bus/dbusimpl.c    | 236 +++++++++++++++++++++++++++++++++++++---------
- bus/engineproxy.c |   9 +-
- bus/ibusimpl.c    |  21 ++++-
- 3 files changed, 215 insertions(+), 51 deletions(-)
+ bus/dbusimpl.c    | 70 +++++++++++++++++++++++++++++++++++++++++------
+ bus/engineproxy.c |  9 +++++-
+ bus/ibusimpl.c    | 21 ++++++++++++--
+ 3 files changed, 88 insertions(+), 12 deletions(-)
 
 diff --git a/bus/dbusimpl.c b/bus/dbusimpl.c
-index b54ef817..2eb5565d 100644
+index b54ef817..fb38faf0 100644
 --- a/bus/dbusimpl.c
 +++ b/bus/dbusimpl.c
 @@ -2,7 +2,8 @@
@@ -55,116 +55,7 @@ index b54ef817..2eb5565d 100644
   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public
-@@ -43,13 +44,23 @@ enum {
- 
- static guint dbus_signals[LAST_SIGNAL] = { 0 };
- 
-+/* rhbz#1663528 Check the value of is_locked before g_mutex_clear()
-+ * because if the mutex is not unlocked, g_mutex_clear() causes assert.
-+ */
-+typedef struct {
-+    GMutex lock;
-+    gboolean is_locked;
-+    GList *queue;
-+} BusDBusQueue;
-+
- struct _BusDBusImpl {
-     IBusService parent;
- 
-     /* instance members */
-     /* a map from a unique bus name (e.g. ":1.0") to a BusConnection. */
-     GHashTable *unique_names;
--    /* a map from a requested well-known name (e.g. "org.freedesktop.IBus.Panel") to a BusNameService. */
-+    /* a map from a requested well-known name (e.g. "org.freedesktop.IBus.Panel"
-+     *) to a BusNameService. */
-     GHashTable *names;
-     /* a list of IBusService objects. */
-     GList *objects;
-@@ -60,11 +71,8 @@ struct _BusDBusImpl {
-     /* a serial number used to generate a unique name of a bus. */
-     guint id;
- 
--    GMutex dispatch_lock;
--    GList *dispatch_queue;
--
--    GMutex forward_lock;
--    GList *forward_queue;
-+    BusDBusQueue *dispatch;
-+    BusDBusQueue *forward;
- 
-     /* a list of BusMethodCall to be used to reply when services are
-        really available */
-@@ -255,6 +263,69 @@ static const gchar introspection_xml[] =
-     "  </interface>"
-     "</node>";
- 
-+/* Use the functions instead of the macros because the macros would caused
-+ * optimized g_mutex_unlock() with GCC9. This also has a dummy return variable
-+ * to prevent the reverse order of safe_lock() and safe_unlock() with the
-+ * optimization in Fedora builds.
-+ */
-+static gboolean
-+bus_dbus_queue_mutex_safe_lock(BusDBusQueue *queue,
-+                               GDBusMessage *message,
-+                               int           line,
-+                               const gchar  *func) {
-+    int tout = 0;
-+    while (G_UNLIKELY ((queue)->is_locked)) {
-+        g_usleep (1);
-+        tout++;
-+        if (tout > 60)
-+            break;
-+    }
-+    if (G_UNLIKELY (tout)) {
-+        const gchar *path = g_dbus_message_get_path (message);
-+        const gchar *interface = g_dbus_message_get_interface (message);
-+        const gchar *member = g_dbus_message_get_member (message);
-+        const gchar *sender = g_dbus_message_get_sender (message);
-+        const gchar *dest = g_dbus_message_get_destination (message);
-+        g_warning ("%d:%s:(%s:%s:%s:%s:%s):%d: was locked.",
-+                   line, func,
-+                   path ? path : "(null)",
-+                   interface ? interface : "(null)",
-+                   member ? member : "(null)",
-+                   sender ? sender : "(null)",
-+                   dest ? dest : "(null)",
-+                   tout);
-+    }
-+    (queue)->is_locked = TRUE;
-+    g_mutex_lock (&((queue)->lock));
-+    return TRUE;
-+}
-+
-+static gboolean
-+bus_dbus_queue_mutex_safe_unlock(BusDBusQueue *queue,
-+                                 GDBusMessage *message,
-+                                 int           line,
-+                                 const gchar  *func) {
-+    if (G_LIKELY ((queue)->is_locked)) {
-+        g_mutex_unlock (&((queue)->lock));
-+    } else {
-+        const gchar *path = g_dbus_message_get_path (message);
-+        const gchar *interface = g_dbus_message_get_interface (message);
-+        const gchar *member = g_dbus_message_get_member (message);
-+        const gchar *sender = g_dbus_message_get_sender (message);
-+        const gchar *dest = g_dbus_message_get_destination (message);
-+        g_warning ("%d:%s:(%s:%s:%s:%s:%s):" \
-+                   " was unlocked by bus_dbus_impl_destroy",
-+                   line, func,
-+                   path ? path : "(null)",
-+                   interface ? interface : "(null)",
-+                   member ? member : "(null)",
-+                   sender ? sender : "(null)",
-+                   dest ? dest : "(null)");
-+    }
-+    (queue)->is_locked = FALSE;
-+    return FALSE;
-+}
-+
- static void
- bus_connection_owner_set_flags (BusConnectionOwner *owner,
-                                 guint32             flags)
-@@ -344,6 +415,8 @@ bus_name_service_set_primary_owner (BusNameService     *service,
+@@ -344,6 +345,8 @@ bus_name_service_set_primary_owner (BusNameService     *service,
                                      BusConnectionOwner *owner,
                                      BusDBusImpl        *dbus)
  {
@@ -173,7 +64,7 @@ index b54ef817..2eb5565d 100644
      g_assert (service != NULL);
      g_assert (owner != NULL);
      g_assert (dbus != NULL);
-@@ -351,6 +424,13 @@ bus_name_service_set_primary_owner (BusNameService     *service,
+@@ -351,6 +354,13 @@ bus_name_service_set_primary_owner (BusNameService     *service,
      BusConnectionOwner *old = service->owners != NULL ?
              (BusConnectionOwner *)service->owners->data : NULL;
  
@@ -187,7 +78,7 @@ index b54ef817..2eb5565d 100644
      if (old != NULL) {
          g_signal_emit (dbus,
                         dbus_signals[NAME_LOST],
-@@ -370,7 +450,8 @@ bus_name_service_set_primary_owner (BusNameService     *service,
+@@ -370,7 +380,8 @@ bus_name_service_set_primary_owner (BusNameService     *service,
                     0,
                     owner->conn,
                     service->name,
@@ -197,7 +88,7 @@ index b54ef817..2eb5565d 100644
                     bus_connection_get_unique_name (owner->conn));
  
      if (old != NULL && old->do_not_queue != 0) {
-@@ -427,6 +508,7 @@ bus_name_service_remove_owner (BusNameService     *service,
+@@ -427,6 +438,7 @@ bus_name_service_remove_owner (BusNameService     *service,
                                 BusDBusImpl        *dbus)
  {
      GSList *owners;
@@ -205,7 +96,7 @@ index b54ef817..2eb5565d 100644
  
      g_assert (service != NULL);
      g_assert (owner != NULL);
-@@ -439,6 +521,13 @@ bus_name_service_remove_owner (BusNameService     *service,
+@@ -439,6 +451,13 @@ bus_name_service_remove_owner (BusNameService     *service,
          BusConnectionOwner *_new = NULL;
          if (owners->next != NULL) {
              _new = (BusConnectionOwner *)owners->next->data;
@@ -219,7 +110,7 @@ index b54ef817..2eb5565d 100644
          }
  
          if (dbus != NULL) {
-@@ -447,7 +536,7 @@ bus_name_service_remove_owner (BusNameService     *service,
+@@ -447,7 +466,7 @@ bus_name_service_remove_owner (BusNameService     *service,
                             0,
                             owner->conn,
                             service->name);
@@ -228,7 +119,7 @@ index b54ef817..2eb5565d 100644
                  g_signal_emit (dbus,
                                 dbus_signals[NAME_ACQUIRED],
                                 0,
-@@ -460,7 +549,7 @@ bus_name_service_remove_owner (BusNameService     *service,
+@@ -460,7 +479,7 @@ bus_name_service_remove_owner (BusNameService     *service,
                      _new != NULL ? _new->conn : NULL,
                      service->name,
                      bus_connection_get_unique_name (owner->conn),
@@ -237,47 +128,57 @@ index b54ef817..2eb5565d 100644
  
          }
      }
-@@ -581,8 +670,10 @@ bus_dbus_impl_init (BusDBusImpl *dbus)
-                                          NULL,
-                                          (GDestroyNotify) bus_name_service_free);
+@@ -591,6 +610,7 @@ static void
+ bus_dbus_impl_destroy (BusDBusImpl *dbus)
+ {
+     GList *p;
++    int i;
  
--    g_mutex_init (&dbus->dispatch_lock);
--    g_mutex_init (&dbus->forward_lock);
-+    dbus->dispatch = g_slice_new0 (BusDBusQueue);
-+    g_mutex_init (&dbus->dispatch->lock);
-+    dbus->forward = g_slice_new0 (BusDBusQueue);
-+    g_mutex_init (&dbus->forward->lock);
+     for (p = dbus->objects; p != NULL; p = p->next) {
+         IBusService *object = (IBusService *) p->data;
+@@ -628,12 +648,39 @@ bus_dbus_impl_destroy (BusDBusImpl *dbus)
+     dbus->unique_names = NULL;
+     dbus->names = NULL;
  
-     /* other members are automatically zero-initialized. */
- }
-@@ -632,8 +723,24 @@ bus_dbus_impl_destroy (BusDBusImpl *dbus)
++    for (i = 0; g_idle_remove_by_data (dbus); i++) {
++        if (i > 1000) {
++            g_warning ("Too many idle threads were generated by " \
++                       "bus_dbus_impl_forward_message_idle_cb and " \
++                       "bus_dbus_impl_dispatch_message_by_rule_idle_cb");
++            break;
++        }
++    }
+     g_list_free_full (dbus->start_service_calls,
                        (GDestroyNotify) bus_method_call_free);
      dbus->start_service_calls = NULL;
  
 -    g_mutex_clear (&dbus->dispatch_lock);
 -    g_mutex_clear (&dbus->forward_lock);
-+    /* rhbz#1663528 Check the value of is_locked before g_mutex_clear()
-+     * because if the mutex is not unlocked, g_mutex_clear() causes assert.
-+     */
-+    if (G_UNLIKELY (dbus->dispatch->is_locked)) {
-+        g_mutex_unlock (&dbus->dispatch->lock);
-+        g_warning ("dbus->dispatch was not unlocked");
-+        dbus->dispatch->is_locked = FALSE;
-+    }
-+    g_mutex_clear (&dbus->dispatch->lock);
-+    g_slice_free (BusDBusQueue, dbus->dispatch);
++   /* rhbz#1663528 Call g_mutex_trylock() before g_mutex_clear()
++    * because if the mutex is not unlocked, g_mutex_clear() causes assert.
++    */
++#define BUS_DBUS_MUTEX_SAFE_CLEAR(mtex) {                               \
++    int count = 0;                                                      \
++    while (!g_mutex_trylock ((mtex))) {                                 \
++        g_usleep (1);                                                   \
++        if (count > 60) {                                               \
++            g_warning (#mtex " is dead lock");                          \
++            break;                                                      \
++        }                                                               \
++        ++count;                                                        \
++    }                                                                   \
++    g_mutex_unlock ((mtex));                                            \
++    g_mutex_clear ((mtex));                                             \
++}
 +
-+    if (G_UNLIKELY (dbus->forward->is_locked)) {
-+        g_mutex_unlock (&dbus->forward->lock);
-+        g_warning ("dbus->forward was not unlocked");
-+        dbus->forward->is_locked = FALSE;
-+    }
-+    g_mutex_clear (&dbus->forward->lock);
-+    g_slice_free (BusDBusQueue, dbus->forward);
++    BUS_DBUS_MUTEX_SAFE_CLEAR (&dbus->dispatch_lock);
++    BUS_DBUS_MUTEX_SAFE_CLEAR (&dbus->forward_lock);
++
++#undef BUS_DBUS_MUTEX_SAFE_CLEAR
  
      /* FIXME destruct _lock and _queue members. */
      IBUS_OBJECT_CLASS(bus_dbus_impl_parent_class)->destroy ((IBusObject *) dbus);
-@@ -1464,13 +1571,20 @@ bus_dbus_impl_connection_filter_cb (GDBusConnection *dbus_connection,
+@@ -1464,13 +1511,20 @@ bus_dbus_impl_connection_filter_cb (GDBusConnection *dbus_connection,
                                      gboolean         incoming,
                                      gpointer         user_data)
  {
@@ -300,145 +201,6 @@ index b54ef817..2eb5565d 100644
  
      if (incoming) {
          /* is incoming message */
-@@ -1721,18 +1835,24 @@ struct _BusForwardData {
- /**
-  * bus_dbus_impl_forward_message_ible_cb:
-  *
-- * Process the first element of the dbus->forward_queue. The first element is forwarded by g_dbus_connection_send_message.
-+ * Process the first element of the dbus->forward->queue. The first element is
-+ * forwarded by g_dbus_connection_send_message.
-  */
- static gboolean
- bus_dbus_impl_forward_message_idle_cb (BusDBusImpl   *dbus)
- {
--    g_return_val_if_fail (dbus->forward_queue != NULL, FALSE);
--
--    g_mutex_lock (&dbus->forward_lock);
--    BusForwardData *data = (BusForwardData *) dbus->forward_queue->data;
--    dbus->forward_queue = g_list_delete_link (dbus->forward_queue, dbus->forward_queue);
--    gboolean has_message = (dbus->forward_queue != NULL);
--    g_mutex_unlock (&dbus->forward_lock);
-+    g_return_val_if_fail (dbus->forward->queue != NULL, FALSE);
-+
-+    BusForwardData *data = (BusForwardData *) dbus->forward->queue->data;
-+    dbus->forward->is_locked = bus_dbus_queue_mutex_safe_lock (
-+            dbus->forward, data->message,
-+            __LINE__, G_STRFUNC);
-+    dbus->forward->queue = g_list_delete_link (dbus->forward->queue,
-+                                               dbus->forward->queue);
-+    gboolean has_message = (dbus->forward->queue != NULL);
-+    dbus->forward->is_locked = bus_dbus_queue_mutex_safe_unlock (
-+            dbus->forward, data->message,
-+            __LINE__, G_STRFUNC);
- 
-     do {
-         const gchar *destination = g_dbus_message_get_destination (data->message);
-@@ -1791,18 +1911,25 @@ bus_dbus_impl_forward_message (BusDBusImpl   *dbus,
- 
-     if (G_UNLIKELY (IBUS_OBJECT_DESTROYED (dbus)))
-         return;
--    /* FIXME the check above might not be sufficient. dbus object could be destroyed in the main thread right after the check, though the
--     * dbus structure itself would not be freed (since the dbus object is ref'ed in bus_dbus_impl_new_connection.)
--     * Anyway, it'd be better to investigate whether the thread safety issue could cause any real problems. */
-+    /* FIXME the check above might not be sufficient. dbus object could be
-+     * destroyed in the main thread right after the check, though the
-+     * dbus structure itself would not be freed (since the dbus object is
-+     * ref'ed in bus_dbus_impl_new_connection.)
-+     * Anyway, it'd be better to investigate whether the thread safety issue
-+     * could cause any real problems. */
- 
-     BusForwardData *data = g_slice_new (BusForwardData);
-     data->message = g_object_ref (message);
-     data->sender_connection = g_object_ref (connection);
- 
--    g_mutex_lock (&dbus->forward_lock);
--    gboolean is_running = (dbus->forward_queue != NULL);
--    dbus->forward_queue = g_list_append (dbus->forward_queue, data);
--    g_mutex_unlock (&dbus->forward_lock);
-+    dbus->forward->is_locked = bus_dbus_queue_mutex_safe_lock (
-+            dbus->forward, message,
-+            __LINE__, G_STRFUNC);
-+    gboolean is_running = (dbus->forward->queue != NULL);
-+    dbus->forward->queue = g_list_append (dbus->forward->queue, data);
-+    dbus->forward->is_locked = bus_dbus_queue_mutex_safe_unlock (
-+            dbus->forward, message,
-+            __LINE__, G_STRFUNC);
- 
-     if (!is_running) {
-         g_idle_add_full (G_PRIORITY_DEFAULT,
-@@ -1840,29 +1967,40 @@ bus_dispatch_data_free (BusDispatchData *data)
- /**
-  * bus_dbus_impl_dispatch_message_by_rule_idle_cb:
-  *
-- * Process the first element of the dbus->dispatch_queue.
-+ * Process the first element of the dbus->dispatch->queue.
-  */
- static gboolean
- bus_dbus_impl_dispatch_message_by_rule_idle_cb (BusDBusImpl *dbus)
- {
--    g_return_val_if_fail (dbus->dispatch_queue != NULL, FALSE);
-+    g_return_val_if_fail (dbus->dispatch->queue != NULL, FALSE);
- 
-     if (G_UNLIKELY (IBUS_OBJECT_DESTROYED (dbus))) {
-         /* dbus was destryed */
--        g_mutex_lock (&dbus->dispatch_lock);
--        g_list_free_full (dbus->dispatch_queue,
-+        BusDispatchData *data = (BusDispatchData *) dbus->dispatch->queue->data;
-+        dbus->dispatch->is_locked = bus_dbus_queue_mutex_safe_lock (
-+                dbus->dispatch, data->message,
-+                __LINE__, G_STRFUNC);
-+        g_list_free_full (dbus->dispatch->queue,
-                           (GDestroyNotify) bus_dispatch_data_free);
--        dbus->dispatch_queue = NULL;
--        g_mutex_unlock (&dbus->dispatch_lock);
--        return FALSE; /* return FALSE to prevent this callback to be called again. */
-+        dbus->dispatch->queue = NULL;
-+        dbus->dispatch->is_locked = bus_dbus_queue_mutex_safe_unlock (
-+                dbus->dispatch, data->message,
-+                __LINE__, G_STRFUNC);
-+        return FALSE;
-+        /* return FALSE to prevent this callback to be called again. */
-     }
- 
-     /* remove fist node */
--    g_mutex_lock (&dbus->dispatch_lock);
--    BusDispatchData *data = (BusDispatchData *) dbus->dispatch_queue->data;
--    dbus->dispatch_queue = g_list_delete_link (dbus->dispatch_queue, dbus->dispatch_queue);
--    gboolean has_message = (dbus->dispatch_queue != NULL);
--    g_mutex_unlock (&dbus->dispatch_lock);
-+    BusDispatchData *data = (BusDispatchData *) dbus->dispatch->queue->data;
-+    dbus->dispatch->is_locked = bus_dbus_queue_mutex_safe_lock (
-+            dbus->dispatch, data->message,
-+            __LINE__, G_STRFUNC);
-+    dbus->dispatch->queue = g_list_delete_link (dbus->dispatch->queue,
-+                                                dbus->dispatch->queue);
-+    gboolean has_message = (dbus->dispatch->queue != NULL);
-+    dbus->dispatch->is_locked = bus_dbus_queue_mutex_safe_unlock (
-+            dbus->dispatch, data->message,
-+            __LINE__, G_STRFUNC);
- 
-     GList *link = NULL;
-     GList *recipients = NULL;
-@@ -1916,11 +2054,15 @@ bus_dbus_impl_dispatch_message_by_rule (BusDBusImpl     *dbus,
-     g_object_set_qdata ((GObject *) message, dispatched_quark, GINT_TO_POINTER (1));
- 
-     /* append dispatch data into the queue, and start idle task if necessary */
--    g_mutex_lock (&dbus->dispatch_lock);
--    gboolean is_running = (dbus->dispatch_queue != NULL);
--    dbus->dispatch_queue = g_list_append (dbus->dispatch_queue,
-+    dbus->dispatch->is_locked = bus_dbus_queue_mutex_safe_lock (
-+            dbus->dispatch, message,
-+            __LINE__, G_STRFUNC);
-+    gboolean is_running = (dbus->dispatch->queue != NULL);
-+    dbus->dispatch->queue = g_list_append (dbus->dispatch->queue,
-                     bus_dispatch_data_new (message, skip_connection));
--    g_mutex_unlock (&dbus->dispatch_lock);
-+    dbus->dispatch->is_locked = bus_dbus_queue_mutex_safe_unlock (
-+            dbus->dispatch, message,
-+            __LINE__, G_STRFUNC);
-     if (!is_running) {
-         g_idle_add_full (G_PRIORITY_DEFAULT,
-                          (GSourceFunc) bus_dbus_impl_dispatch_message_by_rule_idle_cb,
 diff --git a/bus/engineproxy.c b/bus/engineproxy.c
 index 2d98995c..2176e0c9 100644
 --- a/bus/engineproxy.c
@@ -528,5 +290,5 @@ index bbbb5770..77fcf42f 100644
      bus_ibus_impl_component_name_owner_changed (ibus, name, old_name, new_name);
  }
 -- 
-2.19.1
+2.20.1
 

diff --git a/ibus.spec b/ibus.spec
index b6aec93..6592993 100644
--- a/ibus.spec
+++ b/ibus.spec
@@ -35,7 +35,7 @@
 
 Name:           ibus
 Version:        1.5.19
-Release:        15%{?dist}
+Release:        16%{?dist}
 Summary:        Intelligent Input Bus for Linux OS
 License:        LGPLv2+
 URL:            https://github.com/ibus/%name/wiki
@@ -424,6 +424,9 @@ dconf update || :
 %{_datadir}/gtk-doc/html/*
 
 %changelog
+* Tue Feb 05 2019 Takao Fujiwara <tfujiwar@redhat.com> - 1.5.19-16
+- Resolves: #1671286 wrong mutex
+
 * Mon Feb 04 2019 Kalev Lember <klember@redhat.com> - 1.5.19-15
 - Update BRs for vala packaging changes
 - Co-own vala and gir directories