non-blocking IO does not function correctly: connect_nonblock followed by an IO.select

I'm not sure what the correct behavior is here. When I run the following simplified example under both Ruby and JRuby, the results are pretty much identical:

~/projects/jruby ➔ ruby -rsocket -e "addr = Socket.getaddrinfo('google.com', 80).first; sock = Socket.new(addr[4], Socket::SOCK_STREAM, 0); sockaddr = Socket.sockaddr_in(addr[1], addr[3]); begin; sock.connect_nonblock(sockaddr); rescue Errno::EINPROGRESS => e; p e; end; t = Time.now; IO.select(nil, [sock], nil, 5); puts Time.now - t; sock.connect_nonblock(sockaddr)"
#<Errno::EINPROGRESS: Operation now in progress - connect(2)>
0.000818
-e:1:in `connect_nonblock': Socket is already connected - connect(2) (Errno::EISCONN)
	from -e:1

~/projects/jruby &#10132; jruby -rsocket -e "addr = Socket.getaddrinfo('google.com', 80).first; sock = Socket.new(addr[4], Socket::SOCK_STREAM, 0); sockaddr = Socket.sockaddr_in(addr[1], addr[3]); begin; sock.connect_nonblock(sockaddr); rescue Errno::EINPROGRESS => e; p e; end; t = Time.now; IO.select(nil, [sock], nil, 5); puts Time.now - t; sock.connect_nonblock(sockaddr)"
#<Errno::EINPROGRESS: Operation now in progress - Operation now in progress>
0.016
-e:1:in `connect_nonblock': Socket is already connected - Socket is already connected (Errno::EISCONN)
	from -e:1

Sorry for not being clear, my example was running from a console and isn't setup to run as a script.

The thing to pay attention to is the result of the first IO.select call. It should either block for the timeout and then return, or should return that the socket is writable before the timeout. Instead it is just returning immediately that the socket isn't writable.

Everything after the first IO.select is just showing a workaround to coerce jruby into thinking it is connected, and should not be necessary and is not necessary under MRI.

Ok, can you provide a script form of this that shows the problem? I assume some combination of connect_nonblock, followed by a select, followed by a write? The write will fail on JRuby because it's not connected yet?

