ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR
@ 2014-01-03 10:29 charliesome (Charlie Somerville)
  2014-01-03 17:53 ` [ruby-core:59536] " Eric Wong
                   ` (31 more replies)
  0 siblings, 32 replies; 48+ messages in thread
From: charliesome (Charlie Somerville) @ 2014-01-03 10:29 UTC (permalink / raw
  To: ruby-core


Issue #9356 has been reported by charliesome (Charlie Somerville).

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

Author: charliesome (Charlie Somerville)
Status: Open
Priority: Normal
Assignee: 
Category: 
Target version: 
ruby -v: ruby 2.1.0p0 (2013-12-25 revision 44422) [x86_64-darwin13.0]
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.


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:59536] Re: [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
@ 2014-01-03 17:53 ` Eric Wong
  2014-01-04  5:05 ` [ruby-core:59546] [ruby-trunk - Bug #9356] " charliesome (Charlie Somerville)
                   ` (30 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-01-03 17:53 UTC (permalink / raw
  To: Ruby developers

This might be a bug exposed due to r36944
("avoid unnecessary select() calls before doing I/O")
which I don't want to revert.

Does the following fix it?

--- a/ext/socket/init.c
+++ b/ext/socket/init.c
@@ -400,12 +400,6 @@ rsock_connect(int fd, const struct sockaddr *sockaddr, int len, int socks)
 	status = (int)BLOCKING_REGION_FD(func, &arg);
 	if (status < 0) {
 	    switch (errno) {
-	      case EINTR:
-#if defined(ERESTART)
-	      case ERESTART:
-#endif
-		continue;
-
 	      case EAGAIN:
 #ifdef EINPROGRESS
 	      case EINPROGRESS:
@@ -426,6 +420,12 @@ rsock_connect(int fd, const struct sockaddr *sockaddr, int len, int socks)
 #if WAIT_IN_PROGRESS > 0
 		wait_in_progress = WAIT_IN_PROGRESS;
 #endif
+	      case EINTR:
+#if defined(ERESTART)
+	      case ERESTART:
+#endif
+		continue;
+
 		status = wait_connectable(fd);
 		if (status) {
 		    break;
-----------------------------------------------------------------------
Fwiw, I'm not sure if the WAIT_IN_PROGRESS + getsockopt SO_ERROR is
even necessary when we can just do wait_connectable(fd) followed
by an eventual write/send/read/recv...

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:59546] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
  2014-01-03 17:53 ` [ruby-core:59536] " Eric Wong
@ 2014-01-04  5:05 ` charliesome (Charlie Somerville)
  2014-01-04  7:03   ` [ruby-core:59548] " Eric Wong
  2014-02-08  2:40 ` [ruby-core:60571] " normalperson
                   ` (29 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: charliesome (Charlie Somerville) @ 2014-01-04  5:05 UTC (permalink / raw
  To: ruby-core


Issue #9356 has been updated by charliesome (Charlie Somerville).


Eric: Unfortunately your patch doesn't fix it, still getting the same ENOTCONN error.
----------------------------------------
Bug #9356: TCPSocket.new does not seem to handle INTR
https://bugs.ruby-lang.org/issues/9356#change-44083

Author: charliesome (Charlie Somerville)
Status: Open
Priority: Normal
Assignee: 
Category: 
Target version: 
ruby -v: ruby 2.1.0p0 (2013-12-25 revision 44422) [x86_64-darwin13.0]
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.


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:59548] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-04  5:05 ` [ruby-core:59546] [ruby-trunk - Bug #9356] " charliesome (Charlie Somerville)
@ 2014-01-04  7:03   ` Eric Wong
  2014-02-08  2:35     ` [ruby-core:60570] " Eric Wong
  0 siblings, 1 reply; 48+ messages in thread
From: Eric Wong @ 2014-01-04  7:03 UTC (permalink / raw
  To: Ruby developers

Thanks for trying.  This is probably specific to *BSD sockets
implementation, so I can't reproduce it at the moment.

Can you try reverting parts of r36944 which affect IO#write?
("avoid unnecessary select() calls before doing I/O")

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60570] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-04  7:03   ` [ruby-core:59548] " Eric Wong
@ 2014-02-08  2:35     ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-02-08  2:35 UTC (permalink / raw
  To: Ruby developers

Eric Wong <normalperson@yhbt.net> wrote:
> Thanks for trying.  This is probably specific to *BSD sockets
> implementation, so I can't reproduce it at the moment.

Not happening on FreeBSD 9.x kernels, at least.  I could not reproduce
the problem on FreeBSD 9.2 nor Debian GNU/kFreeBSD sid (x86_64).

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60571] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
  2014-01-03 17:53 ` [ruby-core:59536] " Eric Wong
  2014-01-04  5:05 ` [ruby-core:59546] [ruby-trunk - Bug #9356] " charliesome (Charlie Somerville)
@ 2014-02-08  2:40 ` normalperson
  2014-02-18  9:34 ` [ruby-core:60819] " shugo
                   ` (28 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-08  2:40 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 Eric Wong <normalperson@yhbt.net> wrote:
 > Thanks for trying.  This is probably specific to *BSD sockets
 > implementation, so I can't reproduce it at the moment.
 
 Not happening on FreeBSD 9.x kernels, at least.  I could not reproduce
 the problem on FreeBSD 9.2 nor Debian GNU/kFreeBSD sid (x86_64).

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

* Author: Charlie Somerville
* Status: Open
* Priority: Normal
* Assignee: 
* Category: 
* Target version: 
* ruby -v: ruby 2.1.0p0 (2013-12-25 revision 44422) [x86_64-darwin13.0]
* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60819] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (2 preceding siblings ...)
  2014-02-08  2:40 ` [ruby-core:60571] " normalperson
@ 2014-02-18  9:34 ` shugo
  2014-02-18  9:49   ` [ruby-core:60820] " U.Nakamura
  2014-02-18  9:58 ` [ruby-core:60821] " usa
                   ` (27 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: shugo @ 2014-02-18  9:34 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Shugo Maeda.


Eric Wong wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
>  > Thanks for trying.  This is probably specific to *BSD sockets
>  > implementation, so I can't reproduce it at the moment.
>  
>  Not happening on FreeBSD 9.x kernels, at least.  I could not reproduce
>  the problem on FreeBSD 9.2 nor Debian GNU/kFreeBSD sid (x86_64).

On FreeBSD 10, Errno::ENOTCONN isn't raised, but Ruby goes in an infinite loop because getsockopt(2) in wait_connectable() sets sockerr to 0.

Does anybody know why the following code in ext/socket/init.c is necessary?

            if (sockerr == 0)
                continue;       /* workaround for winsock */


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

* Author: Charlie Somerville
* Status: Open
* Priority: Normal
* Assignee: 
* Category: 
* Target version: 
* ruby -v: ruby 2.1.0p0 (2013-12-25 revision 44422) [x86_64-darwin13.0]
* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60820] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-18  9:34 ` [ruby-core:60819] " shugo
@ 2014-02-18  9:49   ` U.Nakamura
  2014-02-18 10:36     ` [ruby-core:60822] " Eric Wong
  0 siblings, 1 reply; 48+ messages in thread
From: U.Nakamura @ 2014-02-18  9:49 UTC (permalink / raw
  To: Ruby developers

Hi,

In message "[ruby-core:60819] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR"
    on Feb.18,2014 18:34:40, <shugo@ruby-lang.org> wrote:
> Does anybody know why the following code in ext/socket/init.c is necessary?
> 
>             if (sockerr == 0)
>                 continue;       /* workaround for winsock */

It was introduced at r7931 by me (9 years ago!),
but I forgot the reason.
If my comment of those days is believed, we may wrap it
with #ifdef_WIN32.


Regards,
-- 
U.Nakamura <usa@garbagecollect.jp>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60821] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (3 preceding siblings ...)
  2014-02-18  9:34 ` [ruby-core:60819] " shugo
@ 2014-02-18  9:58 ` usa
  2014-02-18 10:43 ` [ruby-core:60823] " normalperson
                   ` (26 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: usa @ 2014-02-18  9:58 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Usaku NAKAMURA.

ruby -v changed from ruby 2.1.0p0 (2013-12-25 revision 44422) [x86_64-darwin13.0] to -

 Hi,
 
 In message "[ruby-core:60819] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR"
     on Feb.18,2014 18:34:40, <shugo@ruby-lang.org> wrote:
 > Does anybody know why the following code in ext/socket/init.c is necessary?
 > 
 >             if (sockerr == 0)
 >                 continue;       /* workaround for winsock */
 
 It was introduced at r7931 by me (9 years ago!),
 but I forgot the reason.
 If my comment of those days is believed, we may wrap it
 with #ifdef_WIN32.
 
 
 Regards,
 -- 
 U.Nakamura <usa@garbagecollect.jp>

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60822] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-18  9:49   ` [ruby-core:60820] " U.Nakamura
@ 2014-02-18 10:36     ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-02-18 10:36 UTC (permalink / raw
  To: Ruby developers

"U.Nakamura" <usa@garbagecollect.jp> wrote:
> In message "[ruby-core:60819] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR"
>     on Feb.18,2014 18:34:40, <shugo@ruby-lang.org> wrote:
> > Does anybody know why the following code in ext/socket/init.c is necessary?
> > 
> >             if (sockerr == 0)
> >                 continue;       /* workaround for winsock */
> 
> It was introduced at r7931 by me (9 years ago!),
> but I forgot the reason.
> If my comment of those days is believed, we may wrap it
> with #ifdef_WIN32.

OK.  I wonder if we should even use getsockopt(SO_ERROR) at all.

I know there's much literature which recommends it, but any error check
in this way is racy.  Better to let any subsequent
write/read/send/recv/etc error out.

The following should work:

  connect() -> EINPROGRESS
  rb_wait_for_single_fd -> (must retry on EINTR)
  (user calls) write() -> 0, ENOTCONN, EPIPE, ...

Note: rb_wait_for_single_fd is necessary in FreeBSD (at least) to avoid
ENOTCONN, Linux just returns EAGAIN on write if write immediately after
connect.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60823] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (4 preceding siblings ...)
  2014-02-18  9:58 ` [ruby-core:60821] " usa
@ 2014-02-18 10:43 ` normalperson
  2014-02-19  3:11 ` [ruby-core:60856] " shugo
                   ` (25 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-18 10:43 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 "U.Nakamura" <usa@garbagecollect.jp> wrote:
 > In message "[ruby-core:60819] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR"
 >     on Feb.18,2014 18:34:40, <shugo@ruby-lang.org> wrote:
 > > Does anybody know why the following code in ext/socket/init.c is necessary?
 > > 
 > >             if (sockerr == 0)
 > >                 continue;       /* workaround for winsock */
 > 
 > It was introduced at r7931 by me (9 years ago!),
 > but I forgot the reason.
 > If my comment of those days is believed, we may wrap it
 > with #ifdef_WIN32.
 
 OK.  I wonder if we should even use getsockopt(SO_ERROR) at all.
 
 I know there's much literature which recommends it, but any error check
 in this way is racy.  Better to let any subsequent
 write/read/send/recv/etc error out.
 
 The following should work:
 
   connect() -> EINPROGRESS
   rb_wait_for_single_fd -> (must retry on EINTR)
   (user calls) write() -> 0, ENOTCONN, EPIPE, ...
 
 Note: rb_wait_for_single_fd is necessary in FreeBSD (at least) to avoid
 ENOTCONN, Linux just returns EAGAIN on write if write immediately after
 connect.

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60856] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (5 preceding siblings ...)
  2014-02-18 10:43 ` [ruby-core:60823] " normalperson
@ 2014-02-19  3:11 ` shugo
  2014-02-19  8:12   ` [ruby-core:60861] " Eric Wong
  2014-02-19  8:19 ` [ruby-core:60862] " normalperson
                   ` (24 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: shugo @ 2014-02-19  3:11 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Shugo Maeda.


Eric Wong wrote:
>  OK.  I wonder if we should even use getsockopt(SO_ERROR) at all.
>  
>  I know there's much literature which recommends it, but any error check
>  in this way is racy.  Better to let any subsequent
>  write/read/send/recv/etc error out.

Could you describe such a race condition in detail?

I don't see a race condition on FreeBSD 10.

The problem I've seen is that rb_wait_for_single_fd() returns RB_WAITFD_OUT, and getsockopt() with SO_ERROR doesn't return any error (this is an expected behavior when the socket is connected), and Ruby goes in an infinite loop.

This problem was introduced by r31424, and usa is not guilty at least for the problem I've seen.

How about to fix the code as follows?

            if (sockerr == 0) {
                if (revents & RB_WAITFD_OUT) {
                    break;
                }
                else {
                    continue;       /* control is reached here on winsock? */
                }
            }

And the following code seems to be unnecessary.

            if ((revents & (RB_WAITFD_IN|RB_WAITFD_OUT)) == RB_WAITFD_OUT) {
                ret = 0;
                break;
            }

As the following comment says, the intentions of `if (revents & (RB_WAITFD_IN|RB_WAITFD_OUT)) {` might be `if (revents & RB_WIATFD_IN && revents & RB_WAITFD_OUT)`, but I guess revents & RB_WAITFD_IN is true not only when a failure occurs, but when data is ready to read.

        /*
         * Stevens book says, successful finish turn on RB_WAITFD_OUT and
         * failure finish turn on both RB_WAITFD_IN and RB_WAITFD_OUT.
         */

Note that the original problem reported by Charlie must be a different issue.
Charlie, could you show the output of dtruss when the problem happens?


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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60861] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-19  3:11 ` [ruby-core:60856] " shugo
@ 2014-02-19  8:12   ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-02-19  8:12 UTC (permalink / raw
  To: Ruby developers

shugo@ruby-lang.org wrote:
> Eric Wong wrote:
> >  OK.  I wonder if we should even use getsockopt(SO_ERROR) at all.
> >  
> >  I know there's much literature which recommends it, but any error check
> >  in this way is racy.  Better to let any subsequent
> >  write/read/send/recv/etc error out.
> 
> Could you describe such a race condition in detail?

	getsockopt(SO_ERROR) => no error
	<kernel sees disconnect>
	read/write => EPIPE, ENOTCONN, ...

Since we must check all (future) read/write operations for errors
anyways, getsockopt(SO_ERROR) is worthless.

> I don't see a race condition on FreeBSD 10.
> 
> The problem I've seen is that rb_wait_for_single_fd() returns RB_WAITFD_OUT, and getsockopt() with SO_ERROR doesn't return any error (this is an expected behavior when the socket is connected), and Ruby goes in an infinite loop.
> 
> This problem was introduced by r31424, and usa is not guilty at least for the problem I've seen.
> 
> How about to fix the code as follows?
> 
>             if (sockerr == 0) {
>                 if (revents & RB_WAITFD_OUT) {
>                     break;
>                 }
>                 else {
>                     continue;       /* control is reached here on winsock? */
>                 }
>             }

Maybe, but I wonder if we can just drop a lot of code...

	http://bogomips.org/ruby.git/patch?id=a4212dc9516f4

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60862] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (6 preceding siblings ...)
  2014-02-19  3:11 ` [ruby-core:60856] " shugo
@ 2014-02-19  8:19 ` normalperson
  2014-02-19  9:01 ` [ruby-core:60863] " shugo
                   ` (23 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-19  8:19 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 shugo@ruby-lang.org wrote:
 > Eric Wong wrote:
 > >  OK.  I wonder if we should even use getsockopt(SO_ERROR) at all.
 > >  
 > >  I know there's much literature which recommends it, but any error check
 > >  in this way is racy.  Better to let any subsequent
 > >  write/read/send/recv/etc error out.
 > 
 > Could you describe such a race condition in detail?
 
 	getsockopt(SO_ERROR) => no error
 	<kernel sees disconnect>
 	read/write => EPIPE, ENOTCONN, ...
 
 Since we must check all (future) read/write operations for errors
 anyways, getsockopt(SO_ERROR) is worthless.
 
 > I don't see a race condition on FreeBSD 10.
 > 
 > The problem I've seen is that rb_wait_for_single_fd() returns RB_WAITFD_OUT, and getsockopt() with SO_ERROR doesn't return any error (this is an expected behavior when the socket is connected), and Ruby goes in an infinite loop.
 > 
 > This problem was introduced by r31424, and usa is not guilty at least for the problem I've seen.
 > 
 > How about to fix the code as follows?
 > 
 >             if (sockerr == 0) {
 >                 if (revents & RB_WAITFD_OUT) {
 >                     break;
 >                 }
 >                 else {
 >                     continue;       /* control is reached here on winsock? */
 >                 }
 >             }
 
 Maybe, but I wonder if we can just drop a lot of code...
 
 	http://bogomips.org/ruby.git/patch?id=a4212dc9516f4

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60863] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (7 preceding siblings ...)
  2014-02-19  8:19 ` [ruby-core:60862] " normalperson
@ 2014-02-19  9:01 ` shugo
  2014-02-19  9:22   ` [ruby-core:60864] " Eric Wong
  2014-02-19  9:33 ` [ruby-core:60866] " normalperson
                   ` (22 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: shugo @ 2014-02-19  9:01 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Shugo Maeda.


Eric Wong wrote:
>  > Could you describe such a race condition in detail?
>  
>  	getsockopt(SO_ERROR) => no error
>  	<kernel sees disconnect>
>  	read/write => EPIPE, ENOTCONN, ...
>  
>  Since we must check all (future) read/write operations for errors

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.

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

>  Maybe, but I wonder if we can just drop a lot of code...
>  
>  	http://bogomips.org/ruby.git/patch?id=a4212dc9516f4

With the patch, connect() is called again even if getsockopt(SO_ERROR) returns an error, so Errno::EINVAL is raised.  It's confusing behavior.


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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60864] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-19  9:01 ` [ruby-core:60863] " shugo
@ 2014-02-19  9:22   ` Eric Wong
  2014-02-19  9:32     ` [ruby-core:60865] " Eric Wong
  0 siblings, 1 reply; 48+ messages in thread
From: Eric Wong @ 2014-02-19  9:22 UTC (permalink / raw
  To: Ruby developers

shugo@ruby-lang.org wrote:
> Eric Wong wrote:
> >  > Could you describe such a race condition in detail?
> >  
> >  	getsockopt(SO_ERROR) => no error
> >  	<kernel sees disconnect>
> >  	read/write => EPIPE, ENOTCONN, ...
> >  
> >  Since we must check all (future) read/write operations for errors
 
> 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.
And even if connect() succeeded, the server could decide to disconnect
right away after accept() due to overload, so probably less important.

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

> >  Maybe, but I wonder if we can just drop a lot of code...
> >  
> >  	http://bogomips.org/ruby.git/patch?id=a4212dc9516f4
> 
> With the patch, connect() is called again even if getsockopt(SO_ERROR) returns an error, so Errno::EINVAL is raised.  It's confusing behavior.

Ah, I forget the outer for(;;) loop.  Maybe it's better to not loop,
the WAIT_IN_PROGRESS stuff is confusing...

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60865] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-19  9:22   ` [ruby-core:60864] " Eric Wong
@ 2014-02-19  9:32     ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-02-19  9:32 UTC (permalink / raw
  To: Ruby developers

Eric Wong <normalperson@yhbt.net> wrote:
> Ah, I forget the outer for(;;) loop.  Maybe it's better to not loop,
> the WAIT_IN_PROGRESS stuff is confusing...

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.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60866] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (8 preceding siblings ...)
  2014-02-19  9:01 ` [ruby-core:60863] " shugo
@ 2014-02-19  9:33 ` normalperson
  2014-02-19  9:33 ` [ruby-core:60867] " normalperson
                   ` (21 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-19  9:33 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 shugo@ruby-lang.org wrote:
 > Eric Wong wrote:
 > >  > Could you describe such a race condition in detail?
 > >  
 > >  	getsockopt(SO_ERROR) => no error
 > >  	<kernel sees disconnect>
 > >  	read/write => EPIPE, ENOTCONN, ...
 > >  
 > >  Since we must check all (future) read/write operations for errors
  
 > 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.
 And even if connect() succeeded, the server could decide to disconnect
 right away after accept() due to overload, so probably less important.
 
 > >  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.
 
 > >  Maybe, but I wonder if we can just drop a lot of code...
 > >  
 > >  	http://bogomips.org/ruby.git/patch?id=a4212dc9516f4
 > 
 > With the patch, connect() is called again even if getsockopt(SO_ERROR) returns an error, so Errno::EINVAL is raised.  It's confusing behavior.
 
 Ah, I forget the outer for(;;) loop.  Maybe it's better to not loop,
 the WAIT_IN_PROGRESS stuff is confusing...

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60867] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (9 preceding siblings ...)
  2014-02-19  9:33 ` [ruby-core:60866] " normalperson
@ 2014-02-19  9:33 ` normalperson
  2014-02-19 12:00 ` [ruby-core:60869] " shugo
                   ` (20 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-19  9:33 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 Eric Wong <normalperson@yhbt.net> wrote:
 > Ah, I forget the outer for(;;) loop.  Maybe it's better to not loop,
 > the WAIT_IN_PROGRESS stuff is confusing...
 
 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.

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60869] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (10 preceding siblings ...)
  2014-02-19  9:33 ` [ruby-core:60867] " normalperson
@ 2014-02-19 12:00 ` shugo
  2014-02-19 20:38   ` [ruby-core:60876] " Eric Wong
  2014-02-19 20:43 ` [ruby-core:60877] " normalperson
                   ` (19 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: shugo @ 2014-02-19 12:00 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Shugo Maeda.


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.

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

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

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

> Ah, I forget the outer for(;;) loop. Maybe it's better to not loop,
> the WAIT_IN_PROGRESS stuff is confusing...

I agree that the code is complicated, and it's better to simplify it, if possible.

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

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.

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60876] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-19 12:00 ` [ruby-core:60869] " shugo
@ 2014-02-19 20:38   ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-02-19 20:38 UTC (permalink / raw
  To: Ruby developers

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60877] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (11 preceding siblings ...)
  2014-02-19 12:00 ` [ruby-core:60869] " shugo
@ 2014-02-19 20:43 ` normalperson
  2014-02-20  8:26 ` [ruby-core:60893] " shugo
                   ` (18 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-19 20:43 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


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

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60893] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (12 preceding siblings ...)
  2014-02-19 20:43 ` [ruby-core:60877] " normalperson
@ 2014-02-20  8:26 ` shugo
  2014-02-20 22:03   ` [ruby-core:60912] " Eric Wong
  2014-02-20 22:09 ` [ruby-core:60913] " normalperson
                   ` (17 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: shugo @ 2014-02-20  8:26 UTC (permalink / raw
  To: ruby-core

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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60912] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-20  8:26 ` [ruby-core:60893] " shugo
@ 2014-02-20 22:03   ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-02-20 22:03 UTC (permalink / raw
  To: Ruby developers

shugo@ruby-lang.org wrote:
> I'd like to hear others' thoughts.

Likewise.  We need to hear folks with more experience on other OSes.

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

Interesting.  Anyways I'm not against handling ERESTART.

Note, POSIX connect manpage says this:

	If connect() is interrupted by a signal that is caught while blocked
	waiting to establish a connection, connect() shall fail and set errno
	to [EINTR], but the connection request shall not be aborted, and the
	connection shall be established asynchronously.

ref: http://pubs.opengroup.org/onlinepubs/009695399/functions/connect.html
Of course, not every system is POSIX-compliant.

Anyways, I have an alternative (v3) patch here which retries connect()
on EINTR and ERESTART:
  http://bogomips.org/ruby.git/patch?id=8f48b1862
  (also note my new switch/case style to avoid inline ifdef :)

However, I still prefer my v2 if possible:
  http://bogomips.org/ruby.git/patch?id=f5e2eb00e5

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60913] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (13 preceding siblings ...)
  2014-02-20  8:26 ` [ruby-core:60893] " shugo
@ 2014-02-20 22:09 ` normalperson
  2014-02-21  3:43 ` [ruby-core:60921] " shugo
                   ` (16 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-20 22:09 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 shugo@ruby-lang.org wrote:
 > I'd like to hear others' thoughts.
 
 Likewise.  We need to hear folks with more experience on other OSes.
 
 > 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.)
 
 Interesting.  Anyways I'm not against handling ERESTART.
 
 Note, POSIX connect manpage says this:
 
 	If connect() is interrupted by a signal that is caught while blocked
 	waiting to establish a connection, connect() shall fail and set errno
 	to [EINTR], but the connection request shall not be aborted, and the
 	connection shall be established asynchronously.
 
 ref: http://pubs.opengroup.org/onlinepubs/009695399/functions/connect.html
 Of course, not every system is POSIX-compliant.
 
 Anyways, I have an alternative (v3) patch here which retries connect()
 on EINTR and ERESTART:
   http://bogomips.org/ruby.git/patch?id=8f48b1862
   (also note my new switch/case style to avoid inline ifdef :)
 
 However, I still prefer my v2 if possible:
   http://bogomips.org/ruby.git/patch?id=f5e2eb00e5

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60921] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (14 preceding siblings ...)
  2014-02-20 22:09 ` [ruby-core:60913] " normalperson
@ 2014-02-21  3:43 ` shugo
  2014-02-21  7:31   ` [ruby-core:60928] " Eric Wong
  2014-02-21  7:38 ` [ruby-core:60930] " normalperson
                   ` (15 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: shugo @ 2014-02-21  3:43 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Shugo Maeda.


Eric Wong wrote:
>  Anyways, I have an alternative (v3) patch here which retries connect()
>  on EINTR and ERESTART:
>    http://bogomips.org/ruby.git/patch?id=8f48b1862
>    (also note my new switch/case style to avoid inline ifdef :)
>  
>  However, I still prefer my v2 if possible:
>    http://bogomips.org/ruby.git/patch?id=f5e2eb00e5

I prefer the latter too, but is `break` missing in the case clause?

Linux's man page of connect(2) says it's conforming to POSIX.1-2001 (in spite of the fact that it returns ERESTART), so I hope we don't need the second connect().  What do you think, Kosaki-san?

Or only retry on ERESTART is enough.  We don't need the nested switch statements in the former patch, do we?

And I'd like to add the following minimal error handling code to wait_connectable():

    static int
    wait_connectable(int fd)
    {
        int sockerr;
        socklen_t sockerrlen;
    
        /*
         * Stevens book says, successful finish turn on RB_WAITFD_OUT and
         * failure finish turn on both RB_WAITFD_IN and RB_WAITFD_OUT.
         * So it's enough to wait only RB_WAITFD_OUT and check the pending error
         * by getsockopt().
         *
         * Note: rb_wait_for_single_fd already retries on EINTR/ERESTART
         */
        int revents = rb_wait_for_single_fd(fd, RB_WAITFD_OUT, NULL);
    
        if (revents < 0)
    	    rb_bug_errno("rb_wait_for_single_fd used improperly", errno);
    
        sockerrlen = (socklen_t)sizeof(sockerr);
        if (getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&sockerr, &sockerrlen) < 0)
            return -1;
        if (sockerr != 0) {
            errno = sockerr;
            return -1;
        }
    
        return 0;
    }
    
This doesn't include difficult-to-read platform-dependent code which causes infinite loops.


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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60928] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-21  3:43 ` [ruby-core:60921] " shugo
@ 2014-02-21  7:31   ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-02-21  7:31 UTC (permalink / raw
  To: Ruby developers

shugo@ruby-lang.org wrote:
> Eric Wong wrote:
> >  Anyways, I have an alternative (v3) patch here which retries connect()
> >  on EINTR and ERESTART:
> >    http://bogomips.org/ruby.git/patch?id=8f48b1862
> >    (also note my new switch/case style to avoid inline ifdef :)
> >  
> >  However, I still prefer my v2 if possible:
> >    http://bogomips.org/ruby.git/patch?id=f5e2eb00e5
> 
> I prefer the latter too, but is `break` missing in the case clause?

I don't usually add it for the last case... Do some compilers need it?

> Linux's man page of connect(2) says it's conforming to POSIX.1-2001 (in spite of the fact that it returns ERESTART), so I hope we don't need the second connect().  What do you think, Kosaki-san?
> 
> Or only retry on ERESTART is enough.  We don't need the nested switch statements in the former patch, do we?

I'm not sure... Anyways, I think I may realize what happened with
Charlie's EINTR...

> And I'd like to add the following minimal error handling code to wait_connectable():

I think it is OK with a minor tweak below:

>     static int
>     wait_connectable(int fd)
>     {
>         int sockerr;
>         socklen_t sockerrlen;
>     
>         /*
>          * Stevens book says, successful finish turn on RB_WAITFD_OUT and
>          * failure finish turn on both RB_WAITFD_IN and RB_WAITFD_OUT.
>          * So it's enough to wait only RB_WAITFD_OUT and check the pending error
>          * by getsockopt().
>          *
>          * Note: rb_wait_for_single_fd already retries on EINTR/ERESTART
>          */
>         int revents = rb_wait_for_single_fd(fd, RB_WAITFD_OUT, NULL);
>     
>         if (revents < 0)
>     	    rb_bug_errno("rb_wait_for_single_fd used improperly", errno);
>     
>         sockerrlen = (socklen_t)sizeof(sockerr);
>         if (getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&sockerr, &sockerrlen) < 0)
>             return -1;
>         if (sockerr != 0) {
>             errno = sockerr;

We should guard against sockerr setting errno to
EINTR, ERESTART, EINPROGRESS, EALREADY, EISCONN here...

Here is what I think happened in Charlie's original case:

	connect() -> EINTR, kernel socket remembers this!
	retry connect() -> EALREADY
	wait_connectable()
	    rb_wait_for_single_fd() -> success
	    getsockopt(SO_ERROR), sockerr = EINTR from first connect!
	    errno = sockerr -> raise :(

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60930] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (15 preceding siblings ...)
  2014-02-21  3:43 ` [ruby-core:60921] " shugo
@ 2014-02-21  7:38 ` normalperson
  2014-02-21  8:17 ` [ruby-core:60933] " shugo
                   ` (14 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-21  7:38 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 shugo@ruby-lang.org wrote:
 > Eric Wong wrote:
 > >  Anyways, I have an alternative (v3) patch here which retries connect()
 > >  on EINTR and ERESTART:
 > >    http://bogomips.org/ruby.git/patch?id=8f48b1862
 > >    (also note my new switch/case style to avoid inline ifdef :)
 > >  
 > >  However, I still prefer my v2 if possible:
 > >    http://bogomips.org/ruby.git/patch?id=f5e2eb00e5
 > 
 > I prefer the latter too, but is `break` missing in the case clause?
 
 I don't usually add it for the last case... Do some compilers need it?
 
 > Linux's man page of connect(2) says it's conforming to POSIX.1-2001 (in spite of the fact that it returns ERESTART), so I hope we don't need the second connect().  What do you think, Kosaki-san?
 > 
 > Or only retry on ERESTART is enough.  We don't need the nested switch statements in the former patch, do we?
 
 I'm not sure... Anyways, I think I may realize what happened with
 Charlie's EINTR...
 
 > And I'd like to add the following minimal error handling code to wait_connectable():
 
 I think it is OK with a minor tweak below:
 
 >     static int
 >     wait_connectable(int fd)
 >     {
 >         int sockerr;
 >         socklen_t sockerrlen;
 >     
 >         /*
 >          * Stevens book says, successful finish turn on RB_WAITFD_OUT and
 >          * failure finish turn on both RB_WAITFD_IN and RB_WAITFD_OUT.
 >          * So it's enough to wait only RB_WAITFD_OUT and check the pending error
 >          * by getsockopt().
 >          *
 >          * Note: rb_wait_for_single_fd already retries on EINTR/ERESTART
 >          */
 >         int revents = rb_wait_for_single_fd(fd, RB_WAITFD_OUT, NULL);
 >     
 >         if (revents < 0)
 >     	    rb_bug_errno("rb_wait_for_single_fd used improperly", errno);
 >     
 >         sockerrlen = (socklen_t)sizeof(sockerr);
 >         if (getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&sockerr, &sockerrlen) < 0)
 >             return -1;
 >         if (sockerr != 0) {
 >             errno = sockerr;
 
 We should guard against sockerr setting errno to
 EINTR, ERESTART, EINPROGRESS, EALREADY, EISCONN here...
 
 Here is what I think happened in Charlie's original case:
 
 	connect() -> EINTR, kernel socket remembers this!
 	retry connect() -> EALREADY
 	wait_connectable()
 	    rb_wait_for_single_fd() -> success
 	    getsockopt(SO_ERROR), sockerr = EINTR from first connect!
 	    errno = sockerr -> raise :(

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60933] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (16 preceding siblings ...)
  2014-02-21  7:38 ` [ruby-core:60930] " normalperson
@ 2014-02-21  8:17 ` shugo
  2014-02-22  0:39   ` [ruby-core:60958] " Eric Wong
  2014-02-21 13:11 ` [ruby-core:60937] " shugo
                   ` (13 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: shugo @ 2014-02-21  8:17 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Shugo Maeda.


Eric Wong wrote:
>  > >  However, I still prefer my v2 if possible:
>  > >    http://bogomips.org/ruby.git/patch?id=f5e2eb00e5
>  > 
>  > I prefer the latter too, but is `break` missing in the case clause?
>  
>  I don't usually add it for the last case... Do some compilers need it?

It's OK if it's your style.  I just noticed it when I was trying to add an extra case clause for ERESTART.

>  >         sockerrlen = (socklen_t)sizeof(sockerr);
>  >         if (getsockopt(fd, SOL_SOCKET, SO_ERROR, (void *)&sockerr, &sockerrlen) < 0)
>  >             return -1;
>  >         if (sockerr != 0) {
>  >             errno = sockerr;
>  
>  We should guard against sockerr setting errno to
>  EINTR, ERESTART, EINPROGRESS, EALREADY, EISCONN here...

Agreed.  You mean to expose only usual errors to users and to hide unusual platform-dependent errors, right?

>  Here is what I think happened in Charlie's original case:
>  
>  	connect() -> EINTR, kernel socket remembers this!
>  	retry connect() -> EALREADY
>  	wait_connectable()
>  	    rb_wait_for_single_fd() -> success
>  	    getsockopt(SO_ERROR), sockerr = EINTR from first connect!
>  	    errno = sockerr -> raise :(

Charlie said Errno::ENOTCONN is raised by TCPSocket#write, not by TCPSocket.new.
I don't know why....

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

* 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/

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60937] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (17 preceding siblings ...)
  2014-02-21  8:17 ` [ruby-core:60933] " shugo
@ 2014-02-21 13:11 ` shugo
  2014-02-21 15:06 ` [ruby-core:60948] " shugo
                   ` (12 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: shugo @ 2014-02-21 13:11 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Shugo Maeda.

File wait_connectable_infinite_loop_minimal_fix.diff added

Shugo Maeda wrote:
> On FreeBSD 10, Errno::ENOTCONN isn't raised, but Ruby goes in an infinite loop because getsockopt(2) in wait_connectable() sets sockerr to 0.

Can I commit the attached minimal fix as a workaround for this infinite loop bug?

Ruby 1.9.3 release is scheduled on Feb 24, and this is the last chance to fix normal bugs.
It's hard to estimate the impact of Eric's patch by then.

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

* 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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60948] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (18 preceding siblings ...)
  2014-02-21 13:11 ` [ruby-core:60937] " shugo
@ 2014-02-21 15:06 ` shugo
  2014-02-22  0:48 ` [ruby-core:60959] " normalperson
                   ` (11 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: shugo @ 2014-02-21 15:06 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Shugo Maeda.


Shugo Maeda wrote:
> Shugo Maeda wrote:
> > On FreeBSD 10, Errno::ENOTCONN isn't raised, but Ruby goes in an infinite loop because getsockopt(2) in wait_connectable() sets sockerr to 0.
> 
> Can I commit the attached minimal fix as a workaround for this infinite loop bug?
> 
> Ruby 1.9.3 release is scheduled on Feb 24, and this is the last chance to fix normal bugs.
> It's hard to estimate the impact of Eric's patch by then.

I talked with Naruse-san and Nakamura-san, and have created #9547 and have committed the workaround.

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

* 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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60958] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-21  8:17 ` [ruby-core:60933] " shugo
@ 2014-02-22  0:39   ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-02-22  0:39 UTC (permalink / raw
  To: Ruby developers

shugo@ruby-lang.org wrote:
> Eric Wong wrote:
> >  We should guard against sockerr setting errno to
> >  EINTR, ERESTART, EINPROGRESS, EALREADY, EISCONN here...
> 
> Agreed.  You mean to expose only usual errors to users and to hide unusual
> platform-dependent errors, right?

Yes, because we already handle most of those with rb_wait_for_single_fd.

http://bogomips.org/ruby.git/patch?id=d8241102f54
  git://80x24.org/ruby socket-connect-check-v4

> Charlie said Errno::ENOTCONN is raised by TCPSocket#write, not by TCPSocket.new.
> I don't know why....

Maybe one additional change helps:
http://bogomips.org/ruby.git/patch?id=66ca3633f43
  git://80x24.org/ruby socket-connect-check-v5

I'm out of ideas.  Charlie?

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60959] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (19 preceding siblings ...)
  2014-02-21 15:06 ` [ruby-core:60948] " shugo
@ 2014-02-22  0:48 ` normalperson
  2014-02-22  0:52 ` [ruby-core:60961] " usa
                   ` (10 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-22  0:48 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 shugo@ruby-lang.org wrote:
 > Eric Wong wrote:
 > >  We should guard against sockerr setting errno to
 > >  EINTR, ERESTART, EINPROGRESS, EALREADY, EISCONN here...
 > 
 > Agreed.  You mean to expose only usual errors to users and to hide unusual
 > platform-dependent errors, right?
 
 Yes, because we already handle most of those with rb_wait_for_single_fd.
 
 http://bogomips.org/ruby.git/patch?id=d8241102f54
   git://80x24.org/ruby socket-connect-check-v4
 
 > Charlie said Errno::ENOTCONN is raised by TCPSocket#write, not by TCPSocket.new.
 > I don't know why....
 
 Maybe one additional change helps:
 http://bogomips.org/ruby.git/patch?id=66ca3633f43
   git://80x24.org/ruby socket-connect-check-v5
 
 I'm out of ideas.  Charlie?

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

* 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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60961] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (20 preceding siblings ...)
  2014-02-22  0:48 ` [ruby-core:60959] " normalperson
@ 2014-02-22  0:52 ` usa
  2014-02-22  5:34 ` [ruby-core:60977] " shugo
                   ` (9 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: usa @ 2014-02-22  0:52 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Usaku NAKAMURA.

Related to Bug #9547: TCPSocket.new causes an infinite loop when interrupted by a signal added

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

* 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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60977] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (21 preceding siblings ...)
  2014-02-22  0:52 ` [ruby-core:60961] " usa
@ 2014-02-22  5:34 ` shugo
  2014-02-22  6:46   ` [ruby-core:60987] " Eric Wong
  2014-02-22  6:49 ` [ruby-core:60988] " normalperson
                   ` (8 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: shugo @ 2014-02-22  5:34 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Shugo Maeda.


Eric Wong wrote:
> shugo@ruby-lang.org wrote:
>  > Eric Wong wrote:
>  > >  We should guard against sockerr setting errno to
>  > >  EINTR, ERESTART, EINPROGRESS, EALREADY, EISCONN here...
>  > 
>  > Agreed.  You mean to expose only usual errors to users and to hide unusual
>  > platform-dependent errors, right?
>  
>  Yes, because we already handle most of those with rb_wait_for_single_fd.

Definitely.

>  http://bogomips.org/ruby.git/patch?id=d8241102f54
>    git://80x24.org/ruby socket-connect-check-v4

The code looks fine, but please remove the following comment in wait_connectable().

     * So it's enough to wait only RB_WAITFD_OUT and check the pending error
     * by getsockopt().

Or there might be no need to wait RB_WAITFD_IN.  I'm not sure.


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

* 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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60987] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-02-22  5:34 ` [ruby-core:60977] " shugo
@ 2014-02-22  6:46   ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-02-22  6:46 UTC (permalink / raw
  To: Ruby developers

shugo@ruby-lang.org wrote:
> >  http://bogomips.org/ruby.git/patch?id=d8241102f54
> >    git://80x24.org/ruby socket-connect-check-v4
> 
> The code looks fine, but please remove the following comment in wait_connectable().
> 
>      * So it's enough to wait only RB_WAITFD_OUT and check the pending error
>      * by getsockopt().

OK, I'll remove if we commit these versions.

> Or there might be no need to wait RB_WAITFD_IN.  I'm not sure.

No need on Linux, but I think it is also harmless.

I'd like to hear from Charlie to see if -v4 or -v5 can fix
his error, first.  -v5 might be more better, in fact..

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:60988] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (22 preceding siblings ...)
  2014-02-22  5:34 ` [ruby-core:60977] " shugo
@ 2014-02-22  6:49 ` normalperson
  2014-09-16 15:37 ` [ruby-core:65063] " noice
                   ` (7 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-02-22  6:49 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 shugo@ruby-lang.org wrote:
 > >  http://bogomips.org/ruby.git/patch?id=d8241102f54
 > >    git://80x24.org/ruby socket-connect-check-v4
 > 
 > The code looks fine, but please remove the following comment in wait_connectable().
 > 
 >      * So it's enough to wait only RB_WAITFD_OUT and check the pending error
 >      * by getsockopt().
 
 OK, I'll remove if we commit these versions.
 
 > Or there might be no need to wait RB_WAITFD_IN.  I'm not sure.
 
 No need on Linux, but I think it is also harmless.
 
 I'd like to hear from Charlie to see if -v4 or -v5 can fix
 his error, first.  -v5 might be more better, in fact..

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

* 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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65063] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (23 preceding siblings ...)
  2014-02-22  6:49 ` [ruby-core:60988] " normalperson
@ 2014-09-16 15:37 ` noice
  2014-09-17  7:50   ` [ruby-core:65085] " Eric Wong
  2014-09-17  7:58 ` [ruby-core:65086] " normalperson
                   ` (6 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: noice @ 2014-09-16 15:37 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by NoICE Reaver.


Any news on this issue? Something is causing this ENOTCONN issue on OS X and this issue might be related:

**ruby 2.0.0p253**

<code>
Errno::ENOTCONN: Socket is not connected
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/protocol.rb:211:in `write'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/protocol.rb:211:in `write0'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/protocol.rb:185:in `block in write'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/protocol.rb:202:in `writing'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/protocol.rb:184:in `write'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/http/generic_request.rb:325:in `write_header'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/http/generic_request.rb:136:in `exec'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/http.rb:1404:in `block in transport_request'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/http.rb:1403:in `catch'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/http.rb:1403:in `transport_request'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/http.rb:1376:in `request'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/open-uri.rb:319:in `block in open_http'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/net/http.rb:852:in `start'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/open-uri.rb:313:in `open_http'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/open-uri.rb:708:in `buffer_open'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/open-uri.rb:210:in `block in open_loop'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/open-uri.rb:208:in `catch'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/open-uri.rb:208:in `open_loop'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/open-uri.rb:149:in `open_uri'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/open-uri.rb:688:in `open'
/Users/noice/.rvm/rubies/ruby-2.0.0-p353/lib/ruby/2.0.0/open-uri.rb:34:in `open'
...
</code>

**ruby 2.1.2p95**

<code>
Errno::ENOTCONN: Socket is not connected
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/protocol.rb:211:in `write'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/protocol.rb:211:in `write0'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/protocol.rb:185:in `block in write'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/protocol.rb:202:in `writing'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/protocol.rb:184:in `write'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/http/generic_request.rb:325:in `write_header'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/http/generic_request.rb:136:in `exec'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/http.rb:1406:in `block in transport_request'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/http.rb:1405:in `catch'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/http.rb:1405:in `transport_request'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/http.rb:1378:in `request'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/open-uri.rb:319:in `block in open_http'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/net/http.rb:853:in `start'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/open-uri.rb:313:in `open_http'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/open-uri.rb:724:in `buffer_open'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/open-uri.rb:210:in `block in open_loop'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/open-uri.rb:208:in `catch'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/open-uri.rb:208:in `open_loop'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/open-uri.rb:149:in `open_uri'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/open-uri.rb:704:in `open'
/Users/noice/.rvm/rubies/ruby-2.1.2/lib/ruby/2.1.0/open-uri.rb:34:in `open'
...
</code>


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

* 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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65085] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-09-16 15:37 ` [ruby-core:65063] " noice
@ 2014-09-17  7:50   ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-09-17  7:50 UTC (permalink / raw
  To: Ruby developers

noice@email.cz wrote:
> Any news on this issue? Something is causing this ENOTCONN issue on OS
> X and this issue might be related:

Can you try the -v4/-v5 patches I proposed?  I'm pretty sure they work
(I haven't thought about this in a while), but Charlie never came back
to test...

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65086] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (24 preceding siblings ...)
  2014-09-16 15:37 ` [ruby-core:65063] " noice
@ 2014-09-17  7:58 ` normalperson
  2014-09-17  9:29 ` [ruby-core:65087] " noice
                   ` (5 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-09-17  7:58 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 noice@email.cz wrote:
 > Any news on this issue? Something is causing this ENOTCONN issue on OS
 > X and this issue might be related:
 
 Can you try the -v4/-v5 patches I proposed?  I'm pretty sure they work
 (I haven't thought about this in a while), but Charlie never came back
 to test...

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

* 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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65087] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (25 preceding siblings ...)
  2014-09-17  7:58 ` [ruby-core:65086] " normalperson
@ 2014-09-17  9:29 ` noice
  2014-09-17 21:20 ` [ruby-core:65095] " normalperson
                   ` (4 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: noice @ 2014-09-17  9:29 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by NoICE Reaver.


Tried both patches at once on current ruby HEAD, let it run for 20 minutes and the issue didn't happen. So .. the patches work :)

Just to be sure, tried to run it with pure ruby head and the issue is still there:

<code>
Errno::ENOTCONN: Socket is not connected
/Users/noice/.rvm/rubies/ruby-head-pure/lib/ruby/2.2.0/net/protocol.rb:211:in `write'
/Users/noice/.rvm/rubies/ruby-head-pure/lib/ruby/2.2.0/net/protocol.rb:211:in `write0'
/Users/noice/.rvm/rubies/ruby-head-pure/lib/ruby/2.2.0/net/protocol.rb:185:in `block in write'
/Users/noice/.rvm/rubies/ruby-head-pure/lib/ruby/2.2.0/net/protocol.rb:202:in `writing'
/Users/noice/.rvm/rubies/ruby-head-pure/lib/ruby/2.2.0/net/protocol.rb:184:in `write'
/Users/noice/.rvm/rubies/ruby-head-pure/lib/ruby/2.2.0/net/http/generic_request.rb:328:in `write_header'
</code>


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

* 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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65095] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (26 preceding siblings ...)
  2014-09-17  9:29 ` [ruby-core:65087] " noice
@ 2014-09-17 21:20 ` normalperson
  2014-09-17 21:26   ` [ruby-core:65096] " Eric Wong
  2014-09-17 21:30 ` [ruby-core:65097] " normalperson
                   ` (3 subsequent siblings)
  31 siblings, 1 reply; 48+ messages in thread
From: normalperson @ 2014-09-17 21:20 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.

Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN to 1.9.3: UNKNOWN, 2.0.0: REQUIRED, 2.1: REQUIRED

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

* Author: Charlie Somerville
* Status: Open
* Priority: Normal
* Assignee: 
* Category: 
* Target version: 
* ruby -v: -
* Backport: 1.9.3: UNKNOWN, 2.0.0: REQUIRED, 2.1: REQUIRED
----------------------------------------
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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65096] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-09-17 21:20 ` [ruby-core:65095] " normalperson
@ 2014-09-17 21:26   ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-09-17 21:26 UTC (permalink / raw
  To: Ruby developers

I've squashed my v5 and v4 patches together as r47617.  Thanks NoICE for
the confirmation on OSX (where this problem was originally reported).

usa:  We may need additional testing on WIN32, but I think
rb_wait_for_single_fd works around many existing compatibility
issues.  Thanks.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65097] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (27 preceding siblings ...)
  2014-09-17 21:20 ` [ruby-core:65095] " normalperson
@ 2014-09-17 21:30 ` normalperson
  2014-09-18 15:03 ` [ruby-core:65108] " usa
                   ` (2 subsequent siblings)
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-09-17 21:30 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 I've squashed my v5 and v4 patches together as r47617.  Thanks NoICE for
 the confirmation on OSX (where this problem was originally reported).
 
 usa:  We may need additional testing on WIN32, but I think
 rb_wait_for_single_fd works around many existing compatibility
 issues.  Thanks.

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

* Author: Charlie Somerville
* Status: Closed
* Priority: Normal
* Assignee: 
* Category: 
* Target version: 
* ruby -v: -
* Backport: 1.9.3: UNKNOWN, 2.0.0: REQUIRED, 2.1: REQUIRED
----------------------------------------
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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65108] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (28 preceding siblings ...)
  2014-09-17 21:30 ` [ruby-core:65097] " normalperson
@ 2014-09-18 15:03 ` usa
  2014-09-18 19:26   ` [ruby-core:65114] " Eric Wong
  2014-09-18 19:32 ` [ruby-core:65116] " normalperson
  2014-09-18 23:30 ` [ruby-core:65119] " usa
  31 siblings, 1 reply; 48+ messages in thread
From: usa @ 2014-09-18 15:03 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Usaku NAKAMURA.


Perhaps, we should write a test that actually causes EINTR during `connect`.

But, when I committed r7931, there was a failure of an existing test, I guess.
At the moment, such failure has not been observed.
So, I think that there is not a big portability problem in this patch.

Thank you, Eric.

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

* Author: Charlie Somerville
* Status: Closed
* Priority: Normal
* Assignee: 
* Category: 
* Target version: 
* ruby -v: -
* Backport: 1.9.3: UNKNOWN, 2.0.0: REQUIRED, 2.1: REQUIRED
----------------------------------------
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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65114] Re: [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-09-18 15:03 ` [ruby-core:65108] " usa
@ 2014-09-18 19:26   ` Eric Wong
  0 siblings, 0 replies; 48+ messages in thread
From: Eric Wong @ 2014-09-18 19:26 UTC (permalink / raw
  To: Ruby developers

usa@garbagecollect.jp wrote:
> Perhaps, we should write a test that actually causes EINTR during `connect`.
> 
> But, when I committed r7931, there was a failure of an existing test, I guess.
> At the moment, such failure has not been observed.
> So, I think that there is not a big portability problem in this patch.

Thanks for confirming.  Unfortunately, EINTR is not easy to trigger on a
connect test case to localhost.  Not even -Wl,--wrap in GNU ld is useful
for testing this, it is dependent on socket state in the kernel.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65116] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (29 preceding siblings ...)
  2014-09-18 15:03 ` [ruby-core:65108] " usa
@ 2014-09-18 19:32 ` normalperson
  2014-09-18 23:30 ` [ruby-core:65119] " usa
  31 siblings, 0 replies; 48+ messages in thread
From: normalperson @ 2014-09-18 19:32 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Eric Wong.


 usa@garbagecollect.jp wrote:
 > Perhaps, we should write a test that actually causes EINTR during `connect`.
 > 
 > But, when I committed r7931, there was a failure of an existing test, I guess.
 > At the moment, such failure has not been observed.
 > So, I think that there is not a big portability problem in this patch.
 
 Thanks for confirming.  Unfortunately, EINTR is not easy to trigger on a
 connect test case to localhost.  Not even -Wl,--wrap in GNU ld is useful
 for testing this, it is dependent on socket state in the kernel.

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

* Author: Charlie Somerville
* Status: Closed
* Priority: Normal
* Assignee: 
* Category: 
* Target version: 
* ruby -v: -
* Backport: 1.9.3: UNKNOWN, 2.0.0: REQUIRED, 2.1: REQUIRED
----------------------------------------
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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* [ruby-core:65119] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR
  2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
                   ` (30 preceding siblings ...)
  2014-09-18 19:32 ` [ruby-core:65116] " normalperson
@ 2014-09-18 23:30 ` usa
  31 siblings, 0 replies; 48+ messages in thread
From: usa @ 2014-09-18 23:30 UTC (permalink / raw
  To: ruby-core

Issue #9356 has been updated by Usaku NAKAMURA.


Eric Wong wrote:
>  Thanks for confirming.  Unfortunately, EINTR is not easy to trigger on a
>  connect test case to localhost.  Not even -Wl,--wrap in GNU ld is useful
>  for testing this, it is dependent on socket state in the kernel.

Thank you, I had thought it would be so :-P
Then, let's wait users' reports or CI failures (of course, I don't want to see them and I hope your patch is all correct.)



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

* Author: Charlie Somerville
* Status: Closed
* Priority: Normal
* Assignee: 
* Category: 
* Target version: 
* ruby -v: -
* Backport: 1.9.3: UNKNOWN, 2.0.0: REQUIRED, 2.1: REQUIRED
----------------------------------------
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)
wait_connectable_infinite_loop_minimal_fix.diff (478 Bytes)


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

^ permalink raw reply	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2014-09-18 23:49 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-03 10:29 [ruby-core:59516] [ruby-trunk - Bug #9356][Open] TCPSocket.new does not seem to handle INTR charliesome (Charlie Somerville)
2014-01-03 17:53 ` [ruby-core:59536] " Eric Wong
2014-01-04  5:05 ` [ruby-core:59546] [ruby-trunk - Bug #9356] " charliesome (Charlie Somerville)
2014-01-04  7:03   ` [ruby-core:59548] " Eric Wong
2014-02-08  2:35     ` [ruby-core:60570] " Eric Wong
2014-02-08  2:40 ` [ruby-core:60571] " normalperson
2014-02-18  9:34 ` [ruby-core:60819] " shugo
2014-02-18  9:49   ` [ruby-core:60820] " U.Nakamura
2014-02-18 10:36     ` [ruby-core:60822] " Eric Wong
2014-02-18  9:58 ` [ruby-core:60821] " usa
2014-02-18 10:43 ` [ruby-core:60823] " normalperson
2014-02-19  3:11 ` [ruby-core:60856] " shugo
2014-02-19  8:12   ` [ruby-core:60861] " Eric Wong
2014-02-19  8:19 ` [ruby-core:60862] " normalperson
2014-02-19  9:01 ` [ruby-core:60863] " shugo
2014-02-19  9:22   ` [ruby-core:60864] " Eric Wong
2014-02-19  9:32     ` [ruby-core:60865] " Eric Wong
2014-02-19  9:33 ` [ruby-core:60866] " normalperson
2014-02-19  9:33 ` [ruby-core:60867] " normalperson
2014-02-19 12:00 ` [ruby-core:60869] " shugo
2014-02-19 20:38   ` [ruby-core:60876] " Eric Wong
2014-02-19 20:43 ` [ruby-core:60877] " normalperson
2014-02-20  8:26 ` [ruby-core:60893] " shugo
2014-02-20 22:03   ` [ruby-core:60912] " Eric Wong
2014-02-20 22:09 ` [ruby-core:60913] " normalperson
2014-02-21  3:43 ` [ruby-core:60921] " shugo
2014-02-21  7:31   ` [ruby-core:60928] " Eric Wong
2014-02-21  7:38 ` [ruby-core:60930] " normalperson
2014-02-21  8:17 ` [ruby-core:60933] " shugo
2014-02-22  0:39   ` [ruby-core:60958] " Eric Wong
2014-02-21 13:11 ` [ruby-core:60937] " shugo
2014-02-21 15:06 ` [ruby-core:60948] " shugo
2014-02-22  0:48 ` [ruby-core:60959] " normalperson
2014-02-22  0:52 ` [ruby-core:60961] " usa
2014-02-22  5:34 ` [ruby-core:60977] " shugo
2014-02-22  6:46   ` [ruby-core:60987] " Eric Wong
2014-02-22  6:49 ` [ruby-core:60988] " normalperson
2014-09-16 15:37 ` [ruby-core:65063] " noice
2014-09-17  7:50   ` [ruby-core:65085] " Eric Wong
2014-09-17  7:58 ` [ruby-core:65086] " normalperson
2014-09-17  9:29 ` [ruby-core:65087] " noice
2014-09-17 21:20 ` [ruby-core:65095] " normalperson
2014-09-17 21:26   ` [ruby-core:65096] " Eric Wong
2014-09-17 21:30 ` [ruby-core:65097] " normalperson
2014-09-18 15:03 ` [ruby-core:65108] " usa
2014-09-18 19:26   ` [ruby-core:65114] " Eric Wong
2014-09-18 19:32 ` [ruby-core:65116] " normalperson
2014-09-18 23:30 ` [ruby-core:65119] " usa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).