[gdm-list] Re: PostSession script support fades out from 2.4 to 2.6?

[George <jirka 5z com>]

On Fri, Aug 19, 2005 at 05:10:01PM -0500, Brian Cameron wrote:
> 
> Jerry/George:
> 
> Thanks for looking into this and for proposing a solution to the problem.
> It seems that this problem was introduced when George fixed bug
> #126071 by adding the call to whack clients in session stop, causing a
> race condition.  Bug #126071 is regarding an issue that GDM was not always
> unmounting the user's home directory because programs like xscreensaver
> retain open file descripters within $HOME when GDM calls pam_close_session.
> The bug reporter proposed fixing the problem by moving the call to
> pam_close_session until after the Xserver is reset so all user programs
> will have closed any open file descriptors by the time pam_close_session
> is called.
> 
> Doesn't it seem better to backout the fix to bug 126071 and fix the problem
> more simply by moving the pam_close_session call rather than having this
> complicated race condition?  Furthermore, George highlighted that this
> could/should perhaps better be fixed in the PAM module code by doing
> an async unmount.  It may be better to fix this in PAM rather than hacking
> up GDM with nasty logic to deal with race conditions.  The person who
> submitted bug 126071 suggested a patch that looks like a much cleaner
> solution to the problem.  I'm not sure why George rejected it.  Perhaps
> he might remember and could tell us.  I'd appreciate some other eyes
> looking at the suggested patch and give feedback if doing something like
> this would be a better way to fix 126071.

As far as I remember and as far as I'm reading the patch now, I suppose the
reason for rejecting it is that it's not functionally equivalent, that is, I
beleive that pam might not get correctly cleaned up in some circumstances
(but this is only from memory I don't know).  And it changes the sequence in
which things are done.

Yes the correct fix would be to fix the pam module that does this.  In any
cse, the race that this adds is not really one that should cause the error
that we're seeing here I don't think.  The race is that even though we
XKillClient all the clients, we don't get notification that the clients
really closed.

Anyway I suppose I understand why the error is happening since at the time
that we try to kill clients the X server doesn't exist anymore.  But I don't
get why the display variable was not NULLed so that the client killing code
would just be a noop in this case.

> Jerry, could you check to see if you backout George's fix for bug
> #126071 and see if that also fixes the problem.  Not running PostSession
> on logout seems much worse than sometimes not unmounting home directories.

As the bug says, it's not a fix, but a "fix". :)  Still it seems to me that
there is a simple fix to this "fix" somewhere where we need to NULL the
display variable.  I don't have the gdm code here to look at to really try to
find the error.

However it seems sane to revert the "fix" to avoid any X errors that might
occur there.  The less X traffic we need to do from daemon the better 

George

-- 
George <jirka 5z com>
   Supreme executive power derives from a mandate from the masses,
   not from some farcical aquatic ceremony.
                       -- Dennis (the bloody peasant)