[ruby-core:60930] [ruby-trunk - Bug #9356] TCPSocket.new does not seem to handle INTR

Issue #9356 has been updated by Eric Wong.


 [e-mail] 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/