From 910665a55a4ef16ff6cd55a1c1799c20ff76db65 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 21 Nov 2018 17:37:16 +0100 Subject: [PATCH] Fixes #3133 - Logging of key.readyOps() can throw unchecked CancelledKeyException. Introduced safeInterestOps() and safeReadyOps() to catch exceptions they may throw and using them in relevant places to fix the issue. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/io/ChannelEndPoint.java | 23 +++------- .../org/eclipse/jetty/io/ManagedSelector.java | 45 +++++++++++++------ 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ChannelEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ChannelEndPoint.java index 0421176298f..84e80276e28 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ChannelEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ChannelEndPoint.java @@ -31,7 +31,6 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; import org.eclipse.jetty.util.thread.Invocable; -import org.eclipse.jetty.util.thread.Locker; import org.eclipse.jetty.util.thread.Scheduler; /** @@ -426,21 +425,11 @@ public abstract class ChannelEndPoint extends AbstractEndPoint implements Manage public String toEndPointString() { // We do a best effort to print the right toString() and that's it. - try - { - boolean valid = _key != null && _key.isValid(); - int keyInterests = valid ? _key.interestOps() : -1; - int keyReadiness = valid ? _key.readyOps() : -1; - return String.format("%s{io=%d/%d,kio=%d,kro=%d}", - super.toEndPointString(), - _currentInterestOps, - _desiredInterestOps, - keyInterests, - keyReadiness); - } - catch (Throwable x) - { - return String.format("%s{io=%s,kio=-2,kro=-2}", super.toString(), _desiredInterestOps); - } + return String.format("%s{io=%d/%d,kio=%d,kro=%d}", + super.toEndPointString(), + _currentInterestOps, + _desiredInterestOps, + ManagedSelector.safeInterestOps(_key), + ManagedSelector.safeReadyOps(_key)); } } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java index 75e67386bad..b379c60a25c 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ManagedSelector.java @@ -272,6 +272,34 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable } } + static int safeReadyOps(SelectionKey selectionKey) + { + try + { + return selectionKey.readyOps(); + } + catch (Throwable x) + { + if (LOG.isDebugEnabled()) + LOG.debug(x); + return -1; + } + } + + static int safeInterestOps(SelectionKey selectionKey) + { + try + { + return selectionKey.interestOps(); + } + catch (Throwable x) + { + if (LOG.isDebugEnabled()) + LOG.debug(x); + return -1; + } + } + @Override public void dump(Appendable out, String indent) throws IOException { @@ -474,7 +502,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable { Object attachment = key.attachment(); if (LOG.isDebugEnabled()) - LOG.debug("selected {} {} {} ",key.readyOps(),key,attachment); + LOG.debug("selected {} {} {} ", safeReadyOps(key), key, attachment); try { if (attachment instanceof Selectable) @@ -490,7 +518,7 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable } else { - throw new IllegalStateException("key=" + key + ", att=" + attachment + ", iOps=" + key.interestOps() + ", rOps=" + key.readyOps()); + throw new IllegalStateException("key=" + key + ", att=" + attachment + ", iOps=" + safeInterestOps(key) + ", rOps=" + safeReadyOps(key)); } } catch (CancelledKeyException x) @@ -571,19 +599,10 @@ public class ManagedSelector extends ContainerLifeCycle implements Dumpable List list = new ArrayList<>(selector_keys.size()); for (SelectionKey key : selector_keys) { - if (key==null) - continue; - try - { - list.add(String.format("SelectionKey@%x{i=%d}->%s", key.hashCode(), key.interestOps(), key.attachment())); - } - catch (Throwable x) - { - list.add(String.format("SelectionKey@%x[%s]->%s", key.hashCode(), x, key.attachment())); - } + if (key != null) + list.add(String.format("SelectionKey@%x{i=%d}->%s", key.hashCode(), safeInterestOps(key), key.attachment())); } keys = list; - latch.countDown(); }