JRuby

connect_nonblock + select needs to eventually finishConnect

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.5.3
  • Fix Version/s: JRuby 1.7.0.pre1
  • Component/s: None
  • Labels:
    None
  • Environment:
    repros under os x 10.6.4 with 1.6.0_22 and centos 5.4 with 1.6.0_21
  • Number of attachments :
    0

Description

After doing a connect_nonblock then it appears that IO.select will always return immediately and indicate the socket is not writable, when it should instead block and eventually return that it is writable, assuming the connection succeeds.

Example below. Note that this does not appear to be a timing specific issue; the sleep 5 gives it plenty of time to connect, and the select does not wait for the timeout before returning. Presumably calling connect_nonblock again updates some state that was out of sync.

>> @timeout=5
=> 5
>> @host='google.com'
=> "google.com"
>> @port=80
=> 80
>> addrinfo = ::Socket::getaddrinfo(@host, @port).first
=> ["AF_INET", 80, "pz-in-f99.1e100.net", "74.125.127.99", 2, 2, 17]
>> @handle = ::Socket.new(addrinfo[4], ::Socket::SOCK_STREAM, 0)
=> #<Socket:0x417470d0>
>> sockaddr = ::Socket.sockaddr_in(addrinfo[1], addrinfo[3])
=> "\020\002\000PJ}\177c\000\000\000\000\000\000\000\000"
>> @handle.connect_nonblock(sockaddr)
Errno::EINPROGRESS: Operation now in progress - Operation now in progress
from (irb):18:in `connect_nonblock'
from (irb):18
>> sleep(5)
=> 5
>> t=Time.now; IO.select(nil, [ @handle ], nil, @timeout); puts Time.now-t
0.001
=> nil
>> @handle.connect_nonblock(sockaddr)
Errno::EISCONN: Socket is already connected - Socket is already connected
from (irb):21:in `connect_nonblock'
from (irb):21
>> IO.select(nil, [ @handle ], nil, @timeout)
=> [[], [#<Socket:0x417470d0>], []]

Issue Links

relates to

Bug - A problem which impairs or prevents the functions of the product. JRUBY-5165 non-blocking IO does not function correctly: connect_nonblock followed by an IO.select

  • Minor - Minor loss of function, or other problem where easy workaround is present.
  • Closed - The issue is considered finished, the resolution is correct. Issues which are not closed can be reopened.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Charles Oliver Nutter added a comment -

This was cloned from JRUBY-5165, for which pull request 109 (https://github.com/jruby/jruby/pull/109) was merged in along with some other tweaks. I believe a more proper fix would be to have RubySocket track that it is in nonblocking mode and do a finishConnect before use if there's still a connection pending, rather than putting socket-specific logic in SelectBlob. This bug is to track that, hopefully to be fixed for 1.7.

Show
Charles Oliver Nutter added a comment - This was cloned from JRUBY-5165 , for which pull request 109 ( https://github.com/jruby/jruby/pull/109 ) was merged in along with some other tweaks. I believe a more proper fix would be to have RubySocket track that it is in nonblocking mode and do a finishConnect before use if there's still a connection pending, rather than putting socket-specific logic in SelectBlob. This bug is to track that, hopefully to be fixed for 1.7.
Hide
Permalink
Charles Oliver Nutter added a comment -

I made some recent fixes to connect_nonblock and select to make it properly return the channel, and as part of that I moved the "not great" logic into the actual select block. This basically then will only do finishConnect if:

  • It's a socket
  • The select was interested in write (Ruby perspective)
  • The "ready" operation was connect
  • The socket is not already connected

I think this may be the cleanest logic for now, given that doing it in the socket would require doing it lazily before all other operations, in case we hadn't done anything after connect_nonblock.

If someone has a better understanding of the low-level nonblocking connect + select sequence in C code, my primary question is when (if ever) C code is required to "complete" the connection. I have not been able to determine this from MRI's impl.

Show
Charles Oliver Nutter added a comment - I made some recent fixes to connect_nonblock and select to make it properly return the channel, and as part of that I moved the "not great" logic into the actual select block. This basically then will only do finishConnect if: It's a socket The select was interested in write (Ruby perspective) The "ready" operation was connect The socket is not already connected I think this may be the cleanest logic for now, given that doing it in the socket would require doing it lazily before all other operations, in case we hadn't done anything after connect_nonblock. If someone has a better understanding of the low-level nonblocking connect + select sequence in C code, my primary question is when (if ever) C code is required to "complete" the connection. I have not been able to determine this from MRI's impl.
Hide
Permalink
Charles Oliver Nutter added a comment -

Naturally I felt I needed to make one more attempt at this before walking away.

After poking around man pages and online docs and forums, it seems like the select(2) call basically does wait for the socket to be connected, so our finishConnect in response to an OP_CONNECT event seems quite appropriate. finishConnect may just be a Java artifact to ensure the socket has been completely set up internally, since it hasn't had a chance to go through the connect() logic successfully.

We should be good, although it would be nice to abstract this into a common library at some point. Ahh well.

Show
Charles Oliver Nutter added a comment - Naturally I felt I needed to make one more attempt at this before walking away. After poking around man pages and online docs and forums, it seems like the select(2) call basically does wait for the socket to be connected, so our finishConnect in response to an OP_CONNECT event seems quite appropriate. finishConnect may just be a Java artifact to ensure the socket has been completely set up internally, since it hasn't had a chance to go through the connect() logic successfully. We should be good, although it would be nice to abstract this into a common library at some point. Ahh well.
Hide
Permalink
Tyler Brock added a comment -

Is there any way we can get this to also work in a current release?

I saw the ticket for which it says it's fixed in 1.6.7 but that actually doesn't seem to be the case.

Show
Tyler Brock added a comment - Is there any way we can get this to also work in a current release? I saw the ticket for which it says it's fixed in 1.6.7 but that actually doesn't seem to be the case.
Hide
Permalink
Charles Oliver Nutter added a comment -

Tyler: At this point there's no plan to release a 1.6.8. We did do a hack in 1.6.7 to "finish" the connection, but that fix does not include the logic to also return it as writable on connect events.

A possible workaround in 1.6.7 might be to use the nio4r library, which implements the logic the correct way and has an NIO-like interface. If you can get that to work, post an example here. I think it should be pretty easy.

Stop by IRC if you want to talk about that.

Show
Charles Oliver Nutter added a comment - Tyler: At this point there's no plan to release a 1.6.8. We did do a hack in 1.6.7 to "finish" the connection, but that fix does not include the logic to also return it as writable on connect events. A possible workaround in 1.6.7 might be to use the nio4r library, which implements the logic the correct way and has an NIO-like interface. If you can get that to work, post an example here. I think it should be pretty easy. Stop by IRC if you want to talk about that.

People

  • Assignee:
    Charles Oliver Nutter
    Reporter:
    Marc Slemko
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: