GNOME.org
 

Re: gio async usage from gtk+ ...

On Thu, 2011-03-24 at 12:53 +0000, Michael Meeks wrote:
> Hi guys,

> 	And I wonder where the GDK_THREADS_ENTER / GDK_THREADS_LEAVE that we
> need when entering from an idle handler is.
> 
> 	Presumably this is necessary for every '_async' callback that occurs
> [ and lets hope they are 100% called from the idle handler, and not
> sometimes for error conditions in-line ;-]. 
> 
> 	And I was wondering: que ? there are ~24 of these '_async (' matches in
> gtk+, and of the few I spot-checked, none take the lock; feature or
> bug ?

I had a look, and its not true that none take the lock, they just call
gdk_threads_enter() directly, not the GDK_THREADS_ENTER macro. However,
there were quite a few places that had it missing.

I'm attaching patches for Gtk+ 2 and 3 (bugzilla is down atm). These are
kinda tricky as its hard to really test them. Would be nice with some
more review of them.

-- 
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Alexander Larsson                                            Red Hat, Inc 
       alexl redhat com            alexander larsson gmail com 
He's a jaded gay messiah on the wrong side of the law. She's a cosmopolitan 
nymphomaniac lawyer married to the Mob. They fight crime! 

commit c267d2d3c194febb4f14444aed712cedee70d6b9
Author: Alexander Larsson <alexl redhat com>
Date:   Fri Mar 25 10:53:05 2011 +0100

    Ensure we always grab the gdk lock in async callbacks
    
    Async callbacks are delivered in idles, so we need to make sure
    we get the gdk lock before calling any gdk/gtk stuff. This was
    missing in a few places.

