[glib/wip/settings-backend: 5/7] GSettingsBackend: turn 'event' into a proper signal

[Ryan Lortie <ryanl src gnome org>]

commit bf866b4ddc7ecf543abcb2211d2d5b26bc05025f
Author: Ryan Lortie <desrt desrt ca>
Date:   Thu Dec 29 14:16:09 2011 -0500

    GSettingsBackend: turn 'event' into a proper signal
    
    Now that signals connected to GObjects are threadsafe, it is far easier
    for us to do it this way.

 gio/gdelayedsettingsbackend.c |   12 ++-
 gio/gsettings.c               |   23 +++---
 gio/gsettingsbackend.c        |  175 +++++------------------------------------
 gio/gsettingsbackend.h        |    2 +-
 4 files changed, 40 insertions(+), 172 deletions(-)
---
diff --git a/gio/gdelayedsettingsbackend.c b/gio/gdelayedsettingsbackend.c
index e45da36..464b9e5 100644
--- a/gio/gdelayedsettingsbackend.c
+++ b/gio/gdelayedsettingsbackend.c
@@ -209,13 +209,14 @@ g_delayed_settings_backend_revert (GDelayedSettingsBackend *delayed)
 }
 
 static void
-g_delayed_settings_got_event (GObject              *target,
-                              const GSettingsEvent *event)
+g_delayed_settings_got_event (GSettingsBackend     *backend,
+                              const GSettingsEvent *event,
+                              gpointer              user_data)
 {
-  GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (target);
+  GDelayedSettingsBackend *delayed = user_data;
 
   if (event->origin_tag != delayed->priv)
-    g_settings_backend_report_event (G_SETTINGS_BACKEND (delayed), event);
+    g_settings_backend_event (G_SETTINGS_BACKEND (delayed), event);
 }
 
 static void
@@ -223,6 +224,7 @@ g_delayed_settings_backend_finalize (GObject *object)
 {
   GDelayedSettingsBackend *delayed = G_DELAYED_SETTINGS_BACKEND (object);
 
+  g_signal_handlers_disconnect_by_func (delayed->priv->backend, g_delayed_settings_got_event, delayed);
   g_mutex_clear (&delayed->priv->lock);
   g_object_unref (delayed->priv->backend);
   g_tree_unref (delayed->priv->delayed);
@@ -270,7 +272,7 @@ g_delayed_settings_backend_new (GSettingsBackend *backend)
   delayed = g_object_new (G_TYPE_DELAYED_SETTINGS_BACKEND, NULL);
   delayed->priv->backend = g_object_ref (backend);
 
-  g_settings_backend_watch (delayed->priv->backend, g_delayed_settings_got_event, G_OBJECT (delayed));
+  g_signal_connect_object (delayed->priv->backend, "event", G_CALLBACK (g_delayed_settings_got_event), delayed, 0);
 
   return delayed;
 }
diff --git a/gio/gsettings.c b/gio/gsettings.c
index 1c5621c..d5db03c 100644
--- a/gio/gsettings.c
+++ b/gio/gsettings.c
@@ -351,10 +351,11 @@ g_settings_dispatch_signal (GSettings          *settings,
 }
 
 static void
-g_settings_got_event (GObject              *target,
-                      const GSettingsEvent *event)
+g_settings_got_event (GSettingsBackend     *backend,
+                      const GSettingsEvent *event,
+                      gpointer              user_data)
 {
-  GSettings *settings = G_SETTINGS (target);
+  GSettings *settings = user_data;
   const gchar *prefix;
   const gchar *path;
   gint prefix_len;
@@ -628,9 +629,8 @@ g_settings_constructed (GObject *object)
   if (settings->priv->backend == NULL)
     settings->priv->backend = g_settings_backend_get_default ();
 
-  g_settings_backend_watch (settings->priv->backend, g_settings_got_event, G_OBJECT (settings));
-  g_settings_backend_subscribe (settings->priv->backend,
-                                settings->priv->path);
+  g_signal_connect_object (settings->priv->backend, "event", G_CALLBACK (g_settings_got_event), settings, 0);
+  g_settings_backend_subscribe (settings->priv->backend, settings->priv->path);
 }
 
 static void
