GNOME Bugzilla – Bug 400793
first login after logout fails with CK
Last modified: 2007-02-05 05:15:43 UTC
I can confirm what Matthias noticed in bug 335786. "I note some issue with a patched gdm 2.17.5: The first login attempt after a logout fails (I see a vt switch, then I end up back at the gdm login), but the next attempt succeeds."
+ Trace 105268
So, it looks like the trouble starts as the previous session ends and close_ck_session is called. Then when the dbus connection is used again the assert is hit. My guess is that one of the slave processes is closing fds and this is confusing dbus-glib. Perhaps we should be using a private connection anyway but this seems to non-standard with the glib bindings.
CloseSession fails with: "Connection was disconnected before a reply was received" If I patch dbus-glib to add a new function dbus_g_bus_get_private() then CloseSession still fails but the following open_ck_session works.
Created attachment 81242 [details] [review] test patch This is the patch that I tested this with. Of course this only works if dbus-glib is patched to add dbus_g_bus_get_private(). This does have to other parts that probably should be committed: * a build fix for the greeter * converting g_warnings to gdm_errors so they are logged Since CloseSession still fails there is still a problem somewhere. However, using a private connection is more failsafe and desirable either way...
Could you get the needed dbus-glib patch upstream before I commit a fix for GDM? Or find a way to fix the problem that doesn't require modifying dbus-glib?
That patch was just a test not a fix. Still trying to understand the problem.
Created attachment 81292 [details] [review] patch Ok, first I needed to be able to get debug output from the dbus client side library. So I "export DBUS_VERBOSE=1" in /etc/X11/prefdm (also could have been in /usr/sbin/gdm) and changed the output redirection from /dev/null to /tmp/gdm.log. However, that didn't work because in gdm we explicitly set the stdout/err to NULL for both the master and slave process. So, in the attached patch you will see that I changed it to only close fds above 2 and inherit the stdin/out/err from the parent. I think that is a reasonable thing to do in general but isn't really necessary for this patch. Once I was able to see the dbus debug output it was showing that the socket was disconnecting sometime around the time CloseSession was called. But the is-disconnected state was never propagated to the rest of the library until later so the proxy calls didn't fail in a well defined way. Then it occurred to me that we don't reenter the main loop at all while waiting for the session child process. We do some selects, loop, and then waitpid. This breaks the dbus client library. Long term I think it makes sense to rework the slave code around a mainloop (ie. more object oriented, see bug #376010). However, for now I think an appropriate fix is to just explicitly interate the main loop. It seems to work. This patch also explicitly says to not exit on bus disconnection. It still seems like there is a bug somewhere causing the disconnect in the first place. But I still think these changes are probably ok. The other changes in this patch are: * build fix for greeter * change g_warning to gdm_error so they are logged
William, thanks for looking into this. When you say that "we don't reenter the main loop at all while waiting for the session child process.". I assume you are talking about the slave daemon process here since I note the ConsoleKit code is only called from daemon/slave.c. I worry about a patch that makes the slave iterate around a mainloop in the daemon/consolekit.c code because this code is configurable. If you run configure with consolekit turned off, then this code doesn't get built at all. I think we should avoid making GDM run in very different ways based on configuration choices. If it is necessary to rework the slave code to make it work with D-Bus, then this really should be done in a more general way - not in the ConsoleKit specific code. Your thoughts? Also you say that you would like to better understand why D-Bus is causing the disconnect and that you think your patch is "probably ok". This sort of language doesn't make me feel really confident this is the right patch. :) Perhaps it makes more sense to backout ConsoleKit support in GDM 2.18 or at least update configure to not turn on ConsoleKit support by default. It doesn't seem ready. It seems more "experimental" code that is "try at your own risk". There are other bugs (e.g. bug #134696) regarding the fact that GDM doesn't nicely allow people to access stdout/stderr. I don't think the "#if 0" code in your patch is the right way to do this, though. I'd say that if debug/Enable=true in the configuration file, that it would be okay to avoid closing the file descriptors - and allow debug stderr/stdout messages to go somewhere more useful for debugging. This would be a better approach I think.
Created attachment 81610 [details] [review] patch Well in the end it seems that the glib bindings just aren't up to this task. I've reworked this to use the lower level API and everything works now. This patch should be ready to commit. Look OK?
Looks good, please commit.
Done. Thanks.