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

IO.select raises CancelledKeyException on shutdown #1580

Closed
iconara opened this Issue Mar 25, 2014 · 3 comments

Comments

Projects
None yet
3 participants
@iconara
Copy link
Contributor

iconara commented Mar 25, 2014

If I have a thread blocking on IO.select when my tests (running in RSpec) shut down, IO.select raises java.nio.channels.CancelledKeyException and the stack trace below is printed to the console.

I haven't been able to force this to happen when pressing ctrl-C on a running process, and it doesn't happen every time, but often.

I've observed it in JRuby 1.7.10 and 1.7.11.

From looking at the code (https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/util/io/SelectBlob.java#L340) it seems like there are two possibilities on how to end up on line 340, either key.interestOps() or key.readyOps() on line 320 throw CancelledKeyException (I don't see any other calls in the block that can throw the exception) -- but if SelectionKey works the way I think both would throw or neither would throw. Then the first thing that the catch block does is call key.interestOps(), so the answer to the comment above that line is "no" 😄

There's another odd thing in the catch block: errorResults is set only if it is not null, which means that the search that is done a few lines below that could throw an NPE. I think it might be a copy-paste error and should have been == instead of !=. However, because of the problem above I don't think this code is reachable. Could be worth fixing while this bug is fixed, though.

Exception in thread "Ruby-0-Thread-18: /path/to/gems/ione-1.0.0/lib/ione/io/io_reactor.rb:125" java.nio.channels.CancelledKeyException
  at sun.nio.ch.SelectionKeyImpl.ensureValid(SelectionKeyImpl.java:73)
  at sun.nio.ch.SelectionKeyImpl.interestOps(SelectionKeyImpl.java:77)
  at org.jruby.util.io.SelectBlob.processSelectedKeys(SelectBlob.java:340)
  at org.jruby.util.io.SelectBlob.goForIt(SelectBlob.java:90)
  at org.jruby.RubyIO.select_static(RubyIO.java:3677)
  at org.jruby.RubyIO.select(RubyIO.java:3673)
  at org.jruby.RubyIO$INVOKER$s$0$3$select.call(RubyIO$INVOKER$s$0$3$select.gen)
  at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:70)
  at rubyjit.Ione::Io::IoLoopBody$$check_sockets!_d11515667503f3afdc971c3d423944f53ce3fe591539547357.__file__(/path/to/gems/ione-1.0.0/lib/ione/io/io_reactor.rb:307)
  at rubyjit.Ione::Io::IoLoopBody$$check_sockets!_d11515667503f3afdc971c3d423944f53ce3fe591539547357.__file__(/path/to/gems/ione-1.0.0/lib/ione/io/io_reactor.rb)
  at org.jruby.internal.runtime.methods.JittedMethod.call(JittedMethod.java:181)
  at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:168)
  at rubyjit.Ione::Io::IoLoopBody$$tick_275631e3d5785037b81518b0f3ad2a325dff51f01539547357.__file__(/path/to/gems/ione-1.0.0/lib/ione/io/io_reactor.rb:288)
  at rubyjit.Ione::Io::IoLoopBody$$tick_275631e3d5785037b81518b0f3ad2a325dff51f01539547357.__file__(/path/to/gems/ione-1.0.0/lib/ione/io/io_reactor.rb)
  at org.jruby.ast.executable.AbstractScript.__file__(AbstractScript.java:38)
  at org.jruby.internal.runtime.methods.JittedMethod.call(JittedMethod.java:141)
  at org.jruby.runtime.callsite.CachingCallSite.call(CachingCallSite.java:134)
  at org.jruby.ast.CallNoArgNode.interpret(CallNoArgNode.java:60)
  at org.jruby.ast.UntilNode.interpret(UntilNode.java:120)
  at org.jruby.ast.NewlineNode.interpret(NewlineNode.java:105)
  at org.jruby.ast.EnsureNode.interpret(EnsureNode.java:96)
  at org.jruby.ast.BeginNode.interpret(BeginNode.java:83)
  at org.jruby.ast.NewlineNode.interpret(NewlineNode.java:105)
  at org.jruby.ast.BlockNode.interpret(BlockNode.java:71)
  at org.jruby.evaluator.ASTInterpreter.INTERPRET_BLOCK(ASTInterpreter.java:112)
  at org.jruby.runtime.Interpreted19Block.evalBlockBody(Interpreted19Block.java:206)
  at org.jruby.runtime.Interpreted19Block.yield(Interpreted19Block.java:194)
  at org.jruby.runtime.Interpreted19Block.call(Interpreted19Block.java:125)
  at org.jruby.runtime.Block.call(Block.java:101)
  at org.jruby.RubyProc.call(RubyProc.java:290)
  at org.jruby.RubyProc.call(RubyProc.java:228)
  at org.jruby.internal.runtime.RubyRunnable.run(RubyRunnable.java:99)
  at java.lang.Thread.run(Thread.java:744)
@iconara

This comment has been minimized.

Copy link
Contributor Author

iconara commented Mar 25, 2014

If someone could point me to which tests cover this code I could take a stab at fixing it.

@iconara

This comment has been minimized.

Copy link
Contributor Author

iconara commented Mar 25, 2014

Here's a test case, of sorts, that stresses IO.select until the error happens:

queue = java.util.concurrent.ConcurrentLinkedQueue.new

t = Thread.start do
  begin
    while true
      begin
        r, w = IO.pipe
        queue.add([r, w])
        IO.select([r], nil, [r], 0.1)
      rescue Errno::EBADF
      end
    end
  rescue java.nio.channels.CancelledKeyException
    puts "THIS SHOULD NOT HAPPEN"
  end
end

while t.alive?
  pair = queue.poll
  if pair
    r, w = pair
    w.syswrite('x')
    r.to_java.channel.close
  end
end

puts 'FAIL!'

I'm looking at the code and am trying to figure out what it is supposed to do when a CKE is thrown, I'm not sure if there's any cleanup that absolutely must be done. The easiest thing would be to raise a Errno::EBADF and just bail, but I'd like to do the right thing.

iconara added a commit to iconara/jruby that referenced this issue Mar 25, 2014

Handle CancelledKeyException in IO.select
When a CancelledKeyException is raised in IO.select/SelectBlob#processSelectedKeys it caught, but the catch block causes another one to be raised immediately. This patch removes the catch block completely (because it's never run, so nothing can rely on what it does) and raises an Errno::EBADF from SelectBlob#goForIt.

CancelledKeyException is raised when the underlying channel has closed, which is normally handled. There is however a race condition in the channel selection that means that you can get a cancelled key from select if the channel is closed just after the selector determines that it is ready to read or write.

It's unclear what the correct way of handling this really is, but raising EBADF feels reasonable, the channel has been closed and so it's no longer a valid file descriptor.

This closes jruby#1580

@headius headius closed this in a38a622 Jul 27, 2014

@headius

This comment has been minimized.

Copy link
Member

headius commented Jul 27, 2014

Fixed by #1581. I also confirmed that while it does still print FAIL on master (which has new select logic in SelectExecutor) it only does so because it runs out of file descriptors.

@headius headius added this to the JRuby 1.7.14 milestone Jul 27, 2014

@enebo enebo modified the milestones: JRuby 1.7.14, JRuby 1.7.15 Aug 27, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.