From 35c4c240997fb0d6c1f2abf8429b7dd50a4af739 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Sat, 19 Dec 2015 12:01:40 +0100 Subject: [PATCH] 484718 - Review idle timeout handling. Introduced Connection.onIdleExpired(). --- .../eclipse/jetty/io/AbstractConnection.java | 6 +++ .../eclipse/jetty/io/AbstractEndPoint.java | 4 ++ .../java/org/eclipse/jetty/io/Connection.java | 13 ++++++ .../eclipse/jetty/io/ssl/SslConnection.java | 42 ++++++++++++------- 4 files changed, 50 insertions(+), 15 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java index cf83f1c7505..e86d17eb09f 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java @@ -220,6 +220,12 @@ public abstract class AbstractConnection implements Connection getEndPoint().close(); } + @Override + public boolean onIdleExpired() + { + return true; + } + @Override public int getMessagesIn() { diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java index 5b49c8e44ca..6b49e4877c8 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractEndPoint.java @@ -158,6 +158,10 @@ public abstract class AbstractEndPoint extends IdleTimeout implements EndPoint @Override protected void onIdleExpired(TimeoutException timeout) { + Connection connection = _connection; + if (connection != null && !_connection.onIdleExpired()) + return; + boolean output_shutdown=isOutputShutdown(); boolean input_shutdown=isInputShutdown(); boolean fillFailed = _fillInterest.onFail(timeout); diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java index cc53f9cc1f7..be274e910ac 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/Connection.java @@ -73,6 +73,19 @@ public interface Connection extends Closeable @Override public void close(); + /** + *

Callback method invoked upon an idle timeout event.

+ *

Implementations of this method may return true to indicate that the idle timeout + * handling should proceed normally, typically failing the EndPoint and causing it to + * be closed.

+ *

When false is returned, the handling of the idle timeout event is halted + * immediately and the EndPoint left in the state it was before the idle timeout event.

+ * + * @return true to let the EndPoint handle the idle timeout, + * false to tell the EndPoint to halt the handling of the idle timeout. + */ + public boolean onIdleExpired(); + public int getMessagesIn(); public int getMessagesOut(); public long getBytesIn(); diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index ca32841446e..6b34201ad85 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -171,6 +171,12 @@ public class SslConnection extends AbstractConnection getDecryptedEndPoint().getConnection().close(); } + @Override + public boolean onIdleExpired() + { + return getDecryptedEndPoint().getConnection().onIdleExpired(); + } + @Override public void onFillable() { @@ -328,17 +334,29 @@ public class SslConnection extends AbstractConnection public DecryptedEndPoint() { - super(null,getEndPoint().getLocalAddress(), getEndPoint().getRemoteAddress()); - setIdleTimeout(getEndPoint().getIdleTimeout()); + // Disable idle timeout checking: no scheduler and -1 timeout for this instance. + super(null, getEndPoint().getLocalAddress(), getEndPoint().getRemoteAddress()); + super.setIdleTimeout(-1); + } + + @Override + public long getIdleTimeout() + { + return getEndPoint().getIdleTimeout(); } @Override public void setIdleTimeout(long idleTimeout) { - super.setIdleTimeout(idleTimeout); getEndPoint().setIdleTimeout(idleTimeout); } + @Override + public boolean isOpen() + { + return getEndPoint().isOpen(); + } + @Override protected WriteFlusher getWriteFlusher() { @@ -443,10 +461,10 @@ public class SslConnection extends AbstractConnection } } } - + if (fillable) getExecutor().execute(_runFillable); - else + else ensureFillInterested(); } } @@ -710,13 +728,13 @@ public class SslConnection extends AbstractConnection // will return 0 (even if some handshake bytes were flushed and filled). // it is the applications responsibility to call flush again - either in a busy loop // or better yet by using EndPoint#write to do the flushing. - + if (DEBUG) { for (ByteBuffer b : appOuts) LOG.debug("{} flush {}", SslConnection.this, BufferUtil.toHexSummary(b)); } - + try { if (_cannotAcceptMoreAppDataToFlush) @@ -746,7 +764,7 @@ public class SslConnection extends AbstractConnection } if (DEBUG) LOG.debug("{} wrap {}", SslConnection.this, wrapResult.toString().replace('\n',' ')); - + Status wrapResultStatus = wrapResult.getStatus(); boolean allConsumed=true; @@ -906,7 +924,7 @@ public class SslConnection extends AbstractConnection if (!SslConnection.this.isFillInterested()) SslConnection.this.fillInterested(); } - + @Override public boolean isOutputShutdown() { @@ -922,12 +940,6 @@ public class SslConnection extends AbstractConnection super.close(); } - @Override - public boolean isOpen() - { - return getEndPoint().isOpen(); - } - @Override public Object getTransport() {