GNOME Bugzilla – Bug 703073
frequent crash in dconf_engine_change_notify
Last modified: 2013-06-25 19:28:42 UTC
See https://bugzilla.redhat.com/show_bug.cgi?id=975687 This is showing up as a frequent f19 crash in retrace.fedoraproject.org The stacktrace has one thread doing: #0 dconf_engine_change_notify at dconfsettingsbackend.c:242 #1 dconf_engine_emit_changes at dconf-engine.c:871 #2 dconf_engine_change_completed at dconf-engine.c:948 #3 dconf_gdbus_method_call_done at dconf-gdbus-thread.c:230 #4 g_simple_async_result_complete at gsimpleasyncresult.c:777 #5 g_dbus_connection_call_done at gdbusconnection.c:5333 #6 g_simple_async_result_complete at gsimpleasyncresult.c:777 #7 complete_in_idle_cb at gsimpleasyncresult.c:789 #11 g_main_context_iteration at gmain.c:3762 #12 dconf_gdbus_worker_thread at dconf-gdbus-thread.c:81 And the other:
+ Trace 232144
Created attachment 247765 [details] [review] DConfChangeset: expose concept of "sealing" DConfChangeset is a partially threadsafe type. when first created, it is mutable and can only be used from one thread. After it is filled in, the intention is that it can be shared between threads as long as it isn't changed. Previously, this transition was made when dconf_changeset_describe() was called. After that, it was not possible to make any more changes. Formalise and document this concept and add an explicit call for it: dconf_changeset_seal().
Created attachment 247766 [details] [review] engine: seal changesets on changes When we do change operations, make sure we seal our DConfChangeset before sharing it between threads. This will ensure it gets sealed in only one thread instead of being implicitly sealed in two different threads at the same time when each of them calls dconf_changeset_describe().
Review of attachment 247765 [details] [review]: Not sure how seriously you treat the docs for internal apis, but it might be nice to specify which apis require an unsealed DconfChangeset, and also which ones return an unsealed one - e.g. copy seems to return an unsealed changeset, even if the original was sealed.
Review of attachment 247765 [details] [review]: ::: common/dconf-changeset.c @@ +546,3 @@ n_items = g_hash_table_size (changeset->table); + if (!changeset->is_sealed) This is a redundant conditional given that sealing is idempotent. Better to not have callers peek at the internals too.
Review of attachment 247766 [details] [review]: Makes sense to me.
Review of attachment 247766 [details] [review]: ::: engine/dconf-engine.c @@ +1036,3 @@ return FALSE; + dconf_changeset_seal (changeset); Do you have to seal before calling changes_only_writable_keys, to prevent that from changing underneath you ?
Ignore the last comment - the idea of sealing is that as long as its still unsealed, it is private to your thread...
Attachment 247765 [details] pushed as 40f887d - DConfChangeset: expose concept of "sealing" Attachment 247766 [details] pushed as ba512dc - engine: seal changesets on changes