After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 611310 - g_k_store_password and g_k_find_password doesn't work properly when keyring is locked
g_k_store_password and g_k_find_password doesn't work properly when keyring i...
Status: RESOLVED FIXED
Product: libgnome-keyring
Classification: Core
Component: General
git master
Other Linux
: Normal normal
: ---
Assigned To: GNOME keyring maintainer(s)
GNOME keyring maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2010-02-27 18:20 UTC by Krzysztof Klimonda
Modified: 2019-02-22 11:46 UTC
See Also:
GNOME target: 2.30.x
GNOME version: ---


Attachments
a small test case (1.65 KB, text/x-csrc)
2010-02-27 18:21 UTC, Krzysztof Klimonda
  Details
patch to fix this (1.78 KB, patch)
2010-03-18 02:03 UTC, Jonny Lamb
none Details | Review

Description Krzysztof Klimonda 2010-02-27 18:20:47 UTC
Version: 2.29.4git20100224-0ubuntu1 from Ubuntu 10.04

I've written a small program that stores password in keyring. It works as expected when keyring is unlocked but it doesn't work properly when keyring is locked. User is asked to unlock keyring but then password isn't stored nor is password_stored_cb called. It works on the older version of libgnome-keyring (2.26.1-0ubuntu1 from Ubuntu 9.04).
Comment 1 Krzysztof Klimonda 2010-02-27 18:21:27 UTC
Created attachment 154858 [details]
a small test case
Comment 2 Sebastien Bacher 2010-03-15 21:18:08 UTC
Empathy has a similar issue, when empathy is unlocking the keyring the accounts list is empty, nominating the bug for GNOME 2.30 since the that breaks the default im client when using autologin for quite some users.
Comment 3 Jonny Lamb 2010-03-18 02:03:45 UTC
Created attachment 156427 [details] [review]
patch to fix this

So, I had a little look at this and followed the code path with this useful small test-case, and I think I've found the problem. I've written a pretty self explanatory commit message:

commit ffb3203a19128986e73d368b9d265d89d3096d59
Author: Jonny Lamb <jonnylamb@gnome.org>
Date:   Thu Mar 18 01:56:20 2010 +0000

    prompt: keep a ref to the GkrOperation when waiting for the prompt response

    The Prompt method was being called fine, and a callback was set up for
    signals to give the response of the password prompt dialog. However,
    by the time this had happened, the GkrOperation had been finalized and
    the prompt data (and its callback for the Completed signal) were
    freed, meaning when they did get signalled, there was no handler.

    I guess you don't see this bug if you can type your password into the
    GtkEntry, press return, and make D-Bus send your signal in roughly one
    main loop iteration, unless I misunderstand?

    Fixes bug #611310 -- the test-case given in this bug, as well as
    Telepathy's Mission Control now get their callbacks called when the
    keyring is locked.

    Signed-off-by: Jonny Lamb <jonnylamb@gnome.org>
Comment 4 Stef Walter 2010-03-19 21:58:39 UTC
Thanks for that patch. It highlights the problem well. However there were some corner cases where it didn't handle things correctly. In particular the callback arguments for an operation are not freed (GDestroyNotify not called) until the operation completes. So in certain cases this could have resulted in circilar references. 

I've committed a slightly different patch which ties the operation reference (during prompting) to the dbus filter which we're using to listen for the prompt result signal.

Please verify that this fixes the problem appropriately.

commit d2936136d3f884d9bf889c57e80ef9ea3210ea7c
Author: Stef Walter <stef@memberwebs.com>
Date:   Fri Mar 19 21:55:19 2010 +0000

    Hold reference to operation while waiting for prompt result.
    
    We need to hold a reference to the prompt while waiting
    for a prompt result. We tie this reference to the dbus filter
    with which we wait for the prompt result signals.
    
    Fixes bug #611310