From: Eric Wong <normalperson@...>
Date: 2014-02-19T20:38:24+00:00
Subject: [ruby-core:60876] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR

shugo@ruby-lang.org wrote:
> Eric Wong wrote:
> >  > Ah, I see.  However, if getsockopt() returns no error, we can know
> >  > that at least connect() succeeded, right?  I'm not sure whether it's a
> >  > so-called race condition.
> >  
> >  I'm not sure if we can know for sure due to implementation differences.
> 
> Hmm..., do you know any implementation where getsockopt(SO_ERROR) causes a problem?
> If there's such an implementation, it would be better to remove the call, but I don't come up with such a situation.

I'm not sure about implementations of getsockopt(SO_ERROR),
there are too many differences from what I see.

However, our use of getsockopt(SO_ERROR) causes problems
for maintainability and seems to lead to bugs.

> By contraries, getsockopt(SO_ERROR) might be necessary for winsock, if usa's comment is right.

I'm not sure...
usa: can you try my last patch?
  http://bogomips.org/ruby.git/patch?id=f5e2eb00e5

> >  And even if connect() succeeded, the server could decide to disconnect
> >  right away after accept() due to overload, so probably less important.
> 
> It applies equally to connect() without signal interruption.
> TCPSocket.new should handle EINTR as transparently as possible, I think.

Agreed, and my f5e2eb00e5 patch handles EINTR.

> >  > >  anyways, getsockopt(SO_ERROR) is worthless.
> >  
> >  > I don't think getsockopt(SO_ERROR) is worthless because users can know
> >  > error information sooner and more exactly than subsequent read() or
> >  > write().
> >  
> >  The error may be seen sooner, but I think the errors are uncommon
> >  and most users will not care.  They will see any error and handle it
> >  their own way.
> 
> Most applications might handle errors roughly, but logging the exact error information would help trouble shooting.

Yes, and connect() still shows ETIMEDOUT/ECONNRESET/EHOSTUNREACH/etc.
Interrupted connect() is rare, so I think having the extra
hard-to-maintain code causes more problems than it solves.
I don't think users will care which of connect/write/read fails,
they just need to know an error happened.

> > I have no idea how portable this is:
> > http://bogomips.org/ruby.git/patch?id=f5e2eb00e5
> > 
> > Btw, I suspect the WAIT_IN_PROGRESS stuff is carried over from the
> > 1.8 days where all sockets were non-blocking by default, and overly
> > complicated as a result. I don't even think EINPROGRESS/EAGAIN is
> > possible, only EINTR/ERESTART.
> 
> The patch worked both on Linux 3.2.0 and FreeBSD 10.0 for blocking sockets with signal interruption.

Thanks for testing.  I expect it to have no problem on most *BSD.
I mainly wanted some Win/Solaris/Apple users to try it.

> It might be better to re-call connect() on ERESTART as the error suggests, but I'm not sure.
> It seems to be a Linux way to re-call connect(), but there's no description about ERESTART in Linux's manual of connect(2) and instead there's a reference to POSIX.1-2001 (SUSv3), to which Linux seem not to conform.  Funny.

I'm not sure, either.  ERESTART should be handled by the system C
library and not seen by us, even.  Not sure which (or any currently
supported) systems leak ERESTART...