@@ -639,8 +639,7 @@ g_settings_finalize (GObject *object)
   GSettings *settings = G_SETTINGS (object);
 
   g_signal_handlers_disconnect_by_func (settings->priv->backend, g_settings_got_has_unapplied_notify, settings);
-  g_settings_backend_unsubscribe (settings->priv->backend,
-                                  settings->priv->path);
+  g_signal_handlers_disconnect_by_func (settings->priv->backend, g_settings_got_event, settings);
   g_main_context_unref (settings->priv->main_context);
   g_object_unref (settings->priv->backend);
   g_settings_schema_unref (settings->priv->schema);
@@ -1946,14 +1945,14 @@ g_settings_delay (GSettings *settings)
     return;
 
   settings->priv->delayed = g_delayed_settings_backend_new (settings->priv->backend);
-  g_settings_backend_unwatch (settings->priv->backend, G_OBJECT (settings));
+  g_signal_handlers_disconnect_by_func (settings->priv->backend, g_settings_got_event, settings);
   g_object_unref (settings->priv->backend);
 
   settings->priv->backend = G_SETTINGS_BACKEND (settings->priv->delayed);
-  g_settings_backend_watch (settings->priv->backend, g_settings_got_event, G_OBJECT (settings));
+  g_signal_connect_object (settings->priv->backend, "event", G_CALLBACK (g_settings_got_event), settings, 0);
 
-  g_signal_connect (settings->priv->delayed, "notify::has-unapplied",
-                    G_CALLBACK (g_settings_got_has_unapplied_notify), settings);
+  g_signal_connect_object (settings->priv->delayed, "notify::has-unapplied",
+                           G_CALLBACK (g_settings_got_has_unapplied_notify), settings, 0);
   g_object_notify (G_OBJECT (settings), "delay-apply");
 }
 
diff --git a/gio/gsettingsbackend.c b/gio/gsettingsbackend.c
index d9ab68b..55ede78 100644
--- a/gio/gsettingsbackend.c
+++ b/gio/gsettingsbackend.c
@@ -35,13 +35,9 @@
 
 G_DEFINE_ABSTRACT_TYPE (GSettingsBackend, g_settings_backend, G_TYPE_OBJECT)
 
-typedef struct _GSettingsBackendWatch   GSettingsBackendWatch;
-
 struct _GSettingsBackendPrivate
 {
-  GSettingsBackendWatch *watches;
   gboolean has_unapplied;
-  GMutex lock;
 };
 
 enum
@@ -53,6 +49,15 @@ enum
 
 static GParamSpec *g_settings_backend_pspecs[N_PROPS];
 
