summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRyan Lortie <desrt@desrt.ca>2013-06-25 14:39:26 -0400
committerRyan Lortie <desrt@desrt.ca>2013-07-16 12:21:26 -0400
commit0b1b70a53da918b579d55d927c6d019bebe7ecc9 (patch)
treed6f383cf9737a85020215c789a38e13204d17758
parenta58995657d95390e6793fefe101b0155a0217191 (diff)
downloaddconf-0b1b70a53da918b579d55d927c6d019bebe7ecc9.tar.gz
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
-rw-r--r--common/dconf-changeset.c49
-rw-r--r--common/dconf-changeset.h2
-rw-r--r--docs/dconf-sections.txt1
-rw-r--r--engine/dconf-engine.c4
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,