diff --git a/gtk/gtkappchooserdialog.c b/gtk/gtkappchooserdialog.c
index 35d1d1f..3f1b798 100644
--- a/gtk/gtkappchooserdialog.c
+++ b/gtk/gtkappchooserdialog.c
@@ -121,6 +121,8 @@ search_for_mimetype_ready_cb (GObject      *source,
   GtkAppChooserDialog *self = user_data;
   GError *error = NULL;
 
+  gdk_threads_enter ();
+
   _gtk_app_chooser_online_search_for_mimetype_finish (online, res, &error);
 
   if (error != NULL)
@@ -133,6 +135,8 @@ search_for_mimetype_ready_cb (GObject      *source,
     {
       gtk_app_chooser_refresh (GTK_APP_CHOOSER (self->priv->app_chooser_widget));
     }
+
+  gdk_threads_leave ();
 }
 
 static void
@@ -155,6 +159,8 @@ app_chooser_online_get_default_ready_cb (GObject *source,
 {
   GtkAppChooserDialog *self = user_data;
 
+  gdk_threads_enter ();
+
   self->priv->online = _gtk_app_chooser_online_get_default_finish (source, res);
 
   if (self->priv->online != NULL)
@@ -176,6 +182,8 @@ app_chooser_online_get_default_ready_cb (GObject *source,
 
       gtk_widget_show (self->priv->online_button);
     }
+
+  gdk_threads_leave ();
 }
 
 static void
diff --git a/gtk/gtkfilechooserdefault.c b/gtk/gtkfilechooserdefault.c
index aa2cf78..9c6c717 100644
--- a/gtk/gtkfilechooserdefault.c
+++ b/gtk/gtkfilechooserdefault.c
@@ -6542,11 +6542,16 @@ file_system_model_got_thumbnail (GObject *object, GAsyncResult *res, gpointer da
   if (queried == NULL)
     return;
 
+  GDK_THREADS_ENTER ();
+
   /* now we know model is valid */
 
   /* file was deleted */
   if (!_gtk_file_system_model_get_iter_for_file (model, &iter, file))
-    return;
+    {
+      GDK_THREADS_LEAVE ();
+      return;
+    }
 
   info = g_file_info_dup (_gtk_file_system_model_get_info (model, &iter));
 
@@ -6557,6 +6562,8 @@ file_system_model_got_thumbnail (GObject *object, GAsyncResult *res, gpointer da
   _gtk_file_system_model_update_file (model, file, info, FALSE);
 
   g_object_unref (info);
+
+  GDK_THREADS_LEAVE ();
 }
 
 static gboolean
diff --git a/gtk/gtkfilesystem.c b/gtk/gtkfilesystem.c
index 1b81680..30ff484 100644
--- a/gtk/gtkfilesystem.c
+++ b/gtk/gtkfilesystem.c
@@ -1324,6 +1324,8 @@ query_created_file_info_callback (GObject      *source_object,
       return;
     }
 
+  gdk_threads_enter ();
+
   folder = GTK_FOLDER (user_data);
   gtk_folder_add_file (folder, file, info);
 
@@ -1332,6 +1334,7 @@ query_created_file_info_callback (GObject      *source_object,
   g_slist_free (files);
 
   g_object_unref (info);
+  gdk_threads_leave ();
 }
 
 static void
diff --git a/gtk/gtkfilesystemmodel.c b/gtk/gtkfilesystemmodel.c
index c6532c1..92f44dd 100644
--- a/gtk/gtkfilesystemmodel.c
+++ b/gtk/gtkfilesystemmodel.c
@@ -1149,7 +1149,9 @@ gtk_file_system_model_query_done (GObject *     object,
   if (info == NULL)
     return;
 
+  gdk_threads_enter ();
   _gtk_file_system_model_update_file (model, file, info, TRUE);
+  gdk_threads_leave ();
 }
 
 static void
@@ -1174,7 +1176,9 @@ gtk_file_system_model_monitor_change (GFileMonitor *      monitor,
                                  model);
         break;
       case G_FILE_MONITOR_EVENT_DELETED:
+	gdk_threads_enter ();
         remove_file (model, file);
+	gdk_threads_leave ();
         break;
       case G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT:
         /* FIXME: use freeze/thaw with this somehow? */
diff --git a/gtk/gtkrecentmanager.c b/gtk/gtkrecentmanager.c
index 76bbf3f..a33b58a 100644
--- a/gtk/gtkrecentmanager.c
+++ b/gtk/gtkrecentmanager.c
@@ -524,7 +524,9 @@ gtk_recent_manager_monitor_changed (GFileMonitor      *monitor,
     {
     case G_FILE_MONITOR_EVENT_CHANGED:
     case G_FILE_MONITOR_EVENT_CREATED:
+      gdk_threads_enter ();
       gtk_recent_manager_changed (manager);
+      gdk_threads_leave ();
       break;
 
     case G_FILE_MONITOR_EVENT_DELETED:
@@ -768,6 +770,8 @@ gtk_recent_manager_add_item_query_info (GObject      *source_object,
   recent_data.groups = NULL;
   recent_data.is_private = FALSE;
 
+  gdk_threads_enter ();
+
   /* Ignore return value, this can't fail anyway since all required
    * fields are set */
   gtk_recent_manager_add_full (manager, uri, &recent_data);
@@ -775,6 +779,8 @@ gtk_recent_manager_add_item_query_info (GObject      *source_object,
   manager->priv->is_dirty = TRUE;
   gtk_recent_manager_changed (manager);
 
+  gdk_threads_leave ();
+
   g_free (recent_data.mime_type);
   g_free (recent_data.app_name);
   g_free (recent_data.app_exec);
diff --git a/gtk/gtksearchenginetracker.c b/gtk/gtksearchenginetracker.c
index 641e793..68346fa 100644
--- a/gtk/gtksearchenginetracker.c
+++ b/gtk/gtksearchenginetracker.c
@@ -203,6 +203,8 @@ cursor_callback (GObject      *object,
 	GList *hits;
   gboolean success;
 
+  gdk_threads_enter ();
+
   tracker = GTK_SEARCH_ENGINE_TRACKER (user_data);
 
 	cursor = TRACKER_SPARQL_CURSOR (object);
@@ -217,6 +219,7 @@ cursor_callback (GObject      *object,
       if (cursor)
 	      g_object_unref (cursor);
 
+      gdk_threads_leave ();
       return;
     }
 
@@ -227,6 +230,7 @@ cursor_callback (GObject      *object,
 		  if (cursor)
 			  g_object_unref (cursor);
 
+		  gdk_threads_leave ();
 		  return;
 	  }
 
@@ -237,6 +241,9 @@ cursor_callback (GObject      *object,
 
   /* Get next */
   cursor_next (tracker, cursor);
+
+  gdk_threads_leave ();
+
 }
 
 static void
@@ -249,6 +256,8 @@ query_callback (GObject      *object,
   TrackerSparqlCursor *cursor;
   GError *error = NULL;
 
+  gdk_threads_enter ();
+
   tracker = GTK_SEARCH_ENGINE_TRACKER (user_data);
 
   tracker->priv->query_pending = FALSE;
@@ -264,16 +273,19 @@ query_callback (GObject      *object,
     {
       _gtk_search_engine_error (GTK_SEARCH_ENGINE (tracker), error->message);
       g_error_free (error);
+      gdk_threads_leave ();
       return;
     }
 
   if (!cursor)
 	  {
 		  _gtk_search_engine_finished (GTK_SEARCH_ENGINE (tracker));
+		  gdk_threads_leave ();
 		  return;
 	  }
 
   cursor_next (tracker, cursor);
+  gdk_threads_leave ();
 }
 
 static void

commit c267d2d3c194febb4f14444aed712cedee70d6b9
Author: Alexander Larsson <alexl redhat com>
Date:   Fri Mar 25 10:53:05 2011 +0100

    Ensure we always grab the gdk lock in async callbacks
    
    Async callbacks are delivered in idles, so we need to make sure
    we get the gdk lock before calling any gdk/gtk stuff. This was
    missing in a few places.

diff --git a/gtk/gtkappchooserdialog.c b/gtk/gtkappchooserdialog.c
index 35d1d1f..3f1b798 100644
--- a/gtk/gtkappchooserdialog.c
+++ b/gtk/gtkappchooserdialog.c
@@ -121,6 +121,8 @@ search_for_mimetype_ready_cb (GObject      *source,
   GtkAppChooserDialog *self = user_data;
   GError *error = NULL;
 
+  gdk_threads_enter ();
+
   _gtk_app_chooser_online_search_for_mimetype_finish (online, res, &error);
 
   if (error != NULL)
@@ -133,6 +135,8 @@ search_for_mimetype_ready_cb (GObject      *source,
     {
       gtk_app_chooser_refresh (GTK_APP_CHOOSER (self->priv->app_chooser_widget));
     }
+
+  gdk_threads_leave ();
 }
 
 static void
@@ -155,6 +159,8 @@ app_chooser_online_get_default_ready_cb (GObject *source,
 {
   GtkAppChooserDialog *self = user_data;
 
+  gdk_threads_enter ();
+
   self->priv->online = _gtk_app_chooser_online_get_default_finish (source, res);
 
   if (self->priv->online != NULL)
@@ -176,6 +182,8 @@ app_chooser_online_get_default_ready_cb (GObject *source,
 
       gtk_widget_show (self->priv->online_button);
     }
+
+  gdk_threads_leave ();
 }
 
 static void
diff --git a/gtk/gtkfilechooserdefault.c b/gtk/gtkfilechooserdefault.c
index aa2cf78..9c6c717 100644
--- a/gtk/gtkfilechooserdefault.c
+++ b/gtk/gtkfilechooserdefault.c
@@ -6542,11 +6542,16 @@ file_system_model_got_thumbnail (GObject *object, GAsyncResult *res, gpointer da
   if (queried == NULL)
     return;
 
+  GDK_THREADS_ENTER ();
+
   /* now we know model is valid */
 
   /* file was deleted */
   if (!_gtk_file_system_model_get_iter_for_file (model, &iter, file))
-    return;
+    {
+      GDK_THREADS_LEAVE ();
+      return;
+    }
 
   info = g_file_info_dup (_gtk_file_system_model_get_info (model, &iter));
 
@@ -6557,6 +6562,8 @@ file_system_model_got_thumbnail (GObject *object, GAsyncResult *res, gpointer da
   _gtk_file_system_model_update_file (model, file, info, FALSE);
 
   g_object_unref (info);
+
+  GDK_THREADS_LEAVE ();
 }
 
 static gboolean
diff --git a/gtk/gtkfilesystem.c b/gtk/gtkfilesystem.c
index 1b81680..30ff484 100644
--- a/gtk/gtkfilesystem.c
+++ b/gtk/gtkfilesystem.c
@@ -1324,6 +1324,8 @@ query_created_file_info_callback (GObject      *source_object,
       return;
     }
 
+  gdk_threads_enter ();
+
   folder = GTK_FOLDER (user_data);
   gtk_folder_add_file (folder, file, info);
 
@@ -1332,6 +1334,7 @@ query_created_file_info_callback (GObject      *source_object,
   g_slist_free (files);
 
   g_object_unref (info);
+  gdk_threads_leave ();
 }
 
 static void
diff --git a/gtk/gtkfilesystemmodel.c b/gtk/gtkfilesystemmodel.c
index c6532c1..92f44dd 100644
--- a/gtk/gtkfilesystemmodel.c
+++ b/gtk/gtkfilesystemmodel.c
@@ -1149,7 +1149,9 @@ gtk_file_system_model_query_done (GObject *     object,
   if (info == NULL)
     return;
 
+  gdk_threads_enter ();
   _gtk_file_system_model_update_file (model, file, info, TRUE);
+  gdk_threads_leave ();
 }
 
 static void
@@ -1174,7 +1176,9 @@ gtk_file_system_model_monitor_change (GFileMonitor *      monitor,
                                  model);
         break;
       case G_FILE_MONITOR_EVENT_DELETED:
+	gdk_threads_enter ();
         remove_file (model, file);
+	gdk_threads_leave ();
         break;
       case G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT:
         /* FIXME: use freeze/thaw with this somehow? */
diff --git a/gtk/gtkrecentmanager.c b/gtk/gtkrecentmanager.c
index 76bbf3f..a33b58a 100644
--- a/gtk/gtkrecentmanager.c
+++ b/gtk/gtkrecentmanager.c
@@ -524,7 +524,9 @@ gtk_recent_manager_monitor_changed (GFileMonitor      *monitor,
     {
     case G_FILE_MONITOR_EVENT_CHANGED:
     case G_FILE_MONITOR_EVENT_CREATED:
+      gdk_threads_enter ();
       gtk_recent_manager_changed (manager);
+      gdk_threads_leave ();
       break;
 
     case G_FILE_MONITOR_EVENT_DELETED:
@@ -768,6 +770,8 @@ gtk_recent_manager_add_item_query_info (GObject      *source_object,
   recent_data.groups = NULL;
   recent_data.is_private = FALSE;
 
+  gdk_threads_enter ();
+
   /* Ignore return value, this can't fail anyway since all required
    * fields are set */
   gtk_recent_manager_add_full (manager, uri, &recent_data);
@@ -775,6 +779,8 @@ gtk_recent_manager_add_item_query_info (GObject      *source_object,
   manager->priv->is_dirty = TRUE;
   gtk_recent_manager_changed (manager);
 
+  gdk_threads_leave ();
+
   g_free (recent_data.mime_type);
   g_free (recent_data.app_name);
   g_free (recent_data.app_exec);
diff --git a/gtk/gtksearchenginetracker.c b/gtk/gtksearchenginetracker.c
index 641e793..68346fa 100644
--- a/gtk/gtksearchenginetracker.c
+++ b/gtk/gtksearchenginetracker.c
@@ -203,6 +203,8 @@ cursor_callback (GObject      *object,
 	GList *hits;
   gboolean success;
 
+  gdk_threads_enter ();
+
   tracker = GTK_SEARCH_ENGINE_TRACKER (user_data);
 
 	cursor = TRACKER_SPARQL_CURSOR (object);
@@ -217,6 +219,7 @@ cursor_callback (GObject      *object,
       if (cursor)
 	      g_object_unref (cursor);
 
+      gdk_threads_leave ();
       return;
     }
 
@@ -227,6 +230,7 @@ cursor_callback (GObject      *object,
 		  if (cursor)
 			  g_object_unref (cursor);
 
+		  gdk_threads_leave ();
 		  return;
 	  }
 
@@ -237,6 +241,9 @@ cursor_callback (GObject      *object,
 
   /* Get next */
   cursor_next (tracker, cursor);
+
+  gdk_threads_leave ();
+
 }
 
 static void
@@ -249,6 +256,8 @@ query_callback (GObject      *object,
   TrackerSparqlCursor *cursor;
   GError *error = NULL;
 
+  gdk_threads_enter ();
+
   tracker = GTK_SEARCH_ENGINE_TRACKER (user_data);
 
   tracker->priv->query_pending = FALSE;
@@ -264,16 +273,19 @@ query_callback (GObject      *object,
     {
       _gtk_search_engine_error (GTK_SEARCH_ENGINE (tracker), error->message);
       g_error_free (error);
+      gdk_threads_leave ();
       return;
     }
 
   if (!cursor)
 	  {
 		  _gtk_search_engine_finished (GTK_SEARCH_ENGINE (tracker));
+		  gdk_threads_leave ();
 		  return;
 	  }
 
   cursor_next (tracker, cursor);
+  gdk_threads_leave ();
 }
 
 static void
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]