From 0af40c4db2dc01da763f54d7cc02716b02eab218 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sun, 29 Jul 2012 14:57:29 +0200 Subject: [PATCH 1/5] Jetty9 - Added lifecycle callbacks for onOpen() for the application AsyncConnection linked to the SslConnection. --- .../jetty/io/SelectChannelEndPointSslTest.java | 15 ++++++++------- .../org/eclipse/jetty/io/SslConnectionTest.java | 2 +- .../server/ssl/SslSelectChannelConnector.java | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/SelectChannelEndPointSslTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/SelectChannelEndPointSslTest.java index e3715bc7c5c..71f8c8615da 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/SelectChannelEndPointSslTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/SelectChannelEndPointSslTest.java @@ -1,18 +1,11 @@ package org.eclipse.jetty.io; -import static org.hamcrest.Matchers.greaterThan; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; - import java.io.File; import java.io.IOException; import java.io.OutputStream; import java.net.Socket; import java.nio.ByteBuffer; import java.nio.channels.SocketChannel; - import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; import javax.net.ssl.SSLEngineResult.HandshakeStatus; @@ -27,6 +20,12 @@ import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; +import static org.hamcrest.Matchers.greaterThan; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + public class SelectChannelEndPointSslTest extends SelectChannelEndPointTest { @@ -60,6 +59,8 @@ public class SelectChannelEndPointSslTest extends SelectChannelEndPointTest AsyncConnection appConnection = super.newConnection(channel,sslConnection.getSslEndPoint()); sslConnection.getSslEndPoint().setAsyncConnection(appConnection); + _manager.connectionOpened(appConnection); + return sslConnection; } diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java index 93811256c9b..234829fa00c 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java @@ -56,9 +56,9 @@ public class SslConnectionTest AsyncConnection appConnection = new TestConnection(sslConnection.getSslEndPoint()); sslConnection.getSslEndPoint().setAsyncConnection(appConnection); + connectionOpened(appConnection); return sslConnection; - } @Override diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ssl/SslSelectChannelConnector.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ssl/SslSelectChannelConnector.java index 280da352466..fcbac101887 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ssl/SslSelectChannelConnector.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ssl/SslSelectChannelConnector.java @@ -15,7 +15,6 @@ package org.eclipse.jetty.server.ssl; import java.io.IOException; import java.nio.channels.SocketChannel; - import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLSession; @@ -540,6 +539,7 @@ public class SslSelectChannelConnector extends SelectChannelConnector implements SslConnection connection = newSslConnection(endpoint, engine); AsyncConnection delegate = newPlainConnection(channel, connection.getSslEndPoint()); connection.getSslEndPoint().setAsyncConnection(delegate); + getSelectorManager().connectionOpened(delegate); return connection; } catch (IOException e) From 3abb8cfd37c931f9c5717364c875076e73cfd325 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sun, 29 Jul 2012 15:04:27 +0200 Subject: [PATCH 2/5] Jetty9 - Reverted test logging to INFO. --- jetty-spdy/spdy-jetty/src/test/resources/log4j.properties | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/jetty-spdy/spdy-jetty/src/test/resources/log4j.properties b/jetty-spdy/spdy-jetty/src/test/resources/log4j.properties index c7c11c1d807..0459682ca3d 100644 --- a/jetty-spdy/spdy-jetty/src/test/resources/log4j.properties +++ b/jetty-spdy/spdy-jetty/src/test/resources/log4j.properties @@ -12,5 +12,4 @@ log4j.appender.CONSOLE.target=System.err # Level tuning log4j.logger.org.eclipse.jetty=INFO #log4j.logger.org.eclipse.jetty.io=DEBUG -log4j.logger.org.eclipse.jetty.spdy=DEBUG -# thomas +#log4j.logger.org.eclipse.jetty.spdy=DEBUG From fa721bf5104ec9415b43914a0741684d42a54eeb Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sun, 29 Jul 2012 18:03:09 +0200 Subject: [PATCH 3/5] Jetty9 - Making sure that we call EndPoint.close(), and therefore triggering the EndPoint lifecycle callbacks, for exception cases and when the correspondent key is invalid. --- .../java/org/eclipse/jetty/io/EndPoint.java | 3 +- .../jetty/io/SelectChannelEndPoint.java | 6 ++ .../org/eclipse/jetty/io/SelectorManager.java | 97 +++++++------------ 3 files changed, 43 insertions(+), 63 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/EndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/EndPoint.java index 692890fe39c..80713e07e28 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/EndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/EndPoint.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.io; +import java.io.Closeable; import java.io.IOException; import java.net.InetSocketAddress; import java.nio.ByteBuffer; @@ -23,7 +24,7 @@ import java.nio.ByteBuffer; * * A transport EndPoint */ -public interface EndPoint +public interface EndPoint extends Closeable { /* ------------------------------------------------------------ */ /** diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java index 7defd64defe..81d281a3dc2 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java @@ -207,6 +207,12 @@ public class SelectChannelEndPoint extends ChannelEndPoint implements Runnable, catch (CancelledKeyException x) { LOG.debug("Ignoring key update for concurrently closed channel {}", this); + close(); + } + catch (Exception x) + { + LOG.warn("Ignoring key update for " + this, x); + close(); } } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index c1fee7f3206..e043779ec3c 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -13,13 +13,13 @@ package org.eclipse.jetty.io; +import java.io.Closeable; import java.io.IOException; import java.net.ConnectException; import java.net.Socket; import java.net.SocketAddress; import java.nio.channels.CancelledKeyException; import java.nio.channels.ClosedChannelException; -import java.nio.channels.ClosedSelectorException; import java.nio.channels.SelectionKey; import java.nio.channels.Selector; import java.nio.channels.ServerSocketChannel; @@ -336,16 +336,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa _thread.setName(name + " Selector" + _id); LOG.debug("Starting {} on {}", _thread, this); while (isRunning()) - { - try - { - select(); - } - catch (IOException e) - { - LOG.warn(e); - } - } + select(); processChanges(); } finally @@ -358,10 +349,9 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa /** *

