GNOME Bugzilla – Bug 697698
Race condition in media keys
Last modified: 2013-04-10 18:09:53 UTC
For some reason I see a crash in the settings daemon in the settings_changed_cb() callback when accessing manager->priv->keys->len. It seems that the gsettings callback is called before manager->priv->keys is initialized. And that would happen after because init_kbd() which initialized manager->priv->keys is called upon shell's dbus appearance.
Created attachment 241132 [details] [review] Fix race condition
Review of attachment 241132 [details] [review]: ::: plugins/media-keys/gsd-media-keys-manager.c @@ +603,2 @@ /* Find the key that was modified */ + if (manager->priv->keys) { I would bail out of the function if the shell wasn't there yet instead. It makes it clearer what the intent is.
Created attachment 241135 [details] [review] Fix race condition v2
Review of attachment 241135 [details] [review]: Can you add a comment above the patch? Something like "Keys not initialised yet?" Looks good to commit to gnome-3-8 and master.
Created attachment 241145 [details] [review] Fix race condition v3 I actually saw another crasher in the same file. So I reworked a bit the allocation of manager->priv->keys and also left the previous check that is necessary anyway (that function is going to talk to the shell, we need to check that we have a proxy for it).
Review of attachment 241145 [details] [review]: ::: plugins/media-keys/gsd-media-keys-manager.c @@ +770,3 @@ gnome_settings_profile_start (NULL); + g_ptr_array_set_size (manager->priv->keys, 0); Shouldn't we call this when the key-grabbed goes away instead?
Created attachment 241158 [details] [review] Fix race condition v4
Review of attachment 241158 [details] [review]: Ace. Please commit to gnome-3-8 and master
Review of attachment 241158 [details] [review]: Committed.