gsettings/: adjust to GSettingsBackend API changeswip/settings-backend

4 files changed, 391 insertions, 101 deletions

diff --git a/engine/dconf-changeset-list.h b/engine/dconf-changeset-list.h
deleted file mode 100644
index 68963b0..0000000
--- a/engine/dconf-changeset-list.h
+++ /dev/null
@@ -1,38 +0,0 @@
-/*
- * Copyright © 2010 Codethink Limited
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the licence, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the
- * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
- * Boston, MA 02111-1307, USA.
- *
- * Author: Ryan Lortie <desrt@desrt.ca>
- */
-
-#ifndef __dconf_changeset_list_h__
-#define __dconf_changeset_list_h__
-
-#include "../common/dconf-changeset.h"
-
-typedef struct _DConfChangesetList                          DConfChangesetList;
-
-struct _DConfChangesetList
-{
-  GQueue queue;
-};
-
-DConfChangeset *        dconf_changeset_list_new                        (void);
-
-void                    dconf_changeset_list_free                       (DConfChangesetList *changeset_list);
-
-#endif /* __dconf_changeset_list_h__ */
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
index 89a0c67..71c3038 100644
--- a/engine/dconf-engine.c
+++ b/engine/dconf-engine.c
@@ -295,7 +295,7 @@ dconf_engine_unref (DConfEngine *engine)
     goto again;
 }
 
