From 8f4cc73613e5f6953789eb32eb7aba8e26083854 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 21 Dec 2015 11:46:40 +0100 Subject: 484585 - Avoid sending request using a connection that is idle timing out. Rewritten handling of idle timeouts in light of issue #484718. --- .../java/org/eclipse/jetty/client/HttpConnection.java | 6 +----- .../eclipse/jetty/client/http/HttpConnectionOverHTTP.java | 15 +++++---------- .../jetty/client/http/HttpReceiverOverHTTPTest.java | 2 +- .../jetty/fcgi/client/http/HttpConnectionOverFCGI.java | 12 +++++++----- .../java/org/eclipse/jetty/http2/HTTP2Connection.java | 10 +++++----- .../main/java/org/eclipse/jetty/http2/HTTP2Session.java | 13 ++++--------- .../jetty/http2/client/http/HttpConnectionOverHTTP2.java | 1 - 7 files changed, 23 insertions(+), 36 deletions(-) diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java index e382ddd6a8..10887e00e8 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java @@ -220,13 +220,9 @@ public abstract class HttpConnection implements Connection { if (LOG.isDebugEnabled()) LOG.debug("Idle timeout state {} - {}", idleTimeoutState, this); - if (idleTimeoutState.compareAndSet(0, -1)) - close(new TimeoutException("idle_timeout")); - return false; + return idleTimeoutState.compareAndSet(0, -1); } - protected abstract void close(Throwable failure); - @Override public String toString() { diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java index d9fcb24cd8..9e89b5422a 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.client.http; import java.nio.channels.AsynchronousCloseException; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -95,12 +96,12 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements Connec } @Override - protected boolean onReadTimeout() + public boolean onIdleExpired() { boolean close = delegate.onIdleTimeout(); - if (!close && !isClosed()) - fillInterested(); - return close; + if (close) + close(new TimeoutException("Idle timeout " + getEndPoint().getIdleTimeout() + "ms")); + return false; } @Override @@ -206,12 +207,6 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements Connec HttpConnectionOverHTTP.this.close(); } - @Override - protected void close(Throwable failure) - { - HttpConnectionOverHTTP.this.close(failure); - } - @Override public String toString() { diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java index e057cf34f9..e17ce40c66 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java @@ -169,7 +169,7 @@ public class HttpReceiverOverHTTPTest FutureResponseListener listener = (FutureResponseListener)exchange.getResponseListeners().get(0); connection.getHttpChannel().receive(); // Simulate an idle timeout - connection.onReadTimeout(); + connection.onIdleExpired(); try { diff --git a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java index f9da69f3b4..d3c588150c 100644 --- a/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java +++ b/jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java @@ -24,6 +24,7 @@ import java.nio.channels.AsynchronousCloseException; import java.util.LinkedList; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import org.eclipse.jetty.client.HttpClient; @@ -190,12 +191,14 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements Connec } @Override - protected boolean onReadTimeout() + public boolean onIdleExpired() { boolean close = delegate.onIdleTimeout(); - if (!close && !isClosed()) - fillInterested(); - return close; + if (multiplexed) + close &= isFillInterested(); + if (close) + close(new TimeoutException("Idle timeout " + getEndPoint().getIdleTimeout() + "ms")); + return false; } protected void release(HttpChannelOverFCGI channel) @@ -324,7 +327,6 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements Connec HttpConnectionOverFCGI.this.close(); } - @Override protected void close(Throwable failure) { HttpConnectionOverFCGI.this.close(failure); diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java index 13629de0b2..e9b7602ac1 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java @@ -113,12 +113,12 @@ public class HTTP2Connection extends AbstractConnection } @Override - protected boolean onReadTimeout() + public boolean onIdleExpired() { - if (LOG.isDebugEnabled()) - LOG.debug("Idle timeout {}ms expired on {}", getEndPoint().getIdleTimeout(), this); - if (!session.onIdleTimeout()) - fillInterested(); + boolean close = session.onIdleTimeout(); + boolean idle = isFillInterested(); + if (close && idle) + session.close(ErrorCode.NO_ERROR.code, "idle_timeout", Callback.NOOP); return false; } diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java index 89480da632..b57882e977 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java @@ -832,7 +832,7 @@ public abstract class HTTP2Session implements ISession, Parser.Listener * stuck because of TCP congestion), therefore we terminate. * See {@link #onGoAway(GoAwayFrame)}. * - * @return true if the session has been closed, false otherwise + * @return true if the session should be closed, false otherwise * @see #onGoAway(GoAwayFrame) * @see #close(int, String, Callback) * @see #onShutdown() @@ -844,22 +844,17 @@ public abstract class HTTP2Session implements ISession, Parser.Listener { case NOT_CLOSED: { - if (notifyIdleTimeout(this)) - { - close(ErrorCode.NO_ERROR.code, "idle_timeout", Callback.NOOP); - return true; - } - return false; + return notifyIdleTimeout(this); } case LOCALLY_CLOSED: case REMOTELY_CLOSED: { abort(new TimeoutException()); - return true; + return false; } default: { - return true; + return false; } } } diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java index 5c5372f62e..1d3a4c6dbb 100644 --- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java +++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java @@ -76,7 +76,6 @@ public class HttpConnectionOverHTTP2 extends HttpConnection close(new AsynchronousCloseException()); } - @Override protected void close(Throwable failure) { // First close then abort, to be sure that the connection cannot be reused -- cgit v1.2.3