so-called-hang-on-exit

[Jani Jaakkola]

On Mon, 5 Aug 2002, Frank Cusack wrote:

> 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.

The point of the select() timeout was to make sure, that sshd never gets 
stuck on select, if there never was no output from the background 
processes and pty buffer already was empty. 

> 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 suppose you are right; if select() says that there is nothing to read, 
then the EAGAIN test is not needed.

Anyway, I realized that it is probably useless to go through the select 
loop after the child is exited to just be able to empty the pty buffer. I 
checked, and Linux pty buffer size is just 2048 bytes, which is much less 
than 16*1024 buffer in channel_handle_rfd. And since the master processes 
have exited, the unread data is guaranteed to have fit into that 2048 byte 
buffer.

So here is a much much simpler patch achieving essentieally the same thing 
(this time it is no longer possible for sshd to hang-on-exit forever if 
pty background processes keep dumping data to pty):

The included patch is also available from:
http://www.cs.helsinki.fi/u/jjaakkol/openssh-hang-on-exit-v2.patch

- Jani


--- openssh-3.4p1.orig/session.c	Wed Jun 26 16:51:06 2002
+++ openssh-3.4p1/session.c	Tue Aug  6 01:10:20 2002
@@ -1809,6 +1809,32 @@
 	debug("session_exit_message: session %d channel %d pid %ld",
 	    s->self, s->chanid, (long)s->pid);
 
+	/* Check if we need the hang-on-exit bug workaround */
+	if (s->ttyfd != -1 && c->istate == CHAN_INPUT_OPEN) {
+		/* drain the pty buffer before sending the exit status.
+		 * this ensures, that all data from foreground processes
+		 * is sent before the exit status
+		 */
+		fd_set dummy_read, dummy_write;
+		FD_ZERO(&dummy_read);
+		FD_SET(s->ttyfd,&dummy_read);
+		FD_ZERO(&dummy_write);
+		/* We really want to call channel_handle_rfd, but it is not
+		 * exposed through channels.h. This will eventually achieve
+		 * same thing. */
+		/* The read buffer used in channel_handle_rfd is larger
+		 * than Linux pty-buffer size. So at least in Linux it
+		 * should be guaranteed that the pty buffer gets emptied.
+		 */
+		channel_after_select(&dummy_read,&dummy_write);
+		/* This is a policy decision which probably should be done
+		 * in the client side. Sometimes the client might be 
+		 * interested in the output of pty background processes. 
+		 * I leave this as an exercise to the reader.
+		verbose("hang-on-exit-workaround applied");
+		chan_read_failed(c);
+	}
+	
 	if (WIFEXITED(status)) {
 		channel_request_start(s->chanid, "exit-status", 0);
 		packet_put_int(WEXITSTATUS(status));