Improve behaviour when flushing out after a process has exited.

--HG--
branch : channel-fix
extra : convert_revision : e73ee8f7ae404a9355685c30828a0ad4524031bc

5 files changed, 84 insertions, 13 deletions

diff --git a/channel.h b/channel.h
index fc3cb90..1bfe392 100644
--- a/channel.h
+++ b/channel.h
@@ -84,6 +84,8 @@ struct Channel {
 					   for this channel (and are awaiting a confirmation
 					   or failure). */
 
+	int flushing;
+
 	const struct ChanType* type;
 
 };
diff --git a/common-channel.c b/common-channel.c
index 19b807f..d87ea7d 100644
--- a/common-channel.c
+++ b/common-channel.c
@@ -148,6 +148,7 @@ struct Channel* newchannel(unsigned int remotechan,
 	newchan->errfd = FD_CLOSED; /* this isn't always set to start with */
 	newchan->initconn = 0;
 	newchan->await_open = 0;
+	newchan->flushing = 0;
 
 	newchan->writebuf = cbuf_new(RECV_MAXWINDOW);
 	newchan->extrabuf = NULL; /* The user code can set it up */
@@ -203,12 +204,14 @@ void channelio(fd_set *readfds, fd_set *writefds) {
 
 		/* read data and send it over the wire */
 		if (channel->readfd >= 0 && FD_ISSET(channel->readfd, readfds)) {
+			TRACE(("send normal readfd"))
 			send_msg_channel_data(channel, 0);
 		}
 
 		/* read stderr data and send it over the wire */
-		if (ERRFD_IS_READ(channel) &&
-				channel->errfd >= 0 && FD_ISSET(channel->errfd, readfds)) {
+		if (ERRFD_IS_READ(channel) && channel->errfd >= 0 
+			&& FD_ISSET(channel->errfd, readfds)) {
+				TRACE(("send normal errfd"))
 				send_msg_channel_data(channel, 1);
 		}
 
@@ -265,8 +268,14 @@ static void check_close(struct Channel *channel) {
 				cbuf_getused(channel->writebuf),
 				channel->extrabuf ? cbuf_getused(channel->extrabuf) : 0))
 
+	if (!channel->flushing && channel->type->check_close
+		&& channel->type->check_close(channel))
+	{
+		channel->flushing = 1;
+	}
+
 	if (channel->recv_close && !write_pending(channel)) {
-		if (! channel->sent_close) {
+		if (!channel->sent_close) {
 			TRACE(("Sending MSG_CHANNEL_CLOSE in response to same."))
 			send_msg_channel_close(channel);
 		}
@@ -278,6 +287,22 @@ static void check_close(struct Channel *channel) {
 		close_chan_fd(channel, channel->writefd, SHUT_WR);
 	}
 
+	/* Special handling for flushing read data after an exit. We
+	   read regardless of whether the select FD was set,
+	   and if there isn't data available, the channel will get closed. */
+	if (channel->flushing) {
+		TRACE(("might send data, flushing"))
+		if (channel->readfd >= 0 && channel->transwindow > 0) {
+			TRACE(("send data readfd"))
+			send_msg_channel_data(channel, 0);
+		}
+		if (ERRFD_IS_READ(channel) && channel->readfd >= 0 
+			&& channel->transwindow > 0) {
+			TRACE(("send data errfd"))
+			send_msg_channel_data(channel, 1);
+		}
+	}
+
 	/* If we're not going to send any more data, send EOF */
 	if (!channel->sent_eof
 			&& channel->readfd == FD_CLOSED 
@@ -287,15 +312,13 @@ static void check_close(struct Channel *channel) {
 
 	/* And if we can't receive any more data from them either, close up */
 	if (!channel->sent_close
-			&& channel->writefd == FD_CLOSED
 			&& channel->readfd == FD_CLOSED
-			&& channel->errfd == FD_CLOSED) {
+			&& !write_pending(channel)) {
+		TRACE(("sending close, readfd is closed"))
 		send_msg_channel_close(channel);
 	}
-
 }
 
-
 /* Check whether a deferred (EINPROGRESS) connect() was successful, and
  * if so, set up the channel properly. Otherwise, the channel is cleaned up, so
  * it is important that the channel reference isn't used after a call to this
@@ -341,6 +364,9 @@ static void send_msg_channel_close(struct Channel *channel) {
 
 	channel->sent_eof = 1;
 	channel->sent_close = 1;
+	close_chan_fd(channel, channel->readfd, SHUT_RD);
+	close_chan_fd(channel, channel->errfd, SHUT_RDWR);
+	close_chan_fd(channel, channel->writefd, SHUT_WR);
 	TRACE(("leave send_msg_channel_close"))
 }
 
@@ -556,6 +582,7 @@ static void send_msg_channel_data(struct Channel *channel, int isextended) {
 
 	CHECKCLEARTOWRITE();
 
+	TRACE(("enter send_msg_channel_data"))
 	dropbear_assert(!channel->sent_close);
 
 	if (isextended) {
@@ -591,6 +618,9 @@ static void send_msg_channel_data(struct Channel *channel, int isextended) {
 	len = read(fd, buf_getwriteptr(ses.writepayload, maxlen), maxlen);
 	if (len <= 0) {
 		if (len == 0 || errno != EINTR) {
+			/* This will also get hit in the case of EAGAIN. The only
+			time we expect to receive EAGAIN is when we're flushing a FD,
+			in which case it can be treated the same as EOF */
 			close_chan_fd(channel, fd, SHUT_RD);
 		}
 		ses.writepayload->len = ses.writepayload->pos = 0;
@@ -606,6 +636,14 @@ static void send_msg_channel_data(struct Channel *channel, int isextended) {
 	channel->transwindow -= len;
 
 	encrypt_packet();
+	
+	/* If we receive less data than we requested when flushing, we've
+	   reached the equivalent of EOF */
+	if (channel->flushing && len < maxlen)
+	{
+		TRACE(("closing from channel, flushing out."))
+		close_chan_fd(channel, fd, SHUT_RD);
+	}
 	TRACE(("leave send_msg_channel_data"))
 }
 
diff --git a/common-session.c b/common-session.c
index a16d1f9..6e1abf3 100644
--- a/common-session.c
+++ b/common-session.c
@@ -61,6 +61,12 @@ void common_session_init(int sock, char* remotehost) {
 
 	ses.connecttimeout = 0;
 	
+	if (pipe(ses.signal_pipe) < 0) {
+		dropbear_exit("signal pipe failed");
+	}
+	setnonblocking(ses.signal_pipe[0]);
+	setnonblocking(ses.signal_pipe[1]);
+	
 	kexfirstinitialise(); /* initialise the kex state */
 
 	ses.writepayload = buf_new(MAX_TRANS_PAYLOAD_LEN);
@@ -108,7 +114,6 @@ void common_session_init(int sock, char* remotehost) {
 
 	ses.allowprivport = 0;
 
-
 	TRACE(("leave session_init"))
 }
 
@@ -132,6 +137,10 @@ void session_loop(void(*loophandler)()) {
 				FD_SET(ses.sock, &writefd);
 			}
 		}
+		
+		/* We get woken up when signal handlers write to this pipe.
+		   SIGCHLD in svr-chansession is the only one currently. */
+		FD_SET(ses.signal_pipe[0], &readfd);
 
 		/* set up for channels which require reading/writing */
 		if (ses.dataallowed) {
@@ -155,6 +164,14 @@ void session_loop(void(*loophandler)()) {
 			FD_ZERO(&writefd);
 			FD_ZERO(&readfd);
 		}
+		
+		/* We'll just empty out the pipe if required. We don't do
+		any thing with the data, since the pipe's purpose is purely to
+		wake up the select() above. */
+		if (FD_ISSET(ses.signal_pipe[0], &readfd)) {
+			char x;
+			while (read(ses.signal_pipe[0], &x, 1) > 0) {}
+		}
 
 		/* check for auth timeout, rekeying required etc */
 		checktimeouts();
diff --git a/session.h b/session.h
index a4ad45f..78e3e57 100644
--- a/session.h
+++ b/session.h
@@ -123,7 +123,8 @@ struct sshsession {
 
 	unsigned char lastpacket; /* What the last received packet type was */
 	
-
+    int signal_pipe[2]; /* stores endpoints of a self-pipe used for
+						   race-free signal handling */
 
 	/* KEX/encryption related */
 	struct KEXState kexstate;
diff --git a/svr-chansession.c b/svr-chansession.c
index a45e7a6..23cac85 100644
--- a/svr-chansession.c
+++ b/svr-chansession.c
@@ -120,9 +120,21 @@ static void sesssigchild_handler(int UNUSED(dummy)) {
 			/* we use this to determine how pid exited */
 			exit->exitsignal = -1;
 		}
+		
+		/* Make sure that the main select() loop wakes up */
+		while (1) {
+			/* EAGAIN means the pipe's full, so don't need to write anything */
+			/* isserver is just a random byte to write */
+			if (write(ses.signal_pipe[1], &ses.isserver, 1) == 1 || errno == EAGAIN) {
+				break;
+			}
+			if (errno == EINTR) {
+				continue;
+			}
+			dropbear_exit("error writing signal pipe");
+		}
 	}
 
-	
 	sa_chld.sa_handler = sesssigchild_handler;
 	sa_chld.sa_flags = SA_NOCLDSTOP;
 	sigaction(SIGCHLD, &sa_chld, NULL);
@@ -244,16 +256,17 @@ static void closechansess(struct Channel *channel) {
 	unsigned int i;
 	struct logininfo *li;
 
-	chansess = (struct ChanSess*)channel->typedata;
+	TRACE(("enter closechansess"))
 
-	send_exitsignalstatus(channel);
+	chansess = (struct ChanSess*)channel->typedata;
 
-	TRACE(("enter closechansess"))
 	if (chansess == NULL) {
 		TRACE(("leave closechansess: chansess == NULL"))
 		return;
 	}
 
+	send_exitsignalstatus(channel);
+
 	m_free(chansess->cmd);
 	m_free(chansess->term);