session.c Was: Re: OpenSSH_4.3p1 configure patch

[Tim Rice]

On Tue, 7 Feb 2006, Corinna Vinschen wrote:

> On Feb  7 01:06, Darren Tucker wrote:
> > On Mon, Feb 06, 2006 at 02:42:55PM +0100, Corinna Vinschen wrote:
> > > I just tested the below patch and it solves the problem for me.
> > > Since session_exit_message is only called from session_close_by_pid,
> > > the solution seems to be the correct one.
> > > 
> > > 
> > > --- session.c.ORIG	2006-02-06 13:50:21.788927500 +0100
> > > +++ session.c	2006-02-06 13:45:27.042081500 +0100
> > > @@ -2176,7 +2176,6 @@ session_exit_message(Session *s, int sta
> > >  
> > >  	/* disconnect channel */
> > >  	debug("session_exit_message: release channel %d", s->chanid);
> > > -	s->pid = 0;
> > >  
> > >  	/*
> > >  	 * Adjust cleanup callback attachment to send close messages when
> > > @@ -2238,6 +2237,7 @@ session_close_by_pid(pid_t pid, int stat
> > >  		session_exit_message(s, status);
> > >  	if (s->ttyfd != -1)
> > >  		session_pty_cleanup(s);
> > > +	s->pid = 0;
> > >  }
> > 
> > FWIW the s->pid bit was added in this change:
> > 
> > revision 1.308
> > date: 2005/11/05 03:52:51;  author: djm;  state: Exp;  lines: +23 -14
> >    - djm at cvs.openbsd.org 2005/10/10 10:23:08
> >      [channels.c channels.h clientloop.c serverloop.c session.c]
> >      fix regression I introduced in 4.2: X11 forwardings initiated after
> >      a session has exited (e.g. "(sleep 5; xterm) &") would not start.
> >      bz #1086 reported by t8m AT centrum.cz; ok markus@ dtucker@
> > 
> > Not sure what (if any) effect the diff would have on the case above.
> 
> I'm not *quite* sure if I analyzed that correctly, but AFAICS, it should
> have no effect on the situation solved by the patch from 2005/11/05:
> 
> - session_exit_message is a static function which is exclusively called
>   by session_close_by_pid.
> 
> - In session_exit_message, following the setting of s->pid to 0 are only
>   two calls, channel_register_cleanup and chan_write_failed.
> 
> - channel_register_cleanup only marks session_close_by_channel to be run
>   for the channel on cleanup, but it does not run this function immediately,
>   nor does it itself depend on the setting of s->pid.
> 
> - chan_write_failed does also not depend on the setting of s->pid.
> 
> - On return from session_exit_message to session_close_by_pid,
>   session_pty_cleanup is called with correct s->pid setting.
> 
> - s->pid is set to 0.
> 
> - In a later cleanup, session_close_by_channel is called and s->pid has
>   the expected value of 0.
> 
> Did I miss something?
> 

It doesn't look like it to me. (and tests good here)
I've commited your patch.

> 
> Corinna
> 
> 

-- 
Tim Rice				Multitalents	(707) 887-1469
tim at multitalents.net