Issue #9356 has been updated by Shugo Maeda.


Eric Wong wrote:
>  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.

Hmm..., the problem I've seen seem not to be a platform-dependent issue, but a simple bug, because the current code goes in an infinite loop when the connection is established without errors.
The current code wouldn't work on any platform except Linux, where connect() can be restartable without errors when it's interrupted by signals, so control never reach wait_connectable().

If I don't misunderstand Kosaki-san's intention, the code might be able to be simplified by removing for(;;) and waiting only RB_WAITFD_OUT (but I don't know whether it works well with winsock...).

What do you think, Kosaki-san?

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

By "transparently" I meant that if TCPSocket.new raises an exception when it's not interrupted by a signal and fails, it should also raise an exception when it's interrupted by a signal and asynchronous connect() fails.

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

It depends on how complex error handling by getsockopt() is, and we have a slight difference of opinion here.
I'd like to hear others' thoughts.

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

Linux's connect() does return ERESTART when it's interrupted by a signal.
On Linux, connect() is restartable.  Please see the following page:

  http://www.madore.org/~david/computers/connect-intr.html

The above page describes connect() returns EINTR on Linux, but it seems to return ERESTART nowadays.
(And it describes connect() returns EADDRINUSE instead of EALREADY on FreeBSD, but it returns EALREADY now.)


----------------------------------------
Bug #9356: TCPSocket.new does not seem to handle INTR
https://bugs.ruby-lang.org/issues/9356#change-45298

* Author: Charlie Somerville
* Status: Open
* Priority: Normal
* Assignee: 
* Category: 
* Target version: 
* ruby -v: -
* Backport: 1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN
----------------------------------------
TCPSocket.new does not seem to handle EINTR properly.

In the attached test script, I try to open a TCP connection to my server and make an HTTP request while a background thread continually sends a signal to the process.

This causes the #write call to fail with:

x.rb:13:in `write': Socket is not connected (Errno::ENOTCONN)
	from x.rb:13:in `<main>'

This also appears to affect 2.0.0. 1.9.3 is unaffected.

---Files--------------------------------
socket-eintr.rb (207 Bytes)


-- 
http://bugs.ruby-lang.org/