Accessing the keyring in a thread make the program crash (was find_credentials segfaults)

Bug #656545 reported by Alejandro J. Cura
32
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Ubuntu Single Sign On Client
Status tracked in Trunk
Stable-1-0
Fix Released
High
Alejandro J. Cura
Trunk
Fix Released
High
Alejandro J. Cura
ubuntu-sso-client (Ubuntu)
Fix Released
High
Natalia Bidart
Maverick
Fix Released
High
Natalia Bidart
Natty
Fix Released
High
Natalia Bidart

Bug Description

Calling the find_credentials in the /com/ubuntu/sso/credentials DBus object makes this service crash very often.

This is caused by accessing the keyring in a thread using functions that are not thread safe.
This also affects the /credentials service since it access the keyring in a thread to store the credentials.

FIX: initialize glib threads on dbus.mainloop.glib

Minimal patch: see linked branch.

TEST CASE: this bug is very difficult to reproduce since involves some thread weirdness. Matt griffin was able to reproduce on a clean maverick install by logging in successfully into Ubuntu One using ussoc. After login, the process died with no much info.

Related branches

Revision history for this message
Alejandro J. Cura (alecu) wrote :

It looks like calling keyring_get_credentials from a thread is not safe.
I propose renaming the current one keyring_get_credentials_sync.
Also I propose not using a thread in the new find_credentials and using the async way of accessing the keyring instead.

tags: added: desktop+ sso-network u1-natty
Changed in ubuntu-sso-client:
status: New → Confirmed
importance: Undecided → High
Changed in ubuntu-sso-client:
assignee: nobody → Alejandro J. Cura (alecu)
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Performing tests with trunk code I've got:

nessita@dali:~/canonical/ubuntu-sso-client/trunk$ killall ubuntu-sso-login; DEBUG=True PYTHONPATH=. ./bin/ubuntu-sso-login
ubuntu-sso-login: no process found
Hooking up SIGHUP with handler <function sighup_handler at 0x23d47d0>.
Starting Ubuntu SSO login manager for bus 'com.ubuntu.sso'.
keyring_delete_credentials: app_name "a".
keyring_delete_credentials: app_name "a".
**
ERROR:gkr-operation.c:342:on_pending_call_notify: assertion failed: (pending == op->pending)
Segmentation fault
nessita@dali:~/canonical/ubuntu-sso-client/trunk$

Changed in ubuntu-sso-client:
status: Confirmed → In Progress
description: updated
summary: - the new find_credentials segfaults
+ Accessing the keyring in a thread make the program crash (was
+ find_credentials segfaults)
Changed in ubuntu-sso-client (Ubuntu):
status: New → In Progress
importance: Undecided → High
assignee: nobody → Alejandro J. Cura (alecu)
tags: added: u1-maverick-sru
dobey (dobey)
Changed in ubuntu-sso-client:
status: In Progress → Fix Committed
Changed in ubuntu-sso-client (Ubuntu):
assignee: Alejandro J. Cura (alecu) → Naty Bidart (nataliabidart)
Changed in ubuntu-sso-client (Ubuntu):
status: In Progress → Triaged
milestone: none → natty-alpha-1
description: updated
description: updated
description: updated
description: updated
Revision history for this message
Martin Pitt (pitti) wrote : Please test proposed package

Accepted ubuntu-sso-client into maverick-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in ubuntu-sso-client (Ubuntu):
status: Triaged → Fix Committed
Changed in ubuntu-sso-client (Ubuntu Maverick):
status: New → Fix Committed
tags: added: verification-needed
Changed in ubuntu-sso-client (Ubuntu Maverick):
importance: Undecided → High
assignee: nobody → Naty Bidart (nataliabidart)
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Tested on a clean maverick install with -proposed enabled. Problem is very tricky to reproduce on maverick, but I can confirm I never got the seg fault.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-sso-client - 1.1.1-0ubuntu1

