[systemd-devel] [PATCH v3] logind: make sure to terminate systemd user on logouts

[Lennart Poettering]

On Thu, 13.02.14 18:31, Djalal Harouni (tixxdz at opendz.org) wrote:

Looks good! Thanks! Applied!

> Currently if the user logs out, the GC may never call user_stop(),
> this will not terminate the systemd user and (sd-pam) of that user.
> 
> To fix this, remove the USER_CLOSING state check that is blocking the
> GC from calling user_stop(). Since if user_check_gc() returns false
> this means that all the sessions of the user were removed which will
> make user_get_state() return USER_CLOSING.
> 
> Conclusion: that test will never be statisfied.
> 
> So we remove the USER_CLOSING check and replace it with a check inside
> user_stop() this way we know that user_stop() has already queued stop
> jobs, no need to redo.
> 
> This ensures that the GC will get its two steps correctly as pointed out
> by Lennart:
> http://lists.freedesktop.org/archives/systemd-devel/2014-February/016825.html
> 
> Note: this also fixes another bug that prevents creating the user
> private dbus socket which will break communications with the user
> manager.
> 
> ---
> v2 -> v3  rebase
> v1 -> v2  updated based on Lennart suggestions
> 
>  src/login/logind-user.c | 6 ++++++
>  src/login/logind.c      | 5 +++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/login/logind-user.c b/src/login/logind-user.c
> index ac4a651..4af0e90 100644
> --- a/src/login/logind-user.c
> +++ b/src/login/logind-user.c
> @@ -499,6 +499,12 @@ int user_stop(User *u, bool force) {
>          int r = 0, k;
>          assert(u);
>  
> +        /* Stop jobs have already been queued */
> +        if (u->stopping) {
> +                user_save(u);
> +                return r;
> +        }
> +
>          LIST_FOREACH(sessions_by_user, s, u->sessions) {
>                  k = session_stop(s, force);
>                  if (k < 0)
> diff --git a/src/login/logind.c b/src/login/logind.c
> index 5544099..2add241 100644
> --- a/src/login/logind.c
> +++ b/src/login/logind.c
> @@ -889,10 +889,11 @@ void manager_gc(Manager *m, bool drop_not_started) {
>                  LIST_REMOVE(gc_queue, m->user_gc_queue, user);
>                  user->in_gc_queue = false;
>  
> -                if (!user_check_gc(user, drop_not_started) &&
> -                    user_get_state(user) != USER_CLOSING)
> +                /* First step: queue stop jobs */
> +                if (!user_check_gc(user, drop_not_started))
>                          user_stop(user, false);
>  
> +                /* Second step: finalize user */
>                  if (!user_check_gc(user, drop_not_started)) {
>                          user_finalize(user);
>                          user_free(user);


Lennart

-- 
Lennart Poettering, Red Hat