[systemd-devel] [PATCH] logind: use session_get_state() to get sessions state of the user

[Djalal Harouni]

On Sun, Feb 09, 2014 at 01:48:04AM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> On Sat, Feb 08, 2014 at 10:20:53PM +0100, Djalal Harouni wrote:
> > On Sat, Feb 08, 2014 at 10:01:18PM +0100, Zbigniew Jędrzejewski-Szmek wrote:
> > > On Sat, Feb 08, 2014 at 08:51:57PM +0100, Djalal Harouni wrote:
> > > > In function user_get_state() remove the session_is_active() check, just
> > > > count on the session_get_state() function to get the correct session
> > > > state.
> > > > 
> > > > session_is_active() may return true before starting the session scope
> > > > and user service, this means it will return true even before the creation
> > > > of the session fifo_fd which will produce incorrect states.
> > > > 
> > > > So be consistent and just use session_get_state().
> > > Sooo... with your patch applied, I see: 
> > > 
> > > sshd[18756]: pam_unix(sshd:session): session closed for user user2
> > > systemd-logind[18687]: Sent message type=method_call sender=n/a destination=org.freedesktop.systemd1 object=/org/freedesktop/systemd1/unit/session_2d10_2escope interface=org.freedesktop.systemd1.Scope member=Abandon cookie=27 reply_cookie=0 error=n/a
> > > 
> > > And nothing afterwards. User manager for user2 is undisturbed.
> > Ah this patch fixes the user state.
> > 
> > The one you should apply is from the other thread:
> > http://lists.freedesktop.org/archives/systemd-devel/2014-February/016754.html
> > 
> > It should work, please give it a try!
> You're right, I applied the wrong patch. It really seems that with your
> * logind: use session_get_state() to get sessions state of the user
> * logind: just call user_stop() if user_check_gc() returns false
> things actually work.
Ok nice, thanks :-)

> One thing that still does not work is terminate-user, even though
> kill-session works. 
> 
> method_terminate_user -> user_stop -*> session_stop -> session_stop_scope -> manager_abandom_scope
>                                    -> user_stop_service
>                                    -> user_stop_slice
> 
> manager_stop_scope calls manager_shall_kill which returns false, so it
> only calls manager_abandon_scope. There seems to be a conflation between
> explicit termination of users and sessions, and automatic termination when
> they log out. According to the man page:
> 
>    KillUserProcesses=
>        Takes a boolean argument. Configures whether the processes
>        of a user should be killed when she or he completely logs
>        out (i.e. after her/his last session ended). Defaults to no.
> 
> KillUserProcesses should not apply to explicit termination. I think
> session_stop_scope should be changed to unconditionally kill the
> session when called from method_terminate_user.
Yes, I do agree. I did also have this one on my todo list, I saw you
posted the patch on the other thread, I'll test it this night or
probably tomorrow.

I still have 2 untested patches, that apply on top of Lennart changes
to close two small race windows...

Thanks!

> Zbyszek

-- 
Djalal Harouni
http://opendz.org