Blob Blame History Raw
From 8447b9f9369ce850f1888b4bd8d03540de88cab0 Mon Sep 17 00:00:00 2001
From: Andrea Azzarone <andrea.azzarone@canonical.com>
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 <time.h>
 
 #include <glib.h>
 #include <glib/gi18n.h>
 #include <gtk/gtk.h>
 
 #include <glib/gstdio.h>
 #include <gio/gio.h>
 
 #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