---------------
ubuntu-sso-client (1.1.1-0ubuntu1) natty; urgency=low

  * New upstream release (1.1.1):

    [ Alejandro J. Cura <email address hidden> ]
      * Call the dbus mainloop thread init (fixes LP: #656545).

    [ Natalia B. Bidart <email address hidden> ]
      * Added the new dbus iface to the executable. Deprecated a few Dbus
      constants.

    [ Natalia B. Bidart <email address hidden> ]
      * Adding a new DBus iface to manage credentials with flexible API (LP:
      #653113).

    [ Natalia B. Bidart <email address hidden> ]
      * Replace twisted gtk reactor with the standard gtk mainloop. (LP:
      #655327).

    [ Natalia B. Bidart <email address hidden> ]
      * The TC browser will not be shown until the TOS page has been loaded. A
      spinner is shown in the mean time (LP: #620456).
      * The TC browser does not allow link navigation other than the TOS page
      (LP: #616961).

    [ Natalia B. Bidart <email address hidden> ]
      * Terms and conditions label is now translatable. Added a button to be
      able to browse the TC (LP: 654534).

    [ Natalia B. Bidart <email address hidden> ]
      * Moved logic from DBus SSOCredentials module into a self-contained, DBus
      agnostic module called 'credentials' (LP: #653113).

    [ Natalia B. Bidart <email address hidden> ]
      * Isolating SSO login processor into a separated module (LP: #637204).

    [ Natalia B. Bidart <email address hidden> ]
      * Cleanup code and modules to prepare the code for the DBus API refactor.

    [ Natalia B. Bidart <email address hidden> ]
      * Made entries name to be unicode to avoid a faulty capitalization when
      using turkish locale (LP: #652939).

    [ Natalia B. Bidart <email address hidden> ]
      * Workaround for LP: #467397 (when using turkish locale, decimal module
      import fails).

    [ Natalia B. Bidart <email address hidden> ]
      * Added encoding declaration on every file. Fixed error passing from GUI
      in GTK signals (LP: #652318).

    [ Natalia B. Bidart <email address hidden> ]
      * Avoiding calling a callback twice for buttons (LP: #652248).

    [ Natalia B. Bidart <email address hidden> ]
      * Removed unneeded assert sentences and replaced with warning logging
      when necessary (LP: #649317).

  * New upstream release (1.1.0):

    [ Natalia B. Bidart <email address hidden> ]
      * Avoiding having the main window freezing when pinging the U1 server
      (LP: #645162).

    [ Natalia B. Bidart <email address hidden> ]
      * Do not create the login keyring, use default instead (LP: #639690).

    [ Natalia B. Bidart <email address hidden> ]
      * Avoid duplicating logging messages for module "main" (LP: #638981).

    [ Natalia B. Bidart <email address hidden> ]
      * Delay import of webkit to be able to build on non-graphical envs (LP:
      #628956).
 -- Natalia Bidart (nessita) <email address hidden> Mon, 11 Oct 2010 10:22:28 -0300

Changed in ubuntu-sso-client (Ubuntu Natty):
status: Fix Committed → Fix Released
tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-sso-client - 1.0.4-0ubuntu1

---------------
ubuntu-sso-client (1.0.4-0ubuntu1) maverick-proposed; urgency=low

  * New upstream release:

  [ Alejandro J. Cura <email address hidden> ]
    * Replace twisted gtk reactor with the standard gtk mainloop. (LP: #655327).

  [ Alejandro J. Cura <email address hidden> ]
    * Call the dbus mainloop thread init (fixes LP: #656545).

  * Adding .bzr-builddeb/default.conf as per Michael Vog (mvo) request.

  * Adding dpkg (>= 1.15.7.2) as Pre-Depends (fixes LP: #658768).

  * Adding gnome-keyring as dep since python-gnomekeyring doesn't install it.
 -- Natalia Bidart (nessita) <email address hidden> Tue, 12 Oct 2010 10:07:55 -0300

Changed in ubuntu-sso-client (Ubuntu Maverick):
status: Fix Committed → Fix Released
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

We have a recent case of segfault within ubuntu-sso-client for maverick release, so I'll be reopening this bug.

Changed in ubuntu-sso-client (Ubuntu Maverick):
status: Fix Released → Triaged
Changed in ubuntu-sso-client (Ubuntu Natty):
status: Fix Released → Triaged
Revision history for this message
Martin Pitt (pitti) wrote :

Accepted ubuntu-sso-client into maverick-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in ubuntu-sso-client (Ubuntu Maverick):
status: Triaged → Fix Committed
tags: removed: verification-done
tags: added: verification-needed
Revision history for this message
Joshua Blount (jblount) wrote :

I was able to consistently reproduce this bug before, but with -proposed it's fixed now for me.

Martin Pitt (pitti)
tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-sso-client - 1.0.7-0ubuntu1

---------------
ubuntu-sso-client (1.0.7-0ubuntu1) maverick-proposed; urgency=low

  * New upstream release (1.0.6, 1.0.7):

    [ Natalia B. Bidart <email address hidden> ]
      * Added a new DBus signal UserNotValidated to indicate when a user is
      registered but not validated (LP: #667899).
      * Added new workflow so email validation is requested if necessary.
      * The verify email page should be always built, not only on registration.

    [ Alejandro J. Cura <email address hidden> ]
      * Store credentials on the keyring *only* from the main thread (LP:
      #656545).

  * New upstream release (1.0.5):

    [ Natalia B. Bidart <email address hidden> ]

      * Credentials are removed if the pinging to the server fails or any
      other exception occurs (LP: #660516).
 -- Natalia Bidart (nessita) <email address hidden> Thu, 04 Nov 2010 09:21:00 -0300

Changed in ubuntu-sso-client (Ubuntu Maverick):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package ubuntu-sso-client - 1.1.5-0ubuntu1

---------------
ubuntu-sso-client (1.1.5-0ubuntu1) natty; urgency=low

  * New upstream release (1.1.5):

    [ Natalia B. Bidart <email address hidden> ]
      * Use "org.freedesktop.secrets" dbus service instead of
      "org.gnome.keyring" (LP: #683088).

  * New upstream release (1.1.4):

    [ Natalia B. Bidart <email address hidden> ]
      * Added a gtk.Notebook to ensure proper window resize at startup
        (LP: #682669).
      * Enabled window resizing to be more user friendly.
      * Remove outdated references to gnome keyring from docstrings.

  * New upstream release (1.1.3):

    [ Natalia B. Bidart <email address hidden> ]
      * Make UI more friendly to resizes and big fonts (LP: #627496).
      * Splitting GUI code out of backend (LP: #677518).
      * Credentials can now be stored using a DBus called (LP: #680253).
      * Status from SSO server is now case sensitive (LP: #653165).
      * Credentials should not be cleared if the ping wasn't made due to empty
        ping url (LP: #676679).

    [ Rodney Dawes <email address hidden> ]
     * Add the utils sub-package to the packages declaration so it installs
     (LP: #680593).

    [ Alejandro J. Cura <email address hidden> ]
      * Fully async keyring access thru DBus. Drops dependency with
      gnomekeyring (LP: #656545).
 -- Natalia Bidart (nessita) <email address hidden> Tue, 30 Nov 2010 10:22:11 -0300

Changed in ubuntu-sso-client (Ubuntu Natty):
status: Triaged → Fix Released
tags: added: testcase
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.