so-called-hang-on-exit

[Frank Cusack]

On Sat, Aug 03, 2002 at 10:02:27PM +0300, Jani Jaakkola wrote:
> On Sat, 3 Aug 2002, Jani Jaakkola wrote:
> 
> > My suggestion is that instead of just closing the channel when the 
> > session process dies, sshd should effort to empty the pty buffer before 
> > closing the channel. This could be implemented so that 

Great idea!

> > when the session process dies, sshd should read (and buffer) the pty 
> > master data until it gets EAGAIN and then close the channel. All 
> > pending output from dead session processes should allready be in the pty 
> > buffer, so first EAGAIN means that all of it has been read processed.
> > 
> > I guess I will attempt to implement this, but it will take some time (I 
> > will need to learn how session.c and serverloop.c work first).
> 
> OK, now I have implemented it. Currently only for protocol 2, but a 
> similar thing can be implemented for protocol 1 (and the protocol 1 patch 
> would be simpler).

I've implemented something similar for protocol 1, and implemented a
slightly simpler version of your patch for protocol 2.  Before I post it,
I'm curious as to what the intent is of being able to specify the
select() timeout.

Also, I'm not sure why you test for EAGAIN instead of just checking
if the bit is set in the readset.  My patch just does an FD_ISSET(...)
on the appropriate bit (c->rfd) iff it was set going into select()
(ie, if the remote_window wasn't already full).

I think the two (yours and mine) ar equivalent except that mine doesn't
allocate more memory for the output buffer, at the expense of having to
test for the fd being set going into the select() call.  That expense
could be alleviated simply by saving the input set, but that means
adding a global (not necessarily bad) or adding an arg to a bunch of
the channel_*() calls.

The reasons I did it that way instead of forcing a read() and testing for
EAGAIN were

- my test for protocol 1 was simpler to do that way
- I figured it's cleaner to have the protocol 2 code do the same
- I figured the code purposely doesn't allocate more memory for the output
  buffer, so I didn't want to second-guess that (even though the patch
  code only runs at the end of the session; but still, something like
  'ssh host "yes & exit"' will generate lots of data)

/fc