+enum
+{
+  SIGNAL_EVENT,
+  N_SIGNALS
+};
+
+static guint g_settings_backend_signals[N_SIGNALS];
+
+
 /* For g_settings_backend_sync_default(), we only want to actually do
  * the sync if the backend already exists.  This avoids us creating an
  * entire GSettingsBackend in order to call a do-nothing sync()
@@ -133,140 +138,11 @@ is_path (const gchar *path)
   return TRUE;
 }
 
-struct _GSettingsBackendWatch
-{
-  GObject               *target;
-  GSettingsEventFunc     function;
-  GSettingsBackendWatch *next;
-};
-
-static void
-g_settings_backend_watch_weak_notify (gpointer  data,
-                                      GObject  *where_the_object_was)
-{
-  GSettingsBackend *backend = data;
-  GSettingsBackendWatch **ptr;
-
-  /* search and remove */
-  g_mutex_lock (&backend->priv->lock);
-  for (ptr = &backend->priv->watches; *ptr; ptr = &(*ptr)->next)
-    if ((*ptr)->target == where_the_object_was)
-      {
-        GSettingsBackendWatch *tmp = *ptr;
-
-        *ptr = tmp->next;
-        g_slice_free (GSettingsBackendWatch, tmp);
-
-        g_mutex_unlock (&backend->priv->lock);
-        return;
-      }
-
-  /* we didn't find it.  that shouldn't happen. */
-  g_assert_not_reached ();
-}
-
-/*< private >
- * g_settings_backend_watch:
- * @backend: a #GSettingsBackend
- * @target: the GObject (typically GSettings instance) to call back to
- * ...: callbacks...
- *
- * Registers a new watch on a #GSettingsBackend.
- **/
-void
-g_settings_backend_watch (GSettingsBackend   *backend,
-                          GSettingsEventFunc  callback,
-                          GObject            *target)
-{
-  GSettingsBackendWatch *watch;
-
-  /* For purposes of discussion, we assume that our target is a
-   * GSettings instance.
-   *
-   * Our strategy to defend against the final reference dropping on the
-   * GSettings object in a thread other than the one that is doing the
-   * dispatching is as follows:
-   *
-   *  1) hold a GObject reference on the GSettings during an outstanding
-   *     dispatch.  This ensures that the delivery is always possible.
-   *
-   *  2) hold a weak reference on the GSettings at other times.  This
-   *     allows us to receive early notification of pending destruction
-   *     of the object.  At this point, it is still safe to obtain a
-   *     reference on the GObject to keep it alive, so #1 will work up
-   *     to that point.  After that point, we'll have been able to drop
-   *     the watch from the list.
-   *
-   * Note, in particular, that it's not possible to simply have an
-   * "unwatch" function that gets called from the finalize function of
-   * the GSettings instance because, by that point it is no longer
-   * possible to keep the object alive using g_object_ref() and we would
-   * have no way of knowing this.
-   *
-   * All access to the list holds a mutex.  We have some strategies to
-   * avoid some of the pain that would be associated with that.
-   */
-
-  watch = g_slice_new (GSettingsBackendWatch);
-  watch->function = callback;
-  watch->target = target;
-  g_object_weak_ref (target, g_settings_backend_watch_weak_notify, backend);
-
-  /* linked list prepend */
-  g_mutex_lock (&backend->priv->lock);
-  watch->next = backend->priv->watches;
-  backend->priv->watches = watch;
-  g_mutex_unlock (&backend->priv->lock);
-}
-
 void
