From 21a4172ab6cbbdbb1f89664d51e1553f04e5b37e Mon Sep 17 00:00:00 2001 From: Tiago Cardoso Date: Tue, 7 Nov 2017 13:17:33 +0100 Subject: [PATCH 1/5] preventing CancelledKeyException, which randomly happens when selector selects an already closed key --- ext/nio4r/org/nio4r/Selector.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/ext/nio4r/org/nio4r/Selector.java b/ext/nio4r/org/nio4r/Selector.java index 88e91b0..b959b16 100644 --- a/ext/nio4r/org/nio4r/Selector.java +++ b/ext/nio4r/org/nio4r/Selector.java @@ -7,6 +7,7 @@ import java.nio.channels.Channel; import java.nio.channels.SelectableChannel; import java.nio.channels.SelectionKey; +import java.nio.channels.CancelledKeyException; import org.jruby.Ruby; import org.jruby.RubyArray; @@ -207,7 +208,14 @@ public synchronized IRubyObject select(ThreadContext context, IRubyObject timeou Iterator selectedKeys = this.selector.selectedKeys().iterator(); while(selectedKeys.hasNext()) { SelectionKey key = (SelectionKey)selectedKeys.next(); - processKey(key); + try { + processKey(key); + } catch(CancelledKeyException ie) { + continue; + // TODO: what to do? + //throw runtime.newIOError(ie.getLocalizedMessage()); + } + selectedKeys.remove(); if(block.isGiven()) { From 6d2534e513c4c4b631095039467d7cf0a1ab2762 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 10 Nov 2017 10:59:49 +0100 Subject: [PATCH 2/5] reverted changes --- ext/nio4r/org/nio4r/Selector.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/ext/nio4r/org/nio4r/Selector.java b/ext/nio4r/org/nio4r/Selector.java index b959b16..86a3035 100644 --- a/ext/nio4r/org/nio4r/Selector.java +++ b/ext/nio4r/org/nio4r/Selector.java @@ -208,13 +208,7 @@ public synchronized IRubyObject select(ThreadContext context, IRubyObject timeou Iterator selectedKeys = this.selector.selectedKeys().iterator(); while(selectedKeys.hasNext()) { SelectionKey key = (SelectionKey)selectedKeys.next(); - try { - processKey(key); - } catch(CancelledKeyException ie) { - continue; - // TODO: what to do? - //throw runtime.newIOError(ie.getLocalizedMessage()); - } + processKey(key); selectedKeys.remove(); From c65706845e8e5f6e6a6ffef5b00819db93ebdd7e Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 10 Nov 2017 11:09:46 +0100 Subject: [PATCH 3/5] added first draft of test --- spec/support/selectable_examples.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/support/selectable_examples.rb b/spec/support/selectable_examples.rb index 81c9cd3..2d1c638 100644 --- a/spec/support/selectable_examples.rb +++ b/spec/support/selectable_examples.rb @@ -54,4 +54,14 @@ expect(m.readiness).to eq(:rw) end end + it "selects as readable if selectable has been closed" do + selector.register(readable_subject, :rw) + selector.select(0) do |m| + expect(m.readiness).to eq(:rw) + end + readable_subject.close + selector.select(0) do |m| + expect(m.readiness).to eq(:r) + end + end end From cc53b970aa6880e0f9d28717d5a0e4eaa3bc9d8a Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Fri, 10 Nov 2017 11:32:38 +0100 Subject: [PATCH 4/5] test inconsistency of readiness when the selectable has been closed --- spec/support/selectable_examples.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/spec/support/selectable_examples.rb b/spec/support/selectable_examples.rb index 2d1c638..715f16a 100644 --- a/spec/support/selectable_examples.rb +++ b/spec/support/selectable_examples.rb @@ -54,14 +54,12 @@ expect(m.readiness).to eq(:rw) end end - it "selects as readable if selectable has been closed" do + it "keeps readiness after the selectable has been closed" do selector.register(readable_subject, :rw) selector.select(0) do |m| expect(m.readiness).to eq(:rw) - end - readable_subject.close - selector.select(0) do |m| - expect(m.readiness).to eq(:r) + readable_subject.close + expect(m.readiness).to eq(:rw) end end end From 01ab970220407eb58e96137713852995ecc3d385 Mon Sep 17 00:00:00 2001 From: HoneyryderChuck Date: Tue, 14 Nov 2017 18:13:38 +0000 Subject: [PATCH 5/5] call SelectionKey#isValid before readyOps, to prevent CancelledKeyException --- ext/nio4r/org/nio4r/Monitor.java | 6 ++++++ ext/nio4r/org/nio4r/Selector.java | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/ext/nio4r/org/nio4r/Monitor.java b/ext/nio4r/org/nio4r/Monitor.java index e903520..9565702 100644 --- a/ext/nio4r/org/nio4r/Monitor.java +++ b/ext/nio4r/org/nio4r/Monitor.java @@ -102,12 +102,16 @@ public IRubyObject removeInterest(ThreadContext context, IRubyObject interest) { @JRubyMethod public IRubyObject readiness(ThreadContext context) { + if(!key.isValid()) + return this.interests; return Nio4r.interestOpsToSymbol(context.getRuntime(), key.readyOps()); } @JRubyMethod(name = "readable?") public IRubyObject isReadable(ThreadContext context) { Ruby runtime = context.getRuntime(); + if (!this.key.isValid()) + return runtime.getTrue(); int readyOps = this.key.readyOps(); if((readyOps & SelectionKey.OP_READ) != 0 || (readyOps & SelectionKey.OP_ACCEPT) != 0) { @@ -120,6 +124,8 @@ public IRubyObject isReadable(ThreadContext context) { @JRubyMethod(name = {"writable?", "writeable?"}) public IRubyObject writable(ThreadContext context) { Ruby runtime = context.getRuntime(); + if (!this.key.isValid()) + return runtime.getTrue(); int readyOps = this.key.readyOps(); if((readyOps & SelectionKey.OP_WRITE) != 0 || (readyOps & SelectionKey.OP_CONNECT) != 0) { diff --git a/ext/nio4r/org/nio4r/Selector.java b/ext/nio4r/org/nio4r/Selector.java index 86a3035..d373d0c 100644 --- a/ext/nio4r/org/nio4r/Selector.java +++ b/ext/nio4r/org/nio4r/Selector.java @@ -271,7 +271,7 @@ private void cancelKeys() { // Remove connect interest from connected sockets // See: http://stackoverflow.com/questions/204186/java-nio-select-returns-without-selected-keys-why private void processKey(SelectionKey key) { - if((key.readyOps() & SelectionKey.OP_CONNECT) != 0) { + if(key.isValid() && (key.readyOps() & SelectionKey.OP_CONNECT) != 0) { int interestOps = key.interestOps(); interestOps &= ~SelectionKey.OP_CONNECT;