Comment 3 for bug 789389

Jason Conti (jconti) wrote :

I noticed the patches in lightdm-0.3.7-0ubuntu2. The TERM signal is working, and lightdm tries to clean up before exiting, but with the processes hash table now owning a reference to each child process, none of them are ever finalized. The user sessions should actually be finalized when they are removed from the hash table in child-process.c/child_process_watch_cb but a small bug in the patch prevents this, because process->priv->pid is set to zero, so g_hash_table_remove tries to remove the key for pid 0, which doesn't exist. That is an easy fix though (just use the pid passed to child_process_watch_cb instead).

The more complicated problem is the xserver child process. When shutting down, this is unrefed by the display and the display-manager, however there is one more reference owned by the hash table.

The first idea I had to fix it was to assume the display stole a reference to the xserver (get rid of the g_object_ref in display.c/display_set_xserver) and update child-process.c/child_process_finalize to use g_hash_table_steal instead (if the extra refs stay, it should really be g_hash_table_steal anyway, because we're in finalize, so don't want to g_object_unref it again). This is kind of awkward though, because then either the display manager or the display assume the hidden reference to the child process in the hash table. (it works though)

The second idea was to get rid of the extra refs for the hash table and delete the lines in child-process.c/child_process_watch_cb that set process->priv->pid to zero and remove it from the hash table. It is really unnecessary because it will be removed from the hash table when the child is finalized anyway. This works but the g_debug statement needs to be removed from from_child_cb as well, because it seems the child process may not be valid in either of this callbacks (possibly more). This seems kind of awkward too.

The third idea I haven't tried yet is to add code to clean out the processes hash table before exiting (possibly in lightdm.c/signal_cb).