dbclient using 100% CPU

[Matt Johnston]

On Tue, Oct 04, 2005 at 01:34:36PM +0100, Jamie Lokier wrote:
> I'm using dbclient on an ARM + uClinux device, and I see that it's
> using 100% CPU although it has nothing to do.
> 
> The command looks like this:
> 
>     dbclient -T -i /root/.ssh/id_rsa.db 192.168.0.100 -R 10136:127.0.0.1:23 'while :; do echo keepalive; sleep 20; done'
> 
> The standard input and standard output are both /dev/null.  ( If
> standard output is not /dev/null, I don't see 100% CPU, but the same
> "select for read on standard output" behaviour described below occurs. )
> 
> The output of strace explains the 100% CPU:
> 
>     select(4, [1 3], [], NULL, {20, 0})     = 1 (in [1], left {20, 0})
>     gettimeofday({1105499730, 917033}, NULL) = 0
>     write(1, NULL, 0)                       = 0
>     select(4, [1 3], [], NULL, {20, 0})     = 1 (in [1], left {20, 0})
>     gettimeofday({1105499731, 117149}, NULL) = 0
>     write(1, NULL, 0)                       = 0
>     ... repeating ...
> 
> Why is it selecting for _read_ on the standard _output_ file
> descriptor?  That looks like a bug to me.

The file descriptors are being treated correctly (or rather
as intended), they're just badly named (when I initially
wrote the code and Dropbear was server-only, I was thinking
in terms of infd being the shell program's stdin etc). I've
been meaning to clean them up to avoid confusion.

The actual problem seems to be in the section of code trying
to detect the pipe being closed, by reading 0 bytes from the
writefd. I don't think that code is required any more,
removing it fixes the CPU spinning. I've attached a patch,
could you try that and see how it goes (and also if you see
any side-effects)?

I'll have a closer look at what's going on and test a few
more platforms before committing that change.

Thanks,
Matt
-------------- next part --------------
# 
# old_revision [8b52f8af5e976e88a542827b554b489a557464da]
# 
# patch "common-channel.c"
#  from [42c4b46eac09d7abf45f066a008e5d2cafbb0127]
#    to [ed7183d857c5dbb3ace56a2cf3b7a6cbf7a9bdd0]
# 
============================================================
--- common-channel.c	42c4b46eac09d7abf45f066a008e5d2cafbb0127
+++ common-channel.c	ed7183d857c5dbb3ace56a2cf3b7a6cbf7a9bdd0
@@ -203,24 +203,6 @@
 				send_msg_channel_data(channel, 1, SSH_EXTENDED_DATA_STDERR);
 		}
 
-		/* if we can read from the infd, it might be closed, so we try to
-		 * see if it has errors */
-		if (channel->infd >= 0 && channel->infd != channel->outfd
-				&& FD_ISSET(channel->infd, readfd)) {
-			if (channel->initconn) {
-				/* Handling for "in progress" connection - this is needed
-				 * to avoid spinning 100% CPU when we connect to a server
-				 * which doesn't send anything (tcpfwding) */
-				checkinitdone(channel);
-				continue; /* Important not to use the channel after 
-							 checkinitdone(), as it may be NULL */
-			}
-			ret = write(channel->infd, NULL, 0); /* Fake write */
-			if (ret < 0 && errno != EINTR && errno != EAGAIN) {
-				closeinfd(channel);
-			}
-		}
-
 		/* write to program/pipe stdin */
 		if (channel->infd >= 0 && FD_ISSET(channel->infd, writefd)) {
 			if (channel->initconn) {
@@ -445,16 +427,6 @@
 			}
 		}
 
-		/* For checking FD status (ie closure etc) - we don't actually
-		 * read data from infd */
-		TRACE(("infd = %d, outfd %d, errfd %d, bufused %d", 
-					channel->infd, channel->outfd,
-					channel->errfd,
-					cbuf_getused(channel->writebuf) ))
-		if (channel->infd >= 0 && channel->infd != channel->outfd) {
-			FD_SET(channel->infd, readfd);
-		}
-
 		/* Stuff from the wire, to local program/shell/user etc */
 		if ((channel->infd >= 0 && cbuf_getused(channel->writebuf) > 0 )
 				|| channel->initconn) {