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 -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[] = - " " - ""; - -+/* 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 - 1.5.19-16 +- Resolves: #1671286 wrong mutex + * Mon Feb 04 2019 Kalev Lember - 1.5.19-15 - Update BRs for vala packaging changes - Co-own vala and gir directories