From 023d50db40912e4f7ee333543e05995a9c730bae Mon Sep 17 00:00:00 2001 From: fujiwarat Date: Fri, 15 May 2020 21:44:16 +0900 Subject: [PATCH] Fix SEGV in bus_panel_proxy_focus_in() rhbz#1350291 SEGV in BUS_IS_CONNECTION(skip_connection) in bus_dbus_impl_dispatch_message_by_rule() check if dbus_connection is closed in bus_dbus_impl_connection_filter_cb(). rhbz#1767976 SEGV in assert(connection != NULL) in bus_dbus_impl_connection_filter_cb() call bus_connection_set_filter() in bus_dbus_impl_destroy(). rhbz#1601577 rhbz#1797726 SEGV in ibus_engine_desc_get_layout() in bus_engine_proxy_new_internal() WIP: Added a GError to get the error message to check why the SEGV happened. rhbz#1663528 SEGV in g_mutex_clear() in bus_dbus_impl_destroy() If the mutex is not unlocked, g_mutex_clear() causes assert. rhbz#1767691 SEGV in client/x11/main.c:_sighandler(). Do not call atexit functions in _sighandler(). rhbz#1795499 SEGV in ibus_bus_get_bus_address() because of no _bus->priv. _changed_cb() should not be called after ibus_bus_destroy() is called. rhbz#1771238 SEGV in assert(m_loop == null) in switcher.vala. Grabbing keyboard could be failed and switcher received the keyboard events and m_loop was not released. rhbz#1797120 SEGV in assert(bus.is_connected()) in panel_binding_construct() Check m_ibus in extension.vala:bus_name_acquired_cb() BUG=rhbz#1350291 BUG=rhbz#1601577 BUG=rhbz#1663528 BUG=rhbz#1767691 BUG=rhbz#1795499 BUG=rhbz#1771238 BUG=rhbz#1767976 BUG=rhbz#1797120 --- bus/dbusimpl.c | 47 ++++++++++++++++++++++++--- bus/engineproxy.c | 51 ++++++++++++++++++++++------- client/x11/main.c | 8 ++++- src/ibusbus.c | 5 +++ ui/gtk3/extension.vala | 4 +++ ui/gtk3/switcher.vala | 73 +++++++++++++++++++++++++----------------- 6 files changed, 141 insertions(+), 47 deletions(-) diff --git a/bus/dbusimpl.c b/bus/dbusimpl.c index 59787a80..af2fbde2 100644 --- a/bus/dbusimpl.c +++ b/bus/dbusimpl.c @@ -610,6 +610,7 @@ static void bus_dbus_impl_destroy (BusDBusImpl *dbus) { GList *p; + int i; for (p = dbus->objects; p != NULL; p = p->next) { IBusService *object = (IBusService *) p->data; @@ -633,6 +634,10 @@ bus_dbus_impl_destroy (BusDBusImpl *dbus) for (p = dbus->connections; p != NULL; p = p->next) { BusConnection *connection = BUS_CONNECTION (p->data); + /* rhbz#1767976 Fix connection == NULL in + * bus_dbus_impl_connection_filter_cb() + */ + bus_connection_set_filter (connection, NULL, NULL, NULL); g_signal_handlers_disconnect_by_func (connection, bus_dbus_impl_connection_destroy_cb, dbus); ibus_object_destroy (IBUS_OBJECT (connection)); @@ -647,12 +652,39 @@ bus_dbus_impl_destroy (BusDBusImpl *dbus) dbus->unique_names = NULL; dbus->names = NULL; + 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 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)); \ +} + + 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); @@ -1483,13 +1515,20 @@ bus_dbus_impl_connection_filter_cb (GDBusConnection *dbus_connection, gboolean incoming, gpointer user_data) { + BusDBusImpl *dbus; + BusConnection *connection; + g_assert (G_IS_DBUS_CONNECTION (dbus_connection)); g_assert (G_IS_DBUS_MESSAGE (message)); g_assert (BUS_IS_DBUS_IMPL (user_data)); - BusDBusImpl *dbus = (BusDBusImpl *) user_data; - BusConnection *connection = bus_connection_lookup (dbus_connection); + if (g_dbus_connection_is_closed (dbus_connection)) + return NULL; + + dbus = (BusDBusImpl *) user_data; + connection = bus_connection_lookup (dbus_connection); g_assert (connection != NULL); + g_assert (BUS_IS_CONNECTION (connection)); if (incoming) { /* is incoming message */ diff --git a/bus/engineproxy.c b/bus/engineproxy.c index 2d98995c..bbbe5532 100644 --- a/bus/engineproxy.c +++ b/bus/engineproxy.c @@ -660,20 +660,33 @@ bus_engine_proxy_g_signal (GDBusProxy *proxy, g_return_if_reached (); } +#pragma GCC optimize ("O0") static BusEngineProxy * bus_engine_proxy_new_internal (const gchar *path, IBusEngineDesc *desc, - GDBusConnection *connection) + GDBusConnection *connection, + GError **error) { + GDBusProxyFlags flags; + BusEngineProxy *engine; + g_assert (path); g_assert (IBUS_IS_ENGINE_DESC (desc)); g_assert (G_IS_DBUS_CONNECTION (connection)); + g_assert (error && *error == NULL); - GDBusProxyFlags flags = G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START; - BusEngineProxy *engine = + /* rhbz#1601577 engine == NULL if connection is closed. */ + if (g_dbus_connection_is_closed (connection)) { + *error = g_error_new (G_DBUS_ERROR, + G_DBUS_ERROR_FAILED, + "Connection is closed."); + return NULL; + } + flags = G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START; + engine = (BusEngineProxy *) g_initable_new (BUS_TYPE_ENGINE_PROXY, NULL, - NULL, + error, "desc", desc, "g-connection", connection, "g-interface-name", IBUS_INTERFACE_ENGINE, @@ -681,12 +694,19 @@ bus_engine_proxy_new_internal (const gchar *path, "g-default-timeout", g_gdbus_timeout, "g-flags", flags, NULL); + /* FIXME: rhbz#1601577 */ + if (!engine) { + /* show abrt local variable */ + gchar *message = g_strdup ((*error)->message); + g_error ("%s", message); + } const gchar *layout = ibus_engine_desc_get_layout (desc); if (layout != NULL && layout[0] != '\0') { engine->keymap = ibus_keymap_get (layout); } return engine; } +#pragma GCC reset_options typedef struct { GTask *task; @@ -748,23 +768,30 @@ create_engine_ready_cb (BusFactoryProxy *factory, GAsyncResult *res, EngineProxyNewData *data) { + GError *error = NULL; + gchar *path; + BusEngineProxy *engine; + g_return_if_fail (data->task != NULL); - GError *error = NULL; - gchar *path = bus_factory_proxy_create_engine_finish (factory, - res, - &error); + path = bus_factory_proxy_create_engine_finish (factory, res, &error); if (path == NULL) { g_task_return_error (data->task, error); engine_proxy_new_data_free (data); return; } - BusEngineProxy *engine = - bus_engine_proxy_new_internal (path, - data->desc, - g_dbus_proxy_get_connection ((GDBusProxy *)data->factory)); + engine = bus_engine_proxy_new_internal ( + path, + data->desc, + g_dbus_proxy_get_connection ((GDBusProxy *)data->factory), + &error); g_free (path); + if (!engine) { + g_task_return_error (data->task, error); + engine_proxy_new_data_free (data); + return; + } /* FIXME: set destroy callback ? */ g_task_return_pointer (data->task, engine, NULL); diff --git a/client/x11/main.c b/client/x11/main.c index c9ee174d..768b91f0 100644 --- a/client/x11/main.c +++ b/client/x11/main.c @@ -40,6 +40,7 @@ #include #include #include +#include #include @@ -1104,7 +1105,12 @@ _atexit_cb () static void _sighandler (int sig) { - exit(EXIT_FAILURE); + /* rhbz#1767691 _sighandler() is called with SIGTERM + * and exit() causes SEGV during calling atexit functions. + * _atexit_cb() might be broken. _exit() does not call + * atexit functions. + */ + _exit(EXIT_FAILURE); } static void diff --git a/src/ibusbus.c b/src/ibusbus.c index b7ffbb47..668c8a26 100644 --- a/src/ibusbus.c +++ b/src/ibusbus.c @@ -689,6 +689,11 @@ ibus_bus_destroy (IBusObject *object) _bus = NULL; if (bus->priv->monitor) { + /* rhbz#1795499 _changed_cb() causes SEGV because of no bus->priv + * after ibus_bus_destroy() is called. + */ + g_signal_handlers_disconnect_by_func (bus->priv->monitor, + (GCallback) _changed_cb, bus); g_object_unref (bus->priv->monitor); bus->priv->monitor = NULL; } diff --git a/ui/gtk3/extension.vala b/ui/gtk3/extension.vala index ea3cd464..57ed1357 100644 --- a/ui/gtk3/extension.vala +++ b/ui/gtk3/extension.vala @@ -73,6 +73,10 @@ class ExtensionGtk : Gtk.Application { string signal_name, Variant parameters) { debug("signal_name = %s", signal_name); + /* rhbz#1797120 Fix assert(bus.is_connected()) in + * panel_binding_construct() + */ + return_if_fail(m_bus.is_connected()); m_panel = new PanelBinding(m_bus, this); m_panel.load_settings(); } diff --git a/ui/gtk3/switcher.vala b/ui/gtk3/switcher.vala index a4529c88..29a70dd5 100644 --- a/ui/gtk3/switcher.vala +++ b/ui/gtk3/switcher.vala @@ -140,8 +140,8 @@ class Switcher : Gtk.Window { IBus.EngineDesc[] engines, int index, string input_context_path) { - assert (m_loop == null); - assert (index < engines.length); + assert(m_loop == null); + assert(index < engines.length); m_is_running = true; m_keyval = keyval; @@ -198,16 +198,18 @@ class Switcher : Gtk.Window { null, event, null); - if (status != Gdk.GrabStatus.SUCCESS) + if (status != Gdk.GrabStatus.SUCCESS) { warning("Grab keyboard failed! status = %d", status); - status = seat.grab(get_window(), - Gdk.SeatCapabilities.POINTER, - true, - null, - event, - null); - if (status != Gdk.GrabStatus.SUCCESS) - warning("Grab pointer failed! status = %d", status); + } else { + status = seat.grab(get_window(), + Gdk.SeatCapabilities.POINTER, + true, + null, + event, + null); + if (status != Gdk.GrabStatus.SUCCESS) + warning("Grab pointer failed! status = %d", status); + } #else Gdk.Device device = event.get_device(); if (device == null) { @@ -243,30 +245,41 @@ class Switcher : Gtk.Window { Gdk.EventMask.KEY_RELEASE_MASK, null, Gdk.CURRENT_TIME); - if (status != Gdk.GrabStatus.SUCCESS) + if (status != Gdk.GrabStatus.SUCCESS) { warning("Grab keyboard failed! status = %d", status); - // Grab all pointer events - status = pointer.grab(get_window(), - Gdk.GrabOwnership.NONE, - true, - Gdk.EventMask.BUTTON_PRESS_MASK | - Gdk.EventMask.BUTTON_RELEASE_MASK, - null, - Gdk.CURRENT_TIME); - if (status != Gdk.GrabStatus.SUCCESS) - warning("Grab pointer failed! status = %d", status); + } else { + // Grab all pointer events + status = pointer.grab(get_window(), + Gdk.GrabOwnership.NONE, + true, + Gdk.EventMask.BUTTON_PRESS_MASK | + Gdk.EventMask.BUTTON_RELEASE_MASK, + null, + Gdk.CURRENT_TIME); + if (status != Gdk.GrabStatus.SUCCESS) + warning("Grab pointer failed! status = %d", status); + } #endif - // Probably we can delete m_popup_delay_time in 1.6 - pointer.get_position_double(null, - out m_mouse_init_x, - out m_mouse_init_y); - m_mouse_moved = false; + /* Fix RHBZ #1771238 assert(m_loop == null) + * Grabbing keyboard can be failed when the second Super-e is typed + * before Switcher dialog is focused. And m_loop could not be released + * if the failed Super-e would call m_loop.run() below and could not + * call key_release_event(). And m_loop == null would be false in the + * third Super-e. + */ + if (status == Gdk.GrabStatus.SUCCESS) { + // Probably we can delete m_popup_delay_time in 1.6 + pointer.get_position_double(null, + out m_mouse_init_x, + out m_mouse_init_y); + m_mouse_moved = false; - m_loop = new GLib.MainLoop(); - m_loop.run(); - m_loop = null; + m_loop = new GLib.MainLoop(); + m_loop.run(); + m_loop = null; + } #if VALA_0_34 seat.ungrab(); -- 2.24.1