On Thu, 13.02.14 18:31, Djalal Harouni ([email protected]) 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 _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