I could see no obvious problem in the code, and attempted to use the following script to exercise your case (which does appear to produce a NotYetConnectedException:

~/projects/jruby ➔ cat connect_nonblock.rb 
require 'socket'
addr = Socket.getaddrinfo('google.com', 80).first
sock = Socket.new(addr[4], Socket::SOCK_STREAM, 0)
sockaddr = Socket.sockaddr_in(addr[1], addr[3])
begin
  sock.connect_nonblock(sockaddr)
rescue Errno::EINPROGRESS => e
  p e
end
t = Time.now
IO.select(nil, [sock], nil, 5)
puts Time.now - t
p sock.write("GET /\n\n")

~/projects/jruby ➔ jruby connect_nonblock.rb 
#<Errno::EINPROGRESS: Operation now in progress - Operation now in progress>
0.035
SocketChannelImpl.java:130:in `ensureWriteOpen': java.nio.channels.NotYetConnectedException
	from SocketChannelImpl.java:324:in `write'
	from ChannelDescriptor.java:582:in `internalWrite'
	from ChannelDescriptor.java:629:in `write'

Ok, so the base problem here appears to be that Selector.select(timeout) is returning immediately from Java itself, even though it appears we've registered everything correctly. Perhaps there's some interaction between a nonblocking connect and a "write" selection operation I don't know about?

Ok, more information.

Java's selectors seem to separate "write" selects from "connect" selects. Selecting for "write" on an in-progress connection seems to return immediately, without adding the channel to the list of selected items...even though it appears it has completed connecting. If we select on "connect", we must make a subsequent call to SocketChannel.finishConnect to complete the connection process. This is largely the reason for the NotYetConnectedException; the channel is connected and almost ready, but we need to finishConnect.

The related code in IO.select_static is so confusing, I've only been able to hack a nasty improvement that causes your script to work right. Essentially, for write selects, it also registers for connect selects, and if the selection event happened because of "connect", it attempts to call finishConnect on the channel. It still doesn't show up in the list of writable sockets.

https://gist.github.com/661723

I'm going to make an attempt to cleanup/rewrite this logic, since it's really dismally awful right now and too hard to patch.

I've had to provide a workaround in my gem, dalli, due to this bug. While jruby has native threads, a blocking connect means that a misbehaving remote server can block a request for up to 30-60 seconds. Providing reliable timeouts for connect/read/write is critical for my gem.

This issue seems to also haunt the thrift gem (version 0.7.0, at least on windows).

From thrift/transport/socket.rb:

37 addrinfo = ::Socket::getaddrinfo(@host, @port).first
38 @handle = ::Socket.new(addrinfo[4], ::Socket::SOCK_STREAM, 0)
39 sockaddr = ::Socket.sockaddr_in(addrinfo[1], addrinfo[3])
40 begin
41 @handle.connect_nonblock(sockaddr)
42 rescue Errno::EINPROGRESS
43 unless IO.select(nil, [ @handle ], nil, @timeout)
44 raise TransportException.new(TransportException::NOT_OPEN, "Connection timeout to #

")
45 end
46 begin
47 @handle.connect_nonblock(sockaddr)
48 rescue Errno::EISCONN
49 end
50 end

We always hit line 44. Seems related to what's discussed in this thread.

Quite unfortunate that it blocks thrift under jruby so I'd love if it could get some attention. I'm not qualified myself to dig into it I'm afraid.

We're also getting blocked by this bug as it effects the thrift gem. Makes it hard to use jruby as a thrift client without resorting to native java thrift interface. A blocker for using Rhino to talk to HBase as it expects a ruby thrift interface. Any chance Charles's previously mentioned patch or its cleaner version could get resolved?

Noting pull request https://github.com/jruby/jruby/pull/109 by Ryan Fowler. Anyone want to comment on his fix?

Ok...I looked over this a week or so ago, and here's my conclusion...

It's not the right fix, but it might be benign enough to add if we additionally add an isConnectionPending call to tell us if we're in the midpoint between nonblocking connect and finishConnect. I'm willing to go ahead with that fix if we can come up with something better.

Backstory: When I started looking into this, it triggered my starting a larger-scale refactoring of RubySocket and friends. The logic in there is horrendous, and I needed to clean it up before I could make the right fix. The right fix is that RubySocket needs to do the finishConnect when coming back from a select operation if the connection has not been finished, but the old code did not make that possible. The refactored code is closer to that point but still not there.

I'm not sure we can really do the best "correct" fix on jruby-1_6, so the small localized fix may be good enough. We can work on doing it right on master.

Does that sound ok? I can merge the pull and add the additional isConnectionPending check.

FWIW, have a look at RubySocket on master compared to RubySocket in 1.6.x. The difference is shocking.

Thanks for looking into it & merging the pull request. I started trying to fix it in RubySocket last month but just couldn't figure out where to make changes, that's how I ended up in SelectBlob. I guess I wasn't just being knucklehead.

It'll be interesting to look at how you've refactored RubySocket.

Thanks again.

Ok...given the temporary fix is in 1.6.7, I'm resolving this for 1.6.7 and will clone it for the "correct" fix in 1.7.

The commits are as follows...

master:

commit 6ff9b45b4cd443d97d272cae94acd331e9d9e95d
Author: Ryan Fowler <rwfowler@gmail.com>
Date:   Wed Jan 11 19:10:26 2012 -0600

    Might fix JRUBY-5165
    
    Run finishConnect when selecting sockets for write.

commit ec54fa5f410f77bd0434a42f1a04229c3616d0c7
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Mon Feb 13 17:07:42 2012 -0600

    Additional tweak for JRUBY-5165
    
    * Check isConnectionPending so we don't finishConnect incorrectly.

jruby-1_6:

commit ac2555f80198a4809040d6ceb31745ba62958202
Author: Ryan Fowler <rwfowler@gmail.com>
Date:   Wed Jan 11 19:10:26 2012 -0600

    Might fix JRUBY-5165
    
    Run finishConnect when selecting sockets for write.
    
    Conflicts:
    
    	src/org/jruby/util/io/SelectBlob.java

commit 826cf57ef1aa2918bce622ba9b0974258032d041
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Mon Feb 13 17:07:42 2012 -0600

    Additional tweak for JRUBY-5165
    
    * Check isConnectionPending so we don't finishConnect incorrectly.

Update for this...I have figured out the right place to do finishConnect (inside Socket connect logic once isConnectionPending) and removed the above hack in the following commits.

commit d80b18fbaec0df59276f5ec2b7ce3de92cce63ce
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Thu Dec 13 14:22:08 2012 -0600

    Remove finishConnect hack for JRUBY-5165.
    
    Commit d3c0496 moves the finishConnect into the Socket connect
    logic where it belongs.

:100644 100644 e414741... 5b24ce9... M	src/org/jruby/util/io/SelectBlob.java

commit bd4916ac6b954692f5ff0ca567cd72aa3a5961d1
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Thu Dec 13 12:20:38 2012 -0600

    Fix connect_nonblock never finishing the connection.
    
    When doing a nonblocking connection, the first SocketChannel
    connect call initiates the connection process. If connect is
    called again, it will always raise ConnectionPending, which we
    propagated as EINPROGRESS. As a result, simply calling
    connect_nonblock repeatedly never finished the connection process
    by calling finishConnect. Our select loop, however, did have a
    finishConnect in it, so doing connect_nonblock + select worked
    ok. This change moves the finishConnect into RubySocket's connect
    logic and calls that once the connection has been initiated.

:100644 100644 7ccbc44... 7469482... M	src/org/jruby/ext/socket/RubySocket.java