Process changes and waits on {@link Selector#select()}.

* - * @throws IOException if the select operation fails * @see #submit(Runnable) */ - public void select() throws IOException + public void select() { boolean debug = LOG.isDebugEnabled(); try @@ -379,32 +369,22 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa Set selectedKeys = _selector.selectedKeys(); for (SelectionKey key : selectedKeys) { - try + if (key.isValid()) { - if (!key.isValid()) - { - if (debug) - LOG.debug("Selector loop ignoring invalid key for channel {}", key.channel()); - continue; - } - processKey(key); } - catch (Exception x) + else { - if (isRunning()) - LOG.warn(x); - else - LOG.debug(x); - - execute(new Close(key)); + if (debug) + LOG.debug("Selector loop ignoring invalid key for channel {}", key.channel()); + Object attachment = key.attachment(); + if (attachment instanceof EndPoint) + ((EndPoint)attachment).close(); } } - - // Everything always handled selectedKeys.clear(); } - catch (ClosedSelectorException x) + catch (IOException x) { if (isRunning()) LOG.warn(x); @@ -429,9 +409,9 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa private void processKey(SelectionKey key) { + Object attachment = key.attachment(); try { - Object attachment = key.attachment(); if (attachment instanceof SelectableAsyncEndPoint) { key.interestOps(0); @@ -458,7 +438,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa catch (Exception x) { connectionFailed(channel, x, attachment); - key.cancel(); + closeNoExceptions(channel); } } else @@ -468,7 +448,27 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa } catch (CancelledKeyException x) { - LOG.debug("Ignoring cancelled key for channel", key.channel()); + LOG.debug("Ignoring cancelled key for channel {}", key.channel()); + if (attachment instanceof EndPoint) + ((EndPoint)attachment).close(); + } + catch (Exception x) + { + LOG.warn("Could not process key for channel " + key.channel(), x); + if (attachment instanceof EndPoint) + ((EndPoint)attachment).close(); + } + } + + private void closeNoExceptions(Closeable closeable) + { + try + { + closeable.close(); + } + catch (IOException x) + { + LOG.ignore(x); } } @@ -643,29 +643,6 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa } } - private class Close implements Runnable - { - private final SelectionKey key; - - private Close(SelectionKey key) - { - this.key = key; - } - - @Override - public void run() - { - try - { - key.channel().close(); - } - catch (IOException x) - { - LOG.ignore(x); - } - } - } - private class Stop implements Runnable { private final CountDownLatch latch = new CountDownLatch(1); @@ -685,11 +662,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa } } - _selector.close(); - } - catch (IOException x) - { - LOG.ignore(x); + closeNoExceptions(_selector); } finally { From c2299154448fc4a5e690fef9165c93cba2921142 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 30 Jul 2012 12:44:48 +0200 Subject: [PATCH 4/5] Jetty9 - Fixed idle timeout expiration, that was not firing if the idle timeout left was zero. Added logging for idle timeout methods. --- .../eclipse/jetty/io/SelectChannelEndPoint.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java index 81d281a3dc2..2bdff94e790 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectChannelEndPoint.java @@ -93,7 +93,16 @@ public class SelectChannelEndPoint extends ChannelEndPoint implements Runnable, private void scheduleIdleTimeout(long delay) { - Future newTimeout = isOpen() && delay > 0 ? _scheduler.schedule(_idleTask, delay, TimeUnit.MILLISECONDS) : null; + Future newTimeout = null; + if (isOpen() && delay > 0) + { + LOG.debug("{} scheduling idle timeout in {} ms", this, delay); + newTimeout = _scheduler.schedule(_idleTask, delay, TimeUnit.MILLISECONDS); + } + else + { + LOG.debug("{} skipped scheduling idle timeout ({} ms)", this, delay); + } Future oldTimeout = _timeout.getAndSet(newTimeout); if (oldTimeout != null) oldTimeout.cancel(false); @@ -145,12 +154,16 @@ public class SelectChannelEndPoint extends ChannelEndPoint implements Runnable, long idleElapsed = System.currentTimeMillis() - idleTimestamp; long idleLeft = idleTimeout - idleElapsed; + LOG.debug("{} idle timeout check, elapsed: {} ms, remaining: {} ms", this, idleElapsed, idleLeft); + if (isOutputShutdown() || _readInterest.isInterested() || _writeFlusher.isWriting()) { if (idleTimestamp != 0 && idleTimeout > 0) { - if (idleLeft < 0) + if (idleLeft <= 0) { + LOG.debug("{} idle timeout expired", this); + if (isOutputShutdown()) close(); notIdle(); From 36729c424886e0544bca345f56d96c3b1c6fbe26 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 30 Jul 2012 12:45:39 +0200 Subject: [PATCH 5/5] Jetty9 - Always returning false from onReadTimeout(). Either we do not have to close, or the goAway() will close after it is written. --- .../main/java/org/eclipse/jetty/spdy/SPDYAsyncConnection.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-spdy/spdy-jetty/src/main/java/org/eclipse/jetty/spdy/SPDYAsyncConnection.java b/jetty-spdy/spdy-jetty/src/main/java/org/eclipse/jetty/spdy/SPDYAsyncConnection.java index 02b21dca42a..39be7454557 100644 --- a/jetty-spdy/spdy-jetty/src/main/java/org/eclipse/jetty/spdy/SPDYAsyncConnection.java +++ b/jetty-spdy/spdy-jetty/src/main/java/org/eclipse/jetty/spdy/SPDYAsyncConnection.java @@ -120,9 +120,9 @@ public class SPDYAsyncConnection extends AbstractAsyncConnection implements Cont @Override protected boolean onReadTimeout() { - if(idle) + if (idle) session.goAway(); - return idle; + return false; } protected Session getSession()