GNOME Bugzilla – Bug 682571
Gdm does not clean up X properly
Last modified: 2012-09-05 01:29:07 UTC
1) Log in with gdm 3.5.90 2) Lock and unlock the screen at least once 3) Log out normally 4) Enjoy the fact that X is not dead, but you're dropped into text mode What's happening: The reauthentication mechanism is leaking a GSimpleAsyncResult, which in turn keeps the GdmSimpleSlave alive. gdm_slave_stopped() causes the main loop to exit, and would finalize the simple slave (causing in turn gdm_slave_stop and thus gdm_server_stop), except that there is extra ref. So the gdm-simple-slave closes without ever stopping X. Patches coming.
Created attachment 222259 [details] [review] Fix stopping of slaves Previously code would emit stopped signal directly, and rely on the side effect of main dropping the last reference to do the actual stop (including killing X). As this would break in case of memory leaks, move to calling stop when the session exits. Also, ensure that stop can be called multiple times (as it still called from finalize).
Created attachment 222260 [details] [review] Fix memory leak in gdm_simple_slave_open_reauthentication_channel The initial reference to the GSimpleAsyncResult was never cleared, as removing from the hash table only cleared the added one. Also, move removing from the hash table to GdmSession signal handler, as shown in GIO documentation, as g_simple_async_result_complete_in_idle ensures that the result stays alive. This way we can use the async function as the source_tag.
Created attachment 222441 [details] [review] GdmServer: take a reference to self in child watch function Emitting server-died or server-exited will trigger finalization through gdm_slave_stop(), so must keep a reference to avoid a segfault towards the end of the function. A small regression caused by freeing stuff too early.
Ping? Can this be integrated in 3.5.91?
yea, i've started to review this a couple of times, my attention just gets diverted to other things, and this really stirs up how things work so I want to fully understand what's going on before putting it in. Will get to it before I release on monday/tuesday.
Attachment 222441 [details] pushed as 09c1058 - GdmServer: take a reference to self in child watch function Okay, I've pushed this. I'm still not 100% confident on attachment 222259 [details] [review] but looking it over, I don't see any immediate problems.