[systemd-devel] [Patch 0/2] logind: make sure that closed sessions will be cleaned

[Djalal Harouni]

On logout pam_systemd should ensures the following:
"If the last concurrent session of a user ends, the $XDG_RUNTIME_DIR
directory and all its contents are removed, too." from manpage.

Using git HEAD, and a simple systemd-nspawn test will show that the
above is not ensured and the sessions will stay!


A simple systemd-nspawn test:

1) login as user X
2) logout
3) login as user Y
4) loginctl      (will list session of user X)


In this example we are session c4:

-bash-4.2# loginctl list-sessions
   SESSION        UID USER             SEAT 
         1       1000 tixxdz           seat0 
        c1       1000 tixxdz           seat0
        c2          0 root             seat0
        c3       1000 tixxdz           seat0
        c4          0 root             seat0

-bash-4.2# loginctl show-session --property=State 1 c1 c2 c3 c4
State=closing

State=closing

State=closing

State=closing

State=active


As shown only session c4 is active, all the others are dead sessions.

To close the dead sessions and clean things up, a dbus
TerminateSession()=>session_stop() must be issued...

Please note that I'm running without pam_loginuid.so, due to another
bug related to audit: https://bugzilla.redhat.com/show_bug.cgi?id=966807


Anyway, after some debugging:

It seems that after ReleaseSession() which is called by pam_systemd,
the user,session and seat state files will also still be available.
The garbage-collector will miss them!

In src/login/logind.c:manager_gc() the while loops will never be entered.


The user slice units will start, then the match_job_removed() and co
signals on these units will call session_add_to_gc_queue() and
user_add_to_gc_queue() to push to gc_queue when done, in the mean time
the manager_gc() will consume the gc_queue and remove them

IOW *just* before and after the ReleaseSession() the manager
"{session|user}_gc_queue" queues might be empty, hence session, users
and seats will never be cleaned! the user's slice will still be alive...


To fix this, I'm attaching two patches and I can say that they are
related to each other from the perspective of the described bug, and at
the same time they are independent of each other from a general
perspective!


1) Make method_release_session() call session_add_to_gc_queue()
This will ensure that the released session is in the gc_queue.

This change gives the garbage collector a chance to collect sessions,
and should not affect the logind behaviour and other display managers,
the session_check_gc() is able to detect if the session is still valid.

The thing is that from a quick git log the method_release_session()
never called session_add_to_gc_queue(), so this bug was introduced or
made *visible* by another change... (not sure)


2) As in commit 63966da86d8e, in function session_check_gc() the session
manager will always be around so don't check it in order to
garbage-collect the session.

Thanks!