Confirming there, the issue seems to annoy quite some users

confirming on mandriva cooker too

I'm wondering if there's a gnome-session bug there, or a bug in how gnome-keyring implements the dbus client part of the session API.

Looking at the debug trace of gnome-session Frédéric provided me, I can see:

gnome-session[32649]: DEBUG(+): GsmDBusClient: obj_path=/org/gnome/SessionManager interface=org.gnome.SessionManager method=UnregisterClient
gnome-session[32649]: DEBUG(+): GsmManager: UnregisterClient /org/gnome/SessionManager/Client1

Then, in gsm-manager.c, I can read this:

        /* don't disconnect client here, only change the status.
           Wait until it leaves the bus before disconnecting it */
        gsm_client_set_status (client, GSM_CLIENT_UNREGISTERED);

So we're waiting for the client to leave the bus. Which doesn't seem to happen, hence the wait for the timeout. But looking at the g-k-d code, clearly, it should exit. Hrm.


Stef: btw, I would think that gnome-keyring-daemon shouldn't connect to the session manager when completing the initialization if it was originally started with --login. In that case, I would expect it should be closed with the end of the pam session.

Doing some brutal printf-debugging, I can see a 10s pause between the call to gkr_daemon_quit() and its reception in signal_thread(). Weird stuff.

Created attachment 144606 [details] [review]
Use pthread_kill instead of raise

Okay, found the issue: we use raise() to send SIGTERM to the signal thread, except that on a multi-threaded program, raise() only sends the signal to the current thread. So we have to use pkill_thread().

There's no GThread API for tihs, so this implies a change from g_thread_create() to pthread_create() too.

(note that I kept a raise() call in case there's no signal thread; this won't really work since g-k-d is still multi-threaded... not really sure there's anything we can do there)

Thanks for your work on this! I can confirm that the patch works (tested against gnome-keyring 2.28.0-0ubuntu1).

I'm confirming the patch fixes the issue, I'm pushing it in Mandriva cooker

Thanks for figuring that out Vincent. Do you know if we should cleanup the thread in any way? IOW: is pthread_join required for a pthread_create? Once that issue is out of the way, we can commit the patch.

The man page for pthread_create says:

       The new thread terminates in one of the following ways:

       * It calls pthread_exit(3), specifying an exit status value that is available to another thread in the same process that calls pthread_join(3).

       * It returns from start_routine().  This is equivalent to calling pthread_exit(3) with the value supplied in the return statement.

       * It is canceled (see pthread_cancel(3)).

       * Any of the threads in the process calls exit(3), or the main thread performs a return from main().  This causes the termination of all threads in the process.

My understanding is that we don't need to have a pthread_join: pthread_join, in this case, is only useful if you want to get the return value from the thread.

Great. Looks good to commit with these two changes: 

s/s_thread/sig_thread/
s/strerror/g_strerror/

Thanks again!

Pushed with those changes.