-g_settings_backend_unwatch (GSettingsBackend *backend,
-                            GObject          *target)
+g_settings_backend_event (GSettingsBackend     *backend,
+                          const GSettingsEvent *event)
 {
-  /* Our caller surely owns a reference on 'target', so the order of
-   * these two calls is unimportant.
-   */
-  g_object_weak_unref (target, g_settings_backend_watch_weak_notify, backend);
-  g_settings_backend_watch_weak_notify (backend, target);
-}
-
-void
-g_settings_backend_report_event (GSettingsBackend     *backend,
-                                 const GSettingsEvent *event)
-{
-  GSettingsBackendWatch *suffix, *watch, *next;
-
-  /* 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_mutex_lock (&backend->priv->lock);
-  suffix = backend->priv->watches;
-  for (watch = suffix; watch; watch = watch->next)
-    g_object_ref (watch->target);
-  g_mutex_unlock (&backend->priv->lock);
-
-  /* The suffix is now immutable, so this is safe. */
-  for (watch = suffix; watch; watch = next)
-    {
-      /* we do this here because 'watch' may not live to the end of this
-       * iteration of the loop (since we unref the target below).
-       */
-      next = watch->next;
-
-      watch->function (watch->target, event);
-      g_object_unref (watch->target); /* free the ref we acquired above */
-    }
+  g_signal_emit (backend, g_settings_backend_signals[SIGNAL_EVENT], 0, event);
 }
 
 /**
@@ -316,7 +192,7 @@ g_settings_backend_changed (GSettingsBackend *backend,
   event.keys = &null;
   event.origin_tag = origin_tag;
 
-  g_settings_backend_report_event (backend, &event);
+  g_settings_backend_event (backend, &event);
 }
 
 /**
@@ -369,7 +245,7 @@ g_settings_backend_keys_changed (GSettingsBackend    *backend,
   event.keys = (gchar **) items;
   event.origin_tag = origin_tag;
 
-  g_settings_backend_report_event (backend, &event);
+  g_settings_backend_event (backend, &event);
 }
 
 /**
@@ -418,7 +294,7 @@ g_settings_backend_path_changed (GSettingsBackend *backend,
   event.keys = &null;
   event.origin_tag = origin_tag;
 
-  g_settings_backend_report_event (backend, &event);
+  g_settings_backend_event (backend, &event);
 }
 
 /**
@@ -448,7 +324,7 @@ g_settings_backend_writable_changed (GSettingsBackend *backend,
   event.keys = &null;
   event.origin_tag = NULL;
 
-  g_settings_backend_report_event (backend, &event);
+  g_settings_backend_event (backend, &event);
 }
 
 /**
@@ -479,7 +355,7 @@ g_settings_backend_path_writable_changed (GSettingsBackend *backend,
   event.keys = &null;
   event.origin_tag = NULL;
 
-  g_settings_backend_report_event (backend, &event);
+  g_settings_backend_event (backend, &event);
 }
 
 typedef struct
@@ -821,17 +697,6 @@ g_settings_backend_get_property (GObject *object, guint prop_id,
 }
 
 static void
-g_settings_backend_finalize (GObject *object)
-{
-  GSettingsBackend *backend = G_SETTINGS_BACKEND (object);
-
-  g_mutex_clear (&backend->priv->lock);
-
-  G_OBJECT_CLASS (g_settings_backend_parent_class)
-    ->finalize (object);
-}
-
-static void
 ignore_subscription (GSettingsBackend *backend,
                      const gchar      *key)
 {
@@ -843,7 +708,6 @@ g_settings_backend_init (GSettingsBackend *backend)
   backend->priv = G_TYPE_INSTANCE_GET_PRIVATE (backend,
                                                G_TYPE_SETTINGS_BACKEND,
                                                GSettingsBackendPrivate);
-  g_mutex_init (&backend->priv->lock);
 }
 
 static void
@@ -855,7 +719,10 @@ g_settings_backend_class_init (GSettingsBackendClass *class)
   class->unsubscribe = ignore_subscription;
 
   gobject_class->get_property = g_settings_backend_get_property;
-  gobject_class->finalize = g_settings_backend_finalize;
+
+  g_settings_backend_signals[SIGNAL_EVENT] =
+    g_signal_new ("event", G_TYPE_SETTINGS_BACKEND, G_SIGNAL_RUN_FIRST, 0, NULL, NULL,
+                  g_cclosure_marshal_VOID__POINTER, G_TYPE_NONE, 1, G_TYPE_POINTER);
 
   g_settings_backend_pspecs[PROP_HAS_UNAPPLIED] =
     g_param_spec_boolean ("has-unapplied", "has unapplied", "TRUE if apply() is meaningful",
diff --git a/gio/gsettingsbackend.h b/gio/gsettingsbackend.h
index b7f89e7..3a6c19f 100644
--- a/gio/gsettingsbackend.h
+++ b/gio/gsettingsbackend.h
@@ -120,7 +120,7 @@ typedef struct
 
 GType                   g_settings_backend_get_type                     (void);
 
-void                    g_settings_backend_report_event                 (GSettingsBackend     *backend,
+void                    g_settings_backend_event                        (GSettingsBackend     *backend,
                                                                          const GSettingsEvent *event);
 void                    g_settings_backend_set_has_unapplied            (GSettingsBackend     *backend,
                                                                          gboolean              has_unapplied);