-static DConfEngine *
+DConfEngine *
 dconf_engine_ref (DConfEngine *engine)
 {
   g_atomic_int_inc (&engine->ref_count);
@@ -360,6 +360,24 @@ dconf_engine_is_writable (DConfEngine *engine,
   return writable;
 }
 
+gboolean
+dconf_engine_is_set (DConfEngine *engine,
+                     const gchar *key)
+{
+  gboolean set;
+
+  dconf_engine_acquire_sources (engine);
+
+  set = engine->n_sources > 0 &&
+        engine->sources[0]->writable &&
+        engine->sources[0]->values &&
+        gvdb_table_has_value (engine->sources[0]->values, key);
+
+  dconf_engine_release_sources (engine);
+
+  return set;
+}
+
 static gboolean
 dconf_engine_find_key_in_queue (GQueue       *queue,
                                 const gchar  *key,
@@ -377,7 +395,7 @@ dconf_engine_find_key_in_queue (GQueue       *queue,
 
 GVariant *
 dconf_engine_read (DConfEngine        *engine,
-                   DConfChangesetList *read_through,
+                   GQueue             *read_through,
                    const gchar        *key)
 {
   GVariant *value = NULL;
@@ -500,7 +518,7 @@ dconf_engine_read (DConfEngine        *engine,
 
       /* Step 2.  Check read_through. */
       if (read_through)
-        found_key = dconf_engine_find_key_in_queue (&read_through->queue, key, &value);
+        found_key = dconf_engine_find_key_in_queue (read_through, key, &value);
 
       /* Step 3.  Check queued changes if we didn't find it in read_through.
        *
diff --git a/engine/dconf-engine.h b/engine/dconf-engine.h
index 3c65e87..f437e37 100644
--- a/engine/dconf-engine.h
+++ b/engine/dconf-engine.h
@@ -22,7 +22,7 @@
 #ifndef __dconf_engine_h__
 #define __dconf_engine_h__
 
-#include "dconf-changeset-list.h"
+#include "../common/dconf-changeset.h"
 
 #include <gio/gio.h>
 
@@ -109,6 +109,8 @@ DConfEngine *           dconf_engine_new                                (gpointe
                                                                          GDestroyNotify           free_func);
 
 G_GNUC_INTERNAL
+DConfEngine *           dconf_engine_ref                                (DConfEngine             *engine);
+G_GNUC_INTERNAL
 void                    dconf_engine_unref                              (DConfEngine             *engine);
 
 /* Read API: always handled immediately */
@@ -116,12 +118,15 @@ G_GNUC_INTERNAL
 guint64                 dconf_engine_get_state                          (DConfEngine             *engine);
 
 G_GNUC_INTERNAL
+gboolean                dconf_engine_is_set                             (DConfEngine             *engine,
+                                                                         const gchar             *key);
+G_GNUC_INTERNAL
 gboolean                dconf_engine_is_writable                        (DConfEngine             *engine,
                                                                          const gchar             *key);
 
 G_GNUC_INTERNAL
 GVariant *              dconf_engine_read                               (DConfEngine             *engine,
-                                                                         DConfChangesetList      *read_through,
+                                                                         GQueue                  *read_through,
                                                                          const gchar             *key);
 
 G_GNUC_INTERNAL
diff --git a/gsettings/dconfsettingsbackend.c b/gsettings/dconfsettingsbackend.c
index 1725d16..384c7ab 100644
--- a/gsettings/dconfsettingsbackend.c
+++ b/gsettings/dconfsettingsbackend.c
@@ -27,82 +27,240 @@
 #include <string.h>
 
 typedef GSettingsBackendClass DConfSettingsBackendClass;
+typedef struct _DConfSettingsBackend DConfSettingsBackend;
 
-typedef struct
+/* The DConfSettingsBackend can be in one of two major states:
+ *
+ *  1) directly connected to the engine
+ *
+ *    In this case the 'changeset' and 'parent' fields are NULL.
+ *
+ *  2) acting as a delayed backend
+ *
+ *    In this case the 'changeset' field is non-NULL and the 'parent'
+ *    field points at the DConfSettingsBackend that is parent to this
+ *    backend.  This is a strong reference.
+ *
+ * In either case the 'children' list contains the list of delayed
+ * settings objects that have this object as their parent.  These are
+ * weak references (since the strong reference is in the child->parent
+ * direction).  We use GWeakRef here to make sure we don't try to send
+ * signals to an object that's already half-disposed in another thread.
+ *
+ * We use code locking.
+ *
+ * The average GSettings-using program will only ever have a single
+ * backend.  Even those that use the "delayed" functionality will only
+ * have one or two and, even in that case, most interactions will
+ * require locking of multiple backends anyway.  The logic is much
+ * easier if there is only a single lock.
+ *
+ * An interesting possibility might be to share a lock per-engine (or
+ * use the engine's lock itself).
+ */
+struct _DConfSettingsBackend
 {
-  GSettingsBackend backend;
-  DConfEngine     *engine;
-} DConfSettingsBackend;
+  GSettingsBackend      parent_instance;
+
+  DConfEngine          *engine;         /* always set */
+  DConfSettingsBackend *parent;         /* set only for delayed backends */
+
+  DConfChangeset       *changeset;      /* will be non-NULL only for delayed backends */
+  GSList               *children;       /* list of delayed backends under us (for change notification) */
+};
+
+static GMutex dconf_settings_backend_lock;
 
 static GType dconf_settings_backend_get_type (void);
 G_DEFINE_TYPE (DConfSettingsBackend, dconf_settings_backend, G_TYPE_SETTINGS_BACKEND)
 
+/* The following three functions are the only functions that ever touch
+ * the 'children' list.
+ *
+ * All three functions should be called unlocked (and each of them will
+ * acquire the lock).
+ *
+ * dconf_settings_backend_add_child: add a backend to the child list of
+ *   its parent (when creating a delayed settings backend object)
+ * dconf_settings_backend_prune_dead_child: cleanup one dead child from
+ *   the parent's list (called from finalize of a delayed backend)
+ * dconf_settings_backend_get_child_list: get a GSList of strong
+ *   references to child objects of this backend (used during the
+ *   propagation of change signals).  Caller must deep-free the list.
+ */
+static void
+dconf_settings_backend_add_child (DConfSettingsBackend *dcsb,
+                                  DConfSettingsBackend *child)
+{
+  /* Abuse the GWeakRef API a bit.
+   *
+   * We use the ->data pointer here, knowing that GWeakRef is the same
+   * size as a pointer.
+   *
+   * On finalize of the child, the weak ref will have been set to NULL,
+   * so we can just remove NULL from the list.
+   */
+  g_mutex_lock (&dconf_settings_backend_lock);
+  dcsb->children = g_slist_prepend (dcsb->children, NULL);
+  g_weak_ref_init ((GWeakRef *) &dcsb->children->data, child);
+  g_mutex_unlock (&dconf_settings_backend_lock);
+}
+
+static void
+dconf_settings_backend_prune_dead_child (DConfSettingsBackend *dcsb)
+{
+  /* Since we're storing weakrefs in our 'children' list, they will be
+   * set back to NULL automatically when the child is freed.
+   *
+   * All that is left is to prune those values from the list so that it
+   * doesn't grow unboundedly as we add and remove children.
+   *
+   * This is called each time we remove a child, so we only really need
+   * to remove one NULL each time.  Even if there is a race and we
+   * remove the "wrong" NULL (ie: the one that used to belong to the
+   * child being finalised in the other thread), the other thread will
+   * remove the one that used to belong to us...
+   *
+   * N.B. It _should_ be safe to access the GWeakRef directly, although
+   * it's definitely 'evil'...
+   */
+  g_mutex_lock (&dconf_settings_backend_lock);
+  dcsb->children = g_slist_remove (dcsb->children, NULL);
+  g_mutex_unlock (&dconf_settings_backend_lock);
+}
+
+static GSList *
+dconf_settings_backend_get_child_list (DConfSettingsBackend *dcsb)
+{
+  GSList *children = NULL;
+  GSList *node;
+
+  /* Turn the instance variable list of weak reference to child objects
+   * into a local copy: a list of strong references.  This ensures that
+   * nobody is freeing objects in another thread as we're trying to
+   * report changes to them.
+   */
+  g_mutex_lock (&dconf_settings_backend_lock);
+  for (node = dcsb->children; node; node = node->next)
+    {
+      DConfSettingsBackend *child;
+
+      child = g_weak_ref_get ((GWeakRef *) &node->data);
+
+      if (child)
+        children = g_slist_prepend (children, child);
+    }
+  g_mutex_unlock (&dconf_settings_backend_lock);
+
+  return children;
+}
+
 static GVariant *
 dconf_settings_backend_read (GSettingsBackend   *backend,
                              const gchar        *key,
-                             const GVariantType *expected_type,
-                             gboolean            default_value)
+                             const GVariantType *expected_type)
 {
   DConfSettingsBackend *dcsb = (DConfSettingsBackend *) backend;
+  GVariant *value;
 
-  /* XXX default value */
+  if (dcsb->changeset)
+    /* The "delayed" case -- need to provide the read_through list */
+    {
+      GQueue read_through = G_QUEUE_INIT;
+      DConfSettingsBackend *node;
+
+      /* We hold the lock for the entire duration of the read in order
+       * to ensure that no other threads are modifying the changesets
+       * while _read() may be iterating over the queue.
+       *
+       * It might be possible to avoid this if we had copy-on-write
+       * changesets, but it's probably not worth the fuss...
+       */
+      g_mutex_lock (&dconf_settings_backend_lock);
+
+      /* Collect the changeset from each backend up to the toplevel one.
+       *
+       * The queue will be iterated from tail to head so we need to make
+       * sure that the "most delayed" changset is the one at the tail.
+       * We do this by prepending parents to the head.
+       */
+      for (node = dcsb; node->changeset; node = node->parent)
+        g_queue_push_head (&read_through, node->changeset);
+
+      /* Actually do the read */
+      value = dconf_engine_read (dcsb->engine, &read_through, key);
+
+      /* Free the queue */
+      g_queue_clear (&read_through);
+
+      /* Drop the lock */
+      g_mutex_unlock (&dconf_settings_backend_lock);
+    }
 
-  return dconf_engine_read (dcsb->engine, NULL, key);
+  else
+    /* Normal read case. */
+    value = dconf_engine_read (dcsb->engine, NULL, key);
+
+  return value;
 }
 
 static gboolean
 dconf_settings_backend_write (GSettingsBackend *backend,
                               const gchar      *key,
-                              GVariant         *value,
-                              gpointer          origin_tag)
+                              GVariant         *value)
 {
   DConfSettingsBackend *dcsb = (DConfSettingsBackend *) backend;
-  DConfChangeset *change;
   gboolean success;
 
-  change = dconf_changeset_new ();
-  dconf_changeset_set (change, key, value);
+  if (dcsb->changeset)
+    {
+      /* We check for writability while holding the lock in order to
+       * ensure that we don't get an interleaved writability change
+       * event in another thread after we check but before we set.
+       *
+       * If the writability change event does come _after_ the set
+       * then it will remove the change from the changeset.
+       */
+      g_mutex_lock (&dconf_settings_backend_lock);
+
+      success = dconf_engine_is_writable (dcsb->engine, key);
+
+      if (success)
+        {
+          dconf_changeset_set (dcsb->changeset, key, value);
+          /* emit changes... */
+        }
+
+      g_mutex_unlock (&dconf_settings_backend_lock);
+    }
+  else
+    {
+      DConfChangeset *changeset;
+
+      changeset = dconf_changeset_new ();
+      dconf_changeset_set (changeset, key, value);
 
-  success = dconf_engine_change_fast (dcsb->engine, change, origin_tag, NULL);
-  dconf_changeset_unref (change);
+      success = dconf_engine_change_fast (dcsb->engine, changeset, NULL, NULL);
+      dconf_changeset_unref (changeset);
+    }
 
   return success;
 }
 
-static gboolean
-dconf_settings_backend_add_to_changeset (gpointer key,
-                                         gpointer value,
-                                         gpointer data)
+static void
+dconf_settings_backend_reset (GSettingsBackend *backend,
+                              const gchar      *key)
 {
-  dconf_changeset_set (data, key, value);
-
-  return FALSE;
+  dconf_settings_backend_write (backend, key, NULL);
 }
 
 static gboolean
-dconf_settings_backend_write_tree (GSettingsBackend *backend,
-                                   GTree            *tree,
-                                   gpointer          origin_tag)
+dconf_settings_backend_is_set (GSettingsBackend *backend,
+                               const gchar      *name)
 {
   DConfSettingsBackend *dcsb = (DConfSettingsBackend *) backend;
-  DConfChangeset *change;
-  gboolean success;
-
-  change= dconf_changeset_new ();
-  g_tree_foreach (tree, dconf_settings_backend_add_to_changeset, change);
-
-  success = dconf_engine_change_fast (dcsb->engine, change, origin_tag, NULL);
-  dconf_changeset_unref (change);
-
-  return success;
-}
 
-static void
-dconf_settings_backend_reset (GSettingsBackend *backend,
-                              const gchar      *key,
-                              gpointer          origin_tag)
-{
-  dconf_settings_backend_write (backend, key, NULL, origin_tag);
+  return dconf_engine_is_set (dcsb->engine, name);
 }
 
 static gboolean
@@ -140,6 +298,58 @@ dconf_settings_backend_sync (GSettingsBackend *backend)
   dconf_engine_sync (dcsb->engine);
 }
 
+static GSettingsBackend *
+dconf_settings_backend_delay (GSettingsBackend *backend)
+{
+  return g_object_new (dconf_settings_backend_get_type (), "parent", backend, NULL);
+}
+
+static void
+dconf_settings_backend_apply (GSettingsBackend *backend)
+{
+  DConfSettingsBackend *dcsb = (DConfSettingsBackend *) backend;
+  DConfChangeset *failed_changes = NULL;
+
+  if (dcsb->changeset == NULL)
+    return;
+
+  g_mutex_lock (&dconf_settings_backend_lock);
+
+  if (dcsb->parent->changeset)
+    dconf_changeset_apply (dcsb->parent->changeset, dcsb->changeset);
+
+  else
+    if (!dconf_engine_change_fast (dcsb->engine, dcsb->changeset, dcsb, NULL))
+      /* The engine rejected the write.  Signal the issue by emitting
+       * a change signal after unlocking.
+       */
+      failed_changes = dconf_changeset_ref (dcsb->changeset);
+
+  dconf_changeset_unref (dcsb->changeset);
+  dcsb->changeset = dconf_changeset_new ();
+
+  g_mutex_unlock (&dconf_settings_backend_lock);
+
+  g_assert (failed_changes == NULL); /* TODO: implement this */
+}
+
+static void
+dconf_settings_backend_revert (GSettingsBackend *backend)
+{
+  DConfSettingsBackend *dcsb = (DConfSettingsBackend *) backend;
+  DConfChangeset *reverted_changes = NULL;
+
+  if (dcsb->changeset == NULL)
+    return;
+
+  g_mutex_lock (&dconf_settings_backend_lock);
+  reverted_changes = dcsb->changeset;
+  dcsb->changeset = dconf_changeset_new ();
+  g_mutex_unlock (&dconf_settings_backend_lock);
+
+  g_assert (reverted_changes == NULL); /* TODO: implement this */
+}
+
 static void
 dconf_settings_backend_free_weak_ref (gpointer data)
 {
@@ -150,13 +360,35 @@ dconf_settings_backend_free_weak_ref (gpointer data)
 }
 
 static void
-dconf_settings_backend_init (DConfSettingsBackend *dcsb)
+dconf_settings_backend_set_property (GObject *object, guint prop_id,
+                                     const GValue *value, GParamSpec *pspec)
 {
-  GWeakRef *weak_ref;
+  DConfSettingsBackend *dcsb = (DConfSettingsBackend *) object;
+
+  g_assert_cmpint (prop_id, ==, 1);
+
+  /* "parent" is marked as a construct property which means we're
+   * guaranteed to hit here during construction, even if "parent" is
+   * NULL (or omitted).
+   */
+
+  dcsb->parent = g_value_dup_object (value);
+
+  if (dcsb->parent)
+    {
+      dcsb->engine = dconf_engine_ref (dcsb->parent->engine);
+      dconf_settings_backend_add_child (dcsb->parent, dcsb);
+      dcsb->changeset = dconf_changeset_new ();
+    }
+  else
+    {
+      GWeakRef *weak_ref;
+
+      weak_ref = g_slice_new (GWeakRef);
+      g_weak_ref_init (weak_ref, dcsb);
 
-  weak_ref = g_slice_new (GWeakRef);
-  g_weak_ref_init (weak_ref, dcsb);
-  dcsb->engine = dconf_engine_new (weak_ref, dconf_settings_backend_free_weak_ref);
+      dcsb->engine = dconf_engine_new (weak_ref, dconf_settings_backend_free_weak_ref);
+    }
 }
 
 static void
@@ -164,6 +396,17 @@ dconf_settings_backend_finalize (GObject *object)
 {
   DConfSettingsBackend *dcsb = (DConfSettingsBackend *) object;
 
+  if (dcsb->parent)
+    {
+      dconf_settings_backend_prune_dead_child (dcsb->parent);
+      g_object_unref (dcsb->parent);
+    }
+
+  if (dcsb->changeset)
+    dconf_changeset_unref (dcsb->changeset);
+
+  g_assert (dcsb->children == NULL);
+
   dconf_engine_unref (dcsb->engine);
 
   G_OBJECT_CLASS (dconf_settings_backend_parent_class)
@@ -171,20 +414,32 @@ dconf_settings_backend_finalize (GObject *object)
 }
 
 static void
+dconf_settings_backend_init (DConfSettingsBackend *dcsb)
+{
+}
+
+static void
 dconf_settings_backend_class_init (GSettingsBackendClass *class)
 {
   GObjectClass *object_class = G_OBJECT_CLASS (class);
 
+  object_class->set_property = dconf_settings_backend_set_property;
   object_class->finalize = dconf_settings_backend_finalize;
 
   class->read = dconf_settings_backend_read;
   class->write = dconf_settings_backend_write;
-  class->write_tree = dconf_settings_backend_write_tree;
   class->reset = dconf_settings_backend_reset;
   class->get_writable = dconf_settings_backend_get_writable;
   class->subscribe = dconf_settings_backend_subscribe;
   class->unsubscribe = dconf_settings_backend_unsubscribe;
   class->sync = dconf_settings_backend_sync;
+  class->delay = dconf_settings_backend_delay;
+
+  g_object_class_install_property (object_class, 1,
+                                   g_param_spec_object ("parent", "parent backend",
+                                                        "the parent backend for delayed backends",
+                                                        dconf_settings_backend_get_type (), G_PARAM_STATIC_STRINGS |
+                                                        G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY));
 }
 
 void
@@ -208,6 +463,49 @@ g_io_module_query (void)
   return g_strsplit (G_SETTINGS_BACKEND_EXTENSION_POINT_NAME, "!", 0);
 }
 
+static void
+dconf_settings_backend_change (DConfSettingsBackend *dcsb,
+                               const gchar          *prefix,
+                               const gchar * const  *changes,
+                               const gchar          *tag,
+                               gpointer              origin_tag)
+{
+  GSList *children;
+
+  /* Avoid reporting changes into delayed DConfSettingsBackend objects
+   * when the changes were caused by apply() being called on that same
+   * object.
+   */
+  if (dcsb == origin_tag)
+    return;
+
+  /* Make a local list of strong references to our children. */
+  children = dconf_settings_backend_get_child_list (dcsb);
+
+  /* Iterate our local list, reporting changes and dropping our
+   * references on each and deconstructing our local list as we go...
+   */
+  while (children)
+    {
+      DConfSettingsBackend *child = children->data;
+
+      dconf_settings_backend_change (child, prefix, changes, tag, origin_tag);
+      children = g_slist_delete_link (children, children);
+      g_object_unref (child);
+    }
+
+  /* Actually cause the change signals to be emitted on this backend. */
+  if (changes[1] == NULL)
+    {
+      if (g_str_has_suffix (prefix, "/"))
+        g_settings_backend_path_changed (G_SETTINGS_BACKEND (dcsb), prefix);
+      else
+        g_settings_backend_changed (G_SETTINGS_BACKEND (dcsb), prefix);
+    }
+  else
+    g_settings_backend_keys_changed (G_SETTINGS_BACKEND (dcsb), prefix, changes);
+}
+
 void
 dconf_engine_change_notify (DConfEngine         *engine,
                             const gchar         *prefix,
@@ -218,7 +516,20 @@ dconf_engine_change_notify (DConfEngine         *engine,
 {
   GWeakRef *weak_ref = user_data;
   DConfSettingsBackend *dcsb;
-
+  GSList *children = NULL;
+  GSList *node;
+
+  /* Notifies are sent on either
+   *
+   *   1) the thread on which a fast write was done; or
+   *
+   *   2) the dconf worker thread for changes reported by the service
+   *
+   * In either of those cases it's possible that a thread other than
+   * this one is currently calling unref() on the backend.  We use a
+   * weakref to make sure that we're not reporting changes to
+   * partially-dead objects.
+   */
   dcsb = g_weak_ref_get (weak_ref);
 
   if (dcsb == NULL)
@@ -227,13 +538,7 @@ dconf_engine_change_notify (DConfEngine         *engine,
   if (changes[0] == NULL)
     return;
 
-  if (changes[1] == NULL)
-    {
-      if (g_str_has_suffix (prefix, "/"))
-        g_settings_backend_path_changed (G_SETTINGS_BACKEND (dcsb), prefix, origin_tag);
-      else
-        g_settings_backend_changed (G_SETTINGS_BACKEND (dcsb), prefix, origin_tag);
-    }
-  else
-    g_settings_backend_keys_changed (G_SETTINGS_BACKEND (dcsb), prefix, changes, origin_tag);
+  dconf_settings_backend_change (dcsb, prefix, changes, tag, origin_tag);
+
+  g_object_unref (dcsb);
 }