GNOME Bugzilla – Bug 759708
NetworkAgent: Fix double-unref in get_secrets_keyring_cb()
Last modified: 2016-03-22 07:43:27 UTC
In get_secrets_keyring_cb, we own a ref on the 'attributes' hash table from secret_item_get_attributes), and a ref on the 'secret' object (from secret_item_get_secret(), but in the SHELL_KEYRING_SK_TAG case, we unref these once before breaking out of the loop, and the second time after breaking out of the loop.
Created attachment 317704 [details] [review] NetworkAgent: Fix double-unref in get_secrets_keyring_cb()
*** Bug 759707 has been marked as a duplicate of this bug. ***
Review of attachment 317704 [details] [review]: LGTM
Attachment 317704 [details] pushed as 021cecb - NetworkAgent: Fix double-unref in get_secrets_keyring_cb()
If more 3.18 releases are planned, it would be worth applying it there as well.
(In reply to Christophe Fergeau from comment #5) > If more 3.18 releases are planned Dunno > it would be worth applying it there as > well. Done regardless. Debian unstable currently has a broken combination of libsecret 0.18.4 plus gnome-shell 3.18.4. I think we didn't hear yelling because they still have gnome-shell 3.18.1 in testing, which is strange, but it's Debian so whatever.
> broken combination of libsecret 0.18.4 plus gnome-shell 3.18.4. We released gnome-shell 3.18.[012] with libsecret 0.18.3 (according to http://ftp.acc.umu.se/pub/GNOME/teams/releng/3.18.[012]/versions); how did it happen that three micro stable versions made for a broken combination, and how would anyone know?
(In reply to Frederic Peters from comment #7) > how did > it happen that three micro stable versions made for a broken combination, We just got unlucky. Christophe fixed a refcounting error in libsecret (missing unref) which happened to expose a refcounting error (extra unref) in gnome-shell. It's pretty common and usually safe for developers to fix memory errors in stable branches, so I don't think anything went wrong here process-wise, besides that the patch in this bug should have been immediately backported. Then we got extra unlucky, because we didn't do a libsecret update for Fedora, so this didn't hit the distro that most developers are using. Or you can say that is a good thing; the scope was limited. :) > and how would anyone know? In Endless, after a libsecret update, this was causing the shell to crash when activating a VPN connection. I don't know if the issue manifested in exactly the same way upstream, since our code is based on very old GNOME, but something must be crashing, or this bug wouldn't exist.
(In reply to Michael Catanzaro from comment #8) > Then we got extra unlucky, because we didn't do a libsecret update for > Fedora, so this didn't hit the distro that most developers are using. Or you > can say that is a good thing; the scope was limited. :) To be clear, I think this only hit sid, Tumbleweed, and Arch.
Thanks for the details.