[Bug 52] ssh hangs on exit

[bugzilla-daemon at mindrot.org]

http://bugzilla.mindrot.org/show_bug.cgi?id=52





------- Comment #32 from tsi at ualberta.ca  2006-11-29 04:41 -------
(In reply to comment #31)
> (From update of attachment 1098 [details])
> The patch has not been applied, I simply pointed someone toward it.

> >-	    FD_ISSET(c->rfd, readset)) {
> >+	    (c->detach_close || FD_ISSET(c->rfd, readset))) {
> > 		len = read(c->rfd, buf, sizeof(buf));
> >-		if (len < 0 && (errno == EINTR || errno == EAGAIN))
> >+		if (len < 0 && errno == EINTR)

> I'm wondering if this could ever cause the channel to be shutdown if
> the read fails with EAGAIN during normal operation.

> POSIX says EAGAIN should be returned when the descriptor is set for
> nonblocking IO and select should not mark the fd in that case so this
> shouldn't happen.

> On the other hand, comment #9 has one counterexample,

How so?

> plus Google finds
> http://archives.neohapsis.com/archives/postfix/2003-09/0507.html, which
> indicates that select can fire on on a non-ready fd (probably not an
> issue here since it appears to affect only TCP sockets) but it makes me
> wonder what other instances are out there...

Well, you could accept EAGAIN only before detach_close is set.  In the
!compat20 case, child_terminated.

> >+			program_alive_scheduled = child_terminated;

> Any reason this is a separate variable rather than just using
> child_terminated?  A possible race where a SIGCHLD arrives after the
> read?

It's been a while, but IIRC, I did this to ensure I was looking at the
same child_terminated value across the select(2).  I wanted to force
read(2)'s only if I also forced select(2) to check the fd(s).

Even so, I wish to emphasise here that it is not possible to miss the
child's death:

In the compat20 case, detach_close ends up taking over from
child_terminated.  I.e. once set, detach_close is permanent, but
collect_children() resets child_terminated.

In the !compat20 case, child_terminated isn't reset.

> On a related note, I'm also wondering if we be aiming to carry this as
> a portable-only change (since OpenBSD isn't directly affected and we
> already have to deal with some pty wackiness from AIX in this area of
> code, so patches won't apply cleanly already) or should we be aiming to
> push the change to OpenBSD too?

That's a decision only you can make.  My personal view is that the
portable version should also run on OpenBSD.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.