Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Socket IO behave like MRI in JRuby 9k #2448

Closed
haberbyte opened this issue Jan 10, 2015 · 9 comments
Closed

Make Socket IO behave like MRI in JRuby 9k #2448

haberbyte opened this issue Jan 10, 2015 · 9 comments

Comments

@haberbyte
Copy link

@haberbyte haberbyte commented Jan 10, 2015

There is an issue with the pow webserver that only occurs with JRuby (basecamp/pow#14).

It is caused in the nack rack server in the following spot: https://github.com/josh/nack/blob/master/lib/nack/server.rb#L142

With JRuby i get:

IOError: Socket is not connected
org/jruby/ext/socket/RubyBasicSocket.java:423:in `close_read'

Here the link to RubyBasicSocket.java:

I'm not sure what shutdownInternal(context, 0); really does and why it fails.

But i would love to see this fixed and i'm willing to help (if i am able to).
It would be great if JRuby 9k can address this.

What further information to debug this issue can i provide?

@headius
Copy link
Member

@headius headius commented Jan 12, 2015

shutdownInternal(context, 0) should shut down just the read channel of the socket.

Have you tried this on master? Is there an easy way for me to reproduce this?

@headius
Copy link
Member

@headius headius commented Jan 12, 2015

I just tried something I think should reproduce the issue, but it passes on master and JRuby 1.7.18. Maybe we're done? Maybe I don't have a good enough case?

us = UNIXServer.new('/tmp/foo')
uc = UNIXSocket.new('/tmp/foo')
uc.close_read

@haberbyte
Copy link
Author

@haberbyte haberbyte commented Jan 12, 2015

I'm afraid it's not a good enough case and it's a bit more complicated :-(

I had a hard time coming up with a small enough example to reproduce this issue, but i came up with something that might help.

You need to run two ruby scripts, a server.rb and a client.rb.

server.rb:

require 'fcntl'
require 'socket'

server = UNIXServer.new('/tmp/foo')
server.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)

clients = []
buffers = {}

loop do
  listeners = clients
  listeners << server unless server.closed?

  readable, writable = nil
  begin
    readable, writable = IO.select(listeners, nil, nil, 60)
  rescue Errno::EBADF
  end

  next unless readable

  readable.each do |sock|
    if sock == server
      client = server.accept_nonblock
      client.fcntl(Fcntl::F_SETFD, Fcntl::FD_CLOEXEC)
      clients << client
    else
      client, buf = sock, buffers[sock] ||= ''

      begin
        buf << client.read_nonblock(1024)
      rescue EOFError
        sock.close_read

        buffers.delete(client)
        clients.delete(client)

        puts 'WORKING'
        exit
      end
    end
  end
end

client.rb:

require 'socket'

s = UNIXSocket.new("/tmp/foo")
s.send "hello", 0

Now in one terminal run ruby server.rb and in another one run ruby client.rb.

The output for MRI on terminal one:

$ ruby server.rb
WORKING

The output for JRuby on terminal one:

$ ruby server.rb
IOError: Socket is not connected
                close_read at org/jruby/ext/socket/RubyBasicSocket.java:451
  chained_2_rescue_line_31 at server.rb:33
                    (root) at server.rb:31
                      each at org/jruby/RubyArray.java:1613
                    (root) at server.rb:22
                      loop at org/jruby/RubyKernel.java:1507
                    (root) at server.rb:10

@headius
Copy link
Member

@headius headius commented Jan 13, 2015

The reproduction looks great, thank you. Will investigate.

@headius
Copy link
Member

@headius headius commented Jan 13, 2015

Five minutes later, I have a fix. Pushing shortly.

@headius
Copy link
Member

@headius headius commented Jan 13, 2015

My fix is on master and I'll be adding some tests to MRI shortly.

@haberbyte
Copy link
Author

@haberbyte haberbyte commented Jan 13, 2015

That's amazing... ❤️

Now that i see what the fix looks like i feel kinda bad that i didn't come up with it myself :-(
Sometimes it's just hard to get started...don't know much about the underlying implementation.

Anyway, thank you, you rock. Achievement unlocked 😉

@headius
Copy link
Member

@headius headius commented Jan 13, 2015

Tip for the future: if there's an unusual error, you can usually pass -Xbacktrace.style=raw to JRuby to get the real Java trace. From there it's often easy to figure it out :-)

Please let me know if there's anything else keeping pow from working on JRuby 9k.

@headius
Copy link
Member

@headius headius commented Jan 13, 2015

Tests added to MRI in ruby/ruby@4eacd5d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants