[glib] Bug 623400 - acquire context before dispatching

[Ryan Lortie <ryanl src gnome org>]

commit 0bd50b39eb8223f2eeffe8dc5fe096b2c7695529
Author: Ryan Lortie <desrt desrt ca>
Date:   Sun Oct 3 17:30:10 2010 -0400

    Bug 623400 - acquire context before dispatching
    
    For GSettings.
    
    Use the functionality introduced in the last commit to simplify our
    notify dispatching and increase the safety of doing so (by ensuring that
    the context is acquired in the current thread for the duration of the
    dispatch).
    
    This closes bugs #623400 and #629849.

 gio/gdelayedsettingsbackend.c  |   16 +-------
 gio/gsettingsbackend.c         |   87 +++++++++++++++-------------------------
 gio/gsettingsbackendinternal.h |    2 -
 3 files changed, 33 insertions(+), 72 deletions(-)
---
diff --git a/gio/gdelayedsettingsbackend.c b/gio/gdelayedsettingsbackend.c
index bb3195a..dce1d7d 100644
--- a/gio/gdelayedsettingsbackend.c
+++ b/gio/gdelayedsettingsbackend.c
@@ -70,21 +70,7 @@ g_delayed_settings_backend_notify_unapplied (GDelayedSettingsBackend *delayed)
   g_static_mutex_unlock (&delayed->priv->lock);
 
   if (target != NULL)
-    {
-      if (g_settings_backend_get_active_context () != target_context)
-        {
-          GSource *source;
-
-          source = g_idle_source_new ();
-          g_source_set_priority (source, G_PRIORITY_DEFAULT);
-          g_source_set_callback (source, invoke_notify_unapplied,
-                                 target, g_object_unref);
-          g_source_attach (source, target_context);
-          g_source_unref (source);
-        }
-      else
-        invoke_notify_unapplied (target);
-    }
+    g_main_context_invoke (target_context, invoke_notify_unapplied, target);
 }
 
 
diff --git a/gio/gsettingsbackend.c b/gio/gsettingsbackend.c
index bde0336..475d055 100644
--- a/gio/gsettingsbackend.c
+++ b/gio/gsettingsbackend.c
@@ -119,26 +119,6 @@ is_path (const gchar *path)
   return TRUE;
 }
 
-GMainContext *
-g_settings_backend_get_active_context (void)
-{
-  GMainContext *context;
-  GSource *source;
-
-  if ((source = g_main_current_source ()))
-    context = g_source_get_context (source);
-
-  else
-    {
-      context = g_main_context_get_thread_default ();
-
-      if (context == NULL)
-        context = g_main_context_default ();
-    }
-
-  return context;
-}
-
 struct _GSettingsBackendWatch
 {
   GObject                       *target;
@@ -316,20 +296,7 @@ g_settings_backend_dispatch_signal (GSettingsBackend *backend,
                                     GBoxedFreeFunc    data1_free,
                                     gpointer          data2)
 {
-  GMainContext *context, *here_and_now;
-  GSettingsBackendWatch *watch;
-
-  /* We need to hold the mutex here (to prevent a node from being
-   * deleted as we are traversing the list).  Since we should not
-   * re-enter user code while holding this mutex, we create a
-   * one-time-use GMainContext and populate it with the events that we
-   * would have called directly.  We dispatch these events after
-   * releasing the lock.  Note that the GObject reference is acquired on
-   * the target while holding the mutex and the mutex needs to be held
-   * as part of the destruction of any GSettings instance (via the weak
-   * reference handling).  This is the key to the safety of the whole
-   * setup.
-   */
+  GSettingsBackendWatch *suffix, *watch, *next;
 
   if (data1_copy == NULL)
     data1_copy = pointer_id;
@@ -337,19 +304,34 @@ g_settings_backend_dispatch_signal (GSettingsBackend *backend,
   if (data1_free == NULL)
     data1_free = pointer_ignore;
 
-  context = g_settings_backend_get_active_context ();
-  here_and_now = g_main_context_new ();
-
-  /* traverse the (immutable while holding lock) list */
+  /* We're in a little bit of a tricky situation here.  We need to hold
+   * a lock while traversing the list, but we don't want to hold the
+   * lock while calling back into user code.
+   *
+   * Since we're not holding the lock while we call user code, we can't
+   * render the list immutable.  We can, however, store a pointer to a
+   * given suffix of the list and render that suffix immutable.
+   *
+   * Adds will never modify the suffix since adds always come in the
+   * form of prepends.  We can also prevent removes from modifying the
+   * suffix since removes only happen in response to the last reference
+   * count dropping -- so just add a reference to everything in the
+   * suffix.
+   */
   g_static_mutex_lock (&backend->priv->lock);
-  for (watch = backend->priv->watches; watch; watch = watch->next)
+  suffix = backend->priv->watches;
+  for (watch = suffix; watch; watch = watch->next)
+    g_object_ref (watch->target);
+  g_static_mutex_unlock (&backend->priv->lock);
+
+  /* The suffix is now immutable, so this is safe. */
+  for (watch = suffix; watch; watch = next)
     {
       GSettingsBackendClosure *closure;
-      GSource *source;
 
       closure = g_slice_new (GSettingsBackendClosure);
       closure->backend = g_object_ref (backend);
-      closure->target = g_object_ref (watch->target);
+      closure->target = watch->target; /* we took our ref above */
       closure->function = G_STRUCT_MEMBER (void *, watch->vtable,
                                            function_offset);
       closure->name = g_strdup (name);
@@ -357,23 +339,18 @@ g_settings_backend_dispatch_signal (GSettingsBackend *backend,
       closure->data1_free = data1_free;
       closure->data2 = data2;
 
-      source = g_idle_source_new ();
-      g_source_set_priority (source, G_PRIORITY_DEFAULT);
-      g_source_set_callback (source,
-                             g_settings_backend_invoke_closure,
-                             closure, NULL);
+      /* we do this here because 'watch' may not live to the end of this
+       * iteration of the loop (since we may unref the target below).
+       */
+      next = watch->next;
 
-      if (watch->context && watch->context != context)
-        g_source_attach (source, watch->context);
+      if (watch->context)
+        g_main_context_invoke (watch->context,
+                               g_settings_backend_invoke_closure,
+                               closure);
       else
-        g_source_attach (source, here_and_now);
-
-      g_source_unref (source);
+        g_settings_backend_invoke_closure (closure);
     }
-  g_static_mutex_unlock (&backend->priv->lock);
-
-  while (g_main_context_iteration (here_and_now, FALSE));
-  g_main_context_unref (here_and_now);
 }
 
 /**
diff --git a/gio/gsettingsbackendinternal.h b/gio/gsettingsbackendinternal.h
index 68a61a8..a6711cb 100644
--- a/gio/gsettingsbackendinternal.h
+++ b/gio/gsettingsbackendinternal.h
@@ -92,8 +92,6 @@ G_GNUC_INTERNAL
 GPermission *           g_settings_backend_get_permission               (GSettingsBackend               *backend,
                                                                          const gchar                    *path);
 G_GNUC_INTERNAL
-GMainContext *          g_settings_backend_get_active_context           (void);
-G_GNUC_INTERNAL
 GSettingsBackend *      g_settings_backend_get_default                  (void);
 G_GNUC_INTERNAL
 void                    g_settings_backend_sync_default                 (void);