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