diff options
author | Ryan Lortie <desrt@desrt.ca> | 2013-06-25 14:39:26 -0400 |
---|---|---|
committer | Ryan Lortie <desrt@desrt.ca> | 2013-07-16 12:21:26 -0400 |
commit | 0b1b70a53da918b579d55d927c6d019bebe7ecc9 (patch) | |
tree | d6f383cf9737a85020215c789a38e13204d17758 | |
parent | a58995657d95390e6793fefe101b0155a0217191 (diff) | |
download | dconf-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.c | 49 | ||||
-rw-r--r-- | common/dconf-changeset.h | 2 | ||||
-rw-r--r-- | docs/dconf-sections.txt | 1 | ||||
-rw-r--r-- | 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, |