From 8447b9f9369ce850f1888b4bd8d03540de88cab0 Mon Sep 17 00:00:00 2001 From: Andrea Azzarone Date: Tue, 2 Apr 2019 17:33:12 +0100 Subject: [PATCH] open-document-selector: Properly remove idle It's not possible to use g_idle_remove_by_data when the idle was added with gdk_threads_add_idle_full. gdk_threads_add_idle_full uses g_idle_add_full internally but it creates a temporary data strucuture. The address of this temporary data structure should be passed to g_idle_remove_by_data. For obvious reasons we cannot do that, so let's use g_source_remove. Failing to remove the idle when the open document selector is disposed could result in a crash because the idle function (real_populate_liststore) would access invalid memory. Also remove populate_scheduled because it's not needed for two reasons: 1. populate_liststore and real_populate_liststore always run in the same thread 2. if populate_liststore is called before real_populate_liststore is run, there is no need to schedule two calls two real_populate_liststore. Close: https://bugs.launchpad.net/bugs/1646762 --- gedit/gedit-open-document-selector.c | 36 ++++++++++------------------ 1 file changed, 12 insertions(+), 24 deletions(-) diff --git a/gedit/gedit-open-document-selector.c b/gedit/gedit-open-document-selector.c index 5389ef0bd..b3c1ed503 100644 --- a/gedit/gedit-open-document-selector.c +++ b/gedit/gedit-open-document-selector.c @@ -24,77 +24,76 @@ #include #include #include #include #include #include #include "gedit-recent.h" #include "gedit-utils.h" #include "gedit-window.h" #include "gedit-debug.h" struct _GeditOpenDocumentSelector { GtkBox parent_instance; GeditWindow *window; GtkWidget *search_entry; GtkWidget *open_button; GtkWidget *treeview; GtkListStore *liststore; GtkCellRenderer *name_renderer; GtkCellRenderer *path_renderer; GtkWidget *placeholder_box; GtkWidget *scrolled_window; + guint populate_listbox_id; + GdkRGBA name_label_color; PangoFontDescription *name_font; GdkRGBA path_label_color; PangoFontDescription *path_font; GeditOpenDocumentSelectorStore *selector_store; GList *recent_items; GList *home_dir_items; GList *desktop_dir_items; GList *local_bookmarks_dir_items; GList *file_browser_root_items; GList *active_doc_dir_items; GList *current_docs_items; GList *all_items; - - guint populate_liststore_is_idle : 1; - guint populate_scheduled : 1; }; typedef enum { SELECTOR_TAG_NONE, SELECTOR_TAG_MATCH } SelectorTag; enum { NAME_COLUMN, PATH_COLUMN, URI_COLUMN, N_COLUMNS }; enum { PROP_0, PROP_WINDOW, LAST_PROP }; static GParamSpec *properties[LAST_PROP]; enum { SELECTOR_FILE_ACTIVATED, LAST_SIGNAL }; @@ -483,61 +482,60 @@ fileitem_list_remove_duplicates (GList *items) GList *l1; if ((l1 = l->next) == NULL) { break; } l_uri = ((FileItem *)l->data)->uri; l1_uri = ((FileItem *)l1->data)->uri; if (g_strcmp0 (l_uri, l1_uri) == 0) { gedit_open_document_selector_free_fileitem_item ((FileItem *)l1->data); dummy_ptr = g_list_delete_link (items, l1); } else { l = l->next; } } } static gboolean real_populate_liststore (gpointer data) { GeditOpenDocumentSelector *selector = GEDIT_OPEN_DOCUMENT_SELECTOR (data); GeditOpenDocumentSelectorStore *selector_store; GList *l; GList *filter_items = NULL; gchar *filter; GRegex *filter_regex = NULL; - selector->populate_liststore_is_idle = FALSE; DEBUG_SELECTOR_TIMER_DECL DEBUG_SELECTOR_TIMER_NEW gtk_list_store_clear (selector->liststore); selector_store = selector->selector_store; filter = gedit_open_document_selector_store_get_filter (selector_store); if (filter && *filter != '\0') { DEBUG_SELECTOR (g_print ("Selector(%p): populate liststore: all lists\n", selector);); filter_items = fileitem_list_filter (selector->all_items, (const gchar *)filter); filter_items = g_list_sort_with_data (filter_items, (GCompareDataFunc)sort_items_by_mru, NULL); fileitem_list_remove_duplicates (filter_items); filter_regex = g_regex_new (filter, G_REGEX_CASELESS, 0, NULL); } else { gint recent_limit; GList *recent_items; DEBUG_SELECTOR (g_print ("Selector(%p): populate liststore: recent files list\n", selector);); recent_limit = gedit_open_document_selector_store_get_recent_limit (selector_store); if (recent_limit > 0 ) { recent_items = fileitem_list_filter (selector->recent_items, NULL); @@ -551,87 +549,79 @@ real_populate_liststore (gpointer data) } g_free (filter); DEBUG_SELECTOR (g_print ("Selector(%p): populate liststore: length:%i\n", selector, g_list_length (filter_items));); /* Show the placeholder if no results, show the treeview otherwise */ gtk_widget_set_visible (selector->scrolled_window, (filter_items != NULL)); gtk_widget_set_visible (selector->placeholder_box, (filter_items == NULL)); for (l = filter_items; l != NULL; l = l->next) { FileItem *item; item = l->data; create_row (selector, (const FileItem *)item, filter_regex); } if (filter_regex) { g_regex_unref (filter_regex); } gedit_open_document_selector_free_file_items_list (filter_items); DEBUG_SELECTOR (g_print ("Selector(%p): populate liststore: time:%lf\n\n", selector, DEBUG_SELECTOR_TIMER_GET);); DEBUG_SELECTOR_TIMER_DESTROY - if (selector->populate_scheduled) - { - selector->populate_scheduled = FALSE; - return G_SOURCE_CONTINUE; - } - else - { - return G_SOURCE_REMOVE; - } + selector->populate_listbox_id = 0; + return G_SOURCE_REMOVE; } static void populate_liststore (GeditOpenDocumentSelector *selector) { /* Populate requests are compressed */ - if (selector->populate_liststore_is_idle) + if (selector->populate_listbox_id != 0) { DEBUG_SELECTOR (g_print ("Selector(%p): populate liststore: idle\n", selector);); - - selector->populate_scheduled = TRUE; return; } DEBUG_SELECTOR (g_print ("Selector(%p): populate liststore: scheduled\n", selector);); - - selector->populate_liststore_is_idle = TRUE; - gdk_threads_add_idle_full (G_PRIORITY_HIGH_IDLE + 30, real_populate_liststore, selector, NULL); + selector->populate_listbox_id = gdk_threads_add_idle_full (G_PRIORITY_HIGH_IDLE + 30, + real_populate_liststore, + selector, + NULL); } static gboolean on_treeview_key_press (GtkTreeView *treeview, GdkEventKey *event, GeditOpenDocumentSelector *selector) { guint keyval; gboolean is_control_pressed; GtkTreeSelection *tree_selection; GtkTreePath *root_path; GdkModifierType modifiers; if (gdk_event_get_keyval ((GdkEvent *)event, &keyval) == TRUE) { tree_selection = gtk_tree_view_get_selection (treeview); root_path = gtk_tree_path_new_from_string ("0"); modifiers = gtk_accelerator_get_default_mod_mask (); is_control_pressed = (event->state & modifiers) == GDK_CONTROL_MASK; if ((keyval == GDK_KEY_Up || keyval == GDK_KEY_KP_Up) && !is_control_pressed) { if (gtk_tree_selection_path_is_selected (tree_selection, root_path)) { gtk_tree_selection_unselect_all (tree_selection); gtk_widget_grab_focus (selector->search_entry); return GDK_EVENT_STOP; @@ -683,66 +673,64 @@ on_entry_activated (GtkEntry *entry, uri = g_strconcat ("file://", entry_text, NULL); } } else { g_free (scheme); uri = g_strdup (entry_text); } file = g_file_new_for_uri (uri); if (g_file_query_exists (file, NULL)) { DEBUG_SELECTOR (g_print ("Selector(%p): search entry activated : loading '%s'\n", selector, uri);); gtk_entry_set_text (entry, ""); selection = gtk_tree_view_get_selection (GTK_TREE_VIEW (selector->treeview)); gtk_tree_selection_unselect_all (selection); g_signal_emit (G_OBJECT (selector), signals[SELECTOR_FILE_ACTIVATED], 0, uri); } g_object_unref (file); } static void gedit_open_document_selector_dispose (GObject *object) { GeditOpenDocumentSelector *selector = GEDIT_OPEN_DOCUMENT_SELECTOR (object); - while (TRUE) + if (selector->populate_listbox_id != 0) { - if (!g_idle_remove_by_data (selector)) - { - break; - } + g_source_remove (selector->populate_listbox_id); + selector->populate_listbox_id = 0; } g_clear_pointer (&selector->name_font, pango_font_description_free); g_clear_pointer (&selector->path_font, pango_font_description_free); if (selector->recent_items) { gedit_open_document_selector_free_file_items_list (selector->recent_items); selector->recent_items = NULL; } if (selector->home_dir_items) { gedit_open_document_selector_free_file_items_list (selector->home_dir_items); selector->home_dir_items = NULL; } if (selector->desktop_dir_items) { gedit_open_document_selector_free_file_items_list (selector->desktop_dir_items); selector->desktop_dir_items = NULL; } if (selector->local_bookmarks_dir_items) { gedit_open_document_selector_free_file_items_list (selector->local_bookmarks_dir_items); selector->local_bookmarks_dir_items = NULL; } if (selector->file_browser_root_items) -- 2.25.1