[dconf/dconf-0.16] Fix crash when using DConfChangeset in two threads



commit 0b1b70a53da918b579d55d927c6d019bebe7ecc9
Author: Ryan Lortie <desrt desrt ca>
Date:   Tue Jun 25 14:39:26 2013 -0400

    Fix crash when using DConfChangeset in two threads
    
    This is a combined cherry-pick of the following commits:
    
      40f887db43dc89e546ecef9c2d2f31a61858badc
      ba512dc4225a043b94ef13718f1cbe8a806f5b55
      cfa25f1adccf0ea097dc3339d41b29b77878c642
    
    This patch introduces API, but it does so in order to fix a bug.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703073

 common/dconf-changeset.c |   49 ++++++++++++++++++++++++++++++++++++++++-----
 common/dconf-changeset.h |    2 +
 docs/dconf-sections.txt  |    1 +
 engine/dconf-engine.c    |    4 +++
 4 files changed, 50 insertions(+), 6 deletions(-)
---
diff --git a/common/dconf-changeset.c b/common/dconf-changeset.c
index d9b9f41..54be719 100644
--- a/common/dconf-changeset.c
+++ b/common/dconf-changeset.c
@@ -54,7 +54,8 @@
 struct _DConfChangeset
 {
   GHashTable *table;
-  gboolean is_database;
+  guint is_database : 1;
+  guint is_sealed : 1;
   gint ref_count;
 
   gchar *prefix;
@@ -195,7 +196,7 @@ dconf_changeset_set (DConfChangeset *changeset,
                      const gchar    *path,
                      GVariant       *value)
 {
-  g_return_if_fail (changeset->prefix == NULL);
+  g_return_if_fail (!changeset->is_sealed);
   g_return_if_fail (dconf_is_path (path, NULL));
 
   /* Check if we are performing a path reset */
@@ -366,12 +367,44 @@ dconf_changeset_string_ptr_compare (gconstpointer a_p,
   return strcmp (*a, *b);
 }
 
-static void
-dconf_changeset_build_description (DConfChangeset *changeset)
+/**
+ * dconf_changeset_seal:
+ * @changeset: a #DConfChangeset
+ *
+ * Seals @changeset.
+ *
+ * When a #DConfChangeset is first created, it is mutable and
+ * non-threadsafe.  Once the changeset is populated with the required
+ * changes, it can be shared between multiple threads, but only by
+ * making it immutable by "sealing" it.
+ *
+ * After the changeset is sealed, you cannot call dconf_changeset_set()
+ * or any other functions that would modify it.  It is safe, however, to
+ * share it between multiple threads.
+ *
+ * All changesets are unsealed on creation, including those that are
+ * made by copying changesets that are sealed.
+ * dconf_changeset_describe() will implicitly seal a changeset.
+ *
+ * This function is idempotent.
+ *
+ * Since: 0.18
+ **/
+void
+dconf_changeset_seal (DConfChangeset *changeset)
 {
   gsize prefix_length;
   gint n_items;
 
+  if (changeset->is_sealed)
+    return;
+
+  changeset->is_sealed = TRUE;
+
+  /* This function used to be called dconf_changeset_build_description()
+   * because that's basically what sealing is...
+   */
+
   n_items = g_hash_table_size (changeset->table);
 
   /* If there are no items then what is there to describe? */
@@ -501,6 +534,9 @@ dconf_changeset_build_description (DConfChangeset *changeset)
  * The @paths array is returned in an order such that dir will always
  * come before keys contained within those dirs.
  *
+ * If @changeset is not already sealed then this call will implicitly
+ * seal it.  See dconf_changeset_seal().
+ *
  * Returns: the number of changes (the length of @changes and @values).
  **/
 guint
@@ -513,8 +549,7 @@ dconf_changeset_describe (DConfChangeset       *changeset,
 
   n_items = g_hash_table_size (changeset->table);
 
-  if (n_items && !changeset->prefix)
-    dconf_changeset_build_description (changeset);
+  dconf_changeset_seal (changeset);
 
   if (prefix)
     *prefix = changeset->prefix;
@@ -664,6 +699,8 @@ dconf_changeset_change (DConfChangeset *changeset,
   gsize prefix_len;
   gint i;
 
+  g_return_if_fail (!changeset->is_sealed);
+
   /* Handling resets is a little bit tricky...
    *
    * Consider the case that we have @changeset containing a key /a/b and
diff --git a/common/dconf-changeset.h b/common/dconf-changeset.h
index 7228105..8a9cb48 100644
--- a/common/dconf-changeset.h
+++ b/common/dconf-changeset.h
@@ -70,4 +70,6 @@ void                    dconf_changeset_change                          (DConfCh
 DConfChangeset *        dconf_changeset_diff                            (DConfChangeset           *from,
                                                                          DConfChangeset           *to);
 
+void                    dconf_changeset_seal                            (DConfChangeset           
*changeset);
+
 #endif /* __dconf_changeset_h__ */
diff --git a/docs/dconf-sections.txt b/docs/dconf-sections.txt
index 2f3e444..f21abc8 100644
--- a/docs/dconf-sections.txt
+++ b/docs/dconf-sections.txt
@@ -51,4 +51,5 @@ dconf_changeset_ref
 dconf_changeset_serialise
 dconf_changeset_set
 dconf_changeset_unref
+dconf_changeset_seal
 </SECTION>
diff --git a/engine/dconf-engine.c b/engine/dconf-engine.c
index 446619e..7beff95 100644
--- a/engine/dconf-engine.c
+++ b/engine/dconf-engine.c
@@ -1035,6 +1035,8 @@ dconf_engine_change_fast (DConfEngine     *engine,
   if (!dconf_engine_changeset_changes_only_writable_keys (engine, changeset, error))
     return FALSE;
 
+  dconf_changeset_seal (changeset);
+
   /* Check for duplicates in the pending queue.
    *
    * Note: order doesn't really matter here since "similarity" is an
@@ -1105,6 +1107,8 @@ dconf_engine_change_sync (DConfEngine     *engine,
   if (!dconf_engine_changeset_changes_only_writable_keys (engine, changeset, error))
     return FALSE;
 
+  dconf_changeset_seal (changeset);
+
   /* we know that we have at least one source because we checked writability */
   reply = dconf_engine_dbus_call_sync_func (engine->sources[0]->bus_type,
                                             engine->sources[0]->bus_name,


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]