Jetty 12 idletimeout (#9905)

* IdleTimeout review

 + pass TimeoutException through all APIs
 + HttpConnection now passes on TimeoutException to HttpChannel.onFailure
* More ServerTests for idletimeout

* Recreated a ServerTimeoutsTest for multiple transports

* more robust tests

Signed-off-by: Simone Bordet <[email protected]>

* merged work from @sbordet and @gregw

* Various improvements to CyclicTimeouts.
* Improved reset of the earliest timeout before iteration.
* Removed check for getExpireNanoTime() == -1, since it's a valid value.
* When onExpired(Expirable) returns false, the Expirable should arrange to move its timeout in the future.

* fix keystore to please BoringSSL + use correct temp path

Signed-off-by: Ludovic Orban <[email protected]>

* Fixed ErrorResponseAndCallback succeeded() and failed() to call super.failed() in all cases to complete the wrapped callback.

Signed-off-by: Simone Bordet <[email protected]>

* Revert "Fixed ErrorResponseAndCallback succeeded() and failed() to call super.failed() in all cases to complete the wrapped callback."

This reverts commit 5ac57c1.

* WIP idleTimeout

* WIP idleTimeout

* Added context wrapper for idle timeout listener

* updates from review

---------

Signed-off-by: Simone Bordet <[email protected]>
Signed-off-by: Ludovic Orban <[email protected]>
Co-authored-by: Simone Bordet <[email protected]>
Co-authored-by: Ludovic Orban <[email protected]>

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks4Proxy.java

@@ -150,9 +150,9 @@ public void failed(Throwable x)
         }
 
         @Override
-        public boolean onIdleExpired()
+        public boolean onIdleExpired(TimeoutException timeout)
         {
-            failed(new TimeoutException("Idle timeout expired"));
+            failed(timeout);
             return false;
         }
 

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/Socks5Proxy.java

@@ -185,9 +185,9 @@ private void fail(Throwable x)
         }
 
         @Override
-        public boolean onIdleExpired()
+        public boolean onIdleExpired(TimeoutException timeout)
         {
-            fail(new TimeoutException("Idle timeout expired"));
+            fail(timeout);
             return false;
         }
 

jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/transport/internal/HttpConnectionOverHTTP.java

@@ -173,12 +173,12 @@ public Object getAttachment()
     }
 
     @Override
-    public boolean onIdleExpired()
+    public boolean onIdleExpired(TimeoutException timeout)
     {
         long idleTimeout = getEndPoint().getIdleTimeout();
         boolean close = onIdleTimeout(idleTimeout);
         if (close)
-            close(new TimeoutException("Idle timeout " + idleTimeout + " ms"));
+            close(timeout);
         return false;
     }
 

jetty-core/jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTPTest.java

@@ -45,6 +45,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
@@ -186,10 +187,12 @@ public void testReceiveResponseContentIdleTimeout(HttpCompliance compliance) thr
         // ByteArrayEndPoint has an idle timeout of 0 by default,
         // so to simulate an idle timeout is enough to wait a bit.
         Thread.sleep(100);
-        connection.onIdleExpired();
+        TimeoutException timeoutException = new TimeoutException();
+        connection.onIdleExpired(timeoutException);
 
         ExecutionException e = assertThrows(ExecutionException.class, () -> listener.get(5, TimeUnit.SECONDS));
         assertThat(e.getCause(), instanceOf(TimeoutException.class));
+        assertThat(e.getCause(), sameInstance(timeoutException));
     }
 
     @ParameterizedTest

jetty-core/jetty-fcgi/jetty-fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/transport/internal/HttpConnectionOverFCGI.java

@@ -217,13 +217,12 @@ private void shutdown()
     }
 
     @Override
-    public boolean onIdleExpired()
+    public boolean onIdleExpired(TimeoutException timeoutException)
     {
         long idleTimeout = getEndPoint().getIdleTimeout();
-        TimeoutException failure = new TimeoutException("Idle timeout " + idleTimeout + " ms");
-        boolean close = delegate.onIdleTimeout(idleTimeout, failure);
+        boolean close = delegate.onIdleTimeout(idleTimeout, timeoutException);
         if (close)
-            close(failure);
+            close(timeoutException);
         return false;
     }
 

jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/HttpStreamOverFCGI.java

@@ -15,6 +15,7 @@
 
 import java.nio.ByteBuffer;
 import java.util.Locale;
+import java.util.concurrent.TimeoutException;
 
 import org.eclipse.jetty.fcgi.FCGI;
 import org.eclipse.jetty.fcgi.generator.Flusher;
@@ -301,6 +302,18 @@ private void generateResponseContent(ByteBufferPool.Accumulator accumulator, boo
         _generator.generateResponseContent(accumulator, _id, buffer, last, _aborted);
     }
 
+    @Override
+    public long getIdleTimeout()
+    {
+        return _connection.getEndPoint().getIdleTimeout();
+    }
+
+    @Override
+    public void setIdleTimeout(long idleTimeoutMs)
+    {
+        _connection.getEndPoint().setIdleTimeout(idleTimeoutMs);
+    }
+
     @Override
     public boolean isCommitted()
     {
@@ -328,9 +341,9 @@ public void failed(Throwable x)
         _connection.onCompleted(x);
     }
 
-    public boolean onIdleTimeout(Throwable timeout)
+    public boolean onIdleTimeout(TimeoutException timeout)
     {
-        Runnable task = _httpChannel.onFailure(timeout);
+        Runnable task = _httpChannel.onIdleTimeout(timeout);
         if (task != null)
             execute(task);
         return false;

jetty-core/jetty-fcgi/jetty-fcgi-server/src/main/java/org/eclipse/jetty/fcgi/server/internal/ServerFCGIConnection.java

@@ -16,6 +16,7 @@
 import java.net.SocketAddress;
 import java.nio.ByteBuffer;
 import java.util.Set;
+import java.util.concurrent.TimeoutException;
 
 import org.eclipse.jetty.fcgi.FCGI;
 import org.eclipse.jetty.fcgi.generator.Flusher;
@@ -291,7 +292,7 @@ private int fillInputBuffer()
     }
 
     @Override
-    protected boolean onReadTimeout(Throwable timeout)
+    protected boolean onReadTimeout(TimeoutException timeout)
     {
         if (stream != null)
             return stream.onIdleTimeout(timeout);
@@ -323,6 +324,15 @@ void onCompleted(Throwable failure)
             getFlusher().shutdown();
     }
 
+    @Override
+    public boolean onIdleExpired(TimeoutException timeoutException)
+    {
+        Runnable task = stream.getHttpChannel().onIdleTimeout(timeoutException);
+        if (task != null)
+            getExecutor().execute(task);
+        return false;
+    }
+
     private class ServerListener implements ServerParser.Listener
     {
         @Override

jetty-core/jetty-fcgi/jetty-fcgi-server/src/test/java/org/eclipse/jetty/fcgi/server/HttpClientTest.java

@@ -522,6 +522,8 @@ public void testConnectionIdleTimeout() throws Exception
             @Override
             public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jetty.server.Response response, Callback callback) throws Exception
             {
+                // Handler says it will handle the idletimeout
+                request.addIdleTimeoutListener(t -> false);
                 TimeUnit.MILLISECONDS.sleep(2 * idleTimeout);
                 callback.succeeded();
                 return true;
@@ -530,7 +532,7 @@ public boolean handle(org.eclipse.jetty.server.Request request, org.eclipse.jett
 
         connector.setIdleTimeout(idleTimeout);
 
-        // Request does not fail because idle timeouts while dispatched are ignored.
+        // Request does not fail because handler says it will handle it.
         ContentResponse response1 = client.newRequest("localhost", connector.getLocalPort())
             .scheme(scheme)
             .idleTimeout(4 * idleTimeout, TimeUnit.MILLISECONDS)

jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/ClientHTTP2StreamEndPoint.java

@@ -13,6 +13,8 @@
 
 package org.eclipse.jetty.http2.client.transport.internal;
 
+import java.util.concurrent.TimeoutException;
+
 import org.eclipse.jetty.http2.HTTP2Channel;
 import org.eclipse.jetty.http2.HTTP2Stream;
 import org.eclipse.jetty.http2.HTTP2StreamEndPoint;
@@ -38,13 +40,13 @@ public void onDataAvailable()
     }
 
     @Override
-    public void onTimeout(Throwable failure, Promise<Boolean> promise)
+    public void onTimeout(TimeoutException timeout, Promise<Boolean> promise)
     {
         if (LOG.isDebugEnabled())
-            LOG.debug("idle timeout on {}: {}", this, failure);
+            LOG.debug("idle timeout on {}", this, timeout);
         Connection connection = getConnection();
         if (connection != null)
-            promise.succeeded(connection.onIdleExpired());
+            promise.succeeded(connection.onIdleExpired(timeout));
         else
             promise.succeeded(true);
     }

jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpChannelOverHTTP2.java

@@ -13,6 +13,8 @@
 
 package org.eclipse.jetty.http2.client.transport.internal;
 
+import java.util.concurrent.TimeoutException;
+
 import org.eclipse.jetty.client.Result;
 import org.eclipse.jetty.client.transport.HttpChannel;
 import org.eclipse.jetty.client.transport.HttpExchange;
@@ -200,7 +202,7 @@ public void onReset(Stream stream, ResetFrame frame, Callback callback)
         }
 
         @Override
-        public void onIdleTimeout(Stream stream, Throwable x, Promise<Boolean> promise)
+        public void onIdleTimeout(Stream stream, TimeoutException x, Promise<Boolean> promise)
         {
             HTTP2Channel.Client channel = (HTTP2Channel.Client)((HTTP2Stream)stream).getAttachment();
             channel.onTimeout(x, promise);

jetty-core/jetty-http2/jetty-http2-client-transport/src/main/java/org/eclipse/jetty/http2/client/transport/internal/HttpReceiverOverHTTP2.java

@@ -14,6 +14,7 @@
 package org.eclipse.jetty.http2.client.transport.internal;
 
 import java.io.IOException;
+import java.util.concurrent.TimeoutException;
 import java.util.function.BiFunction;
 
 import org.eclipse.jetty.client.HttpUpgrader;
@@ -220,7 +221,7 @@ void onReset(ResetFrame frame)
     }
 
     @Override
-    public void onTimeout(Throwable failure, Promise<Boolean> promise)
+    public void onTimeout(TimeoutException failure, Promise<Boolean> promise)
     {
         HttpExchange exchange = getHttpExchange();
         if (exchange != null)

jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Channel.java

@@ -13,6 +13,7 @@
 
 package org.eclipse.jetty.http2;
 
+import java.util.concurrent.TimeoutException;
 import java.util.function.BiConsumer;
 
 import org.eclipse.jetty.http2.frames.HeadersFrame;
@@ -34,7 +35,7 @@ public interface Client
     {
         public void onDataAvailable();
 
-        public void onTimeout(Throwable failure, Promise<Boolean> promise);
+        public void onTimeout(TimeoutException failure, Promise<Boolean> promise);
 
         public void onFailure(Throwable failure, Callback callback);
     }
@@ -54,7 +55,7 @@ public interface Server
         // TODO: review the signature because the serialization done by HttpChannel.onError()
         //  is now failing the callback which fails the HttpStream, which should decide whether
         //  to reset the HTTP/2 stream, so we may not need the boolean return type.
-        public void onTimeout(Throwable failure, BiConsumer<Runnable, Boolean> consumer);
+        public void onTimeout(TimeoutException timeout, BiConsumer<Runnable, Boolean> consumer);
 
         // TODO: can it be simplified? The callback seems to only be succeeded, which
         //  means it can be converted into a Runnable which may just be the return type

jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java

@@ -18,6 +18,7 @@
 import java.util.ArrayDeque;
 import java.util.Queue;
 import java.util.concurrent.Executor;
+import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicLong;
 
 import org.eclipse.jetty.http2.api.Stream;
@@ -174,7 +175,7 @@ private int fill(EndPoint endPoint, ByteBuffer buffer)
     }
 
     @Override
-    public boolean onIdleExpired()
+    public boolean onIdleExpired(TimeoutException timeoutException)
     {
         boolean idle = isFillInterested();
         if (idle)

jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java

@@ -839,7 +839,7 @@ private void notifyReset(Stream stream, ResetFrame frame, Callback callback)
         }
     }
 
-    private void notifyIdleTimeout(Stream stream, Throwable failure, Promise<Boolean> promise)
+    private void notifyIdleTimeout(Stream stream, TimeoutException failure, Promise<Boolean> promise)
     {
         Listener listener = this.listener;
         if (listener != null)

jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java

@@ -14,6 +14,7 @@
 package org.eclipse.jetty.http2.api;
 
 import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeoutException;
 
 import org.eclipse.jetty.http2.frames.DataFrame;
 import org.eclipse.jetty.http2.frames.HeadersFrame;
@@ -218,7 +219,7 @@ public default CompletableFuture<Void> reset(ResetFrame frame)
     /**
      * @param idleTimeout the stream idle timeout
      * @see #getIdleTimeout()
-     * @see Stream.Listener#onIdleTimeout(Stream, Throwable, Promise)
+     * @see Stream.Listener#onIdleTimeout(Stream, TimeoutException, Promise)
      */
     public void setIdleTimeout(long idleTimeout);
 
@@ -369,7 +370,7 @@ public default void onReset(Stream stream, ResetFrame frame, Callback callback)
          * @param promise the promise to complete
          * @see #getIdleTimeout()
          */
-        public default void onIdleTimeout(Stream stream, Throwable x, Promise<Boolean> promise)
+        public default void onIdleTimeout(Stream stream, TimeoutException x, Promise<Boolean> promise)
         {
             promise.succeeded(true);
         }

jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java

@@ -168,7 +168,7 @@ private void onFailure(Stream stream, Throwable failure, Callback callback)
         }
 
         @Override
-        public void onIdleTimeout(Stream stream, Throwable x, Promise<Boolean> promise)
+        public void onIdleTimeout(Stream stream, TimeoutException x, Promise<Boolean> promise)
         {
             getConnection().onStreamTimeout(stream, x, promise);
         }

jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HTTP2ServerConnection.java

@@ -19,6 +19,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.TimeoutException;
 
 import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.HttpField;
@@ -155,14 +156,14 @@ public void onTrailers(Stream stream, HeadersFrame frame)
         }
     }
 
-    public void onStreamTimeout(Stream stream, Throwable failure, Promise<Boolean> promise)
+    public void onStreamTimeout(Stream stream, TimeoutException timeout, Promise<Boolean> promise)
     {
         if (LOG.isDebugEnabled())
-            LOG.debug("Idle timeout on {}", stream, failure);
+            LOG.debug("Idle timeout on {}", stream, timeout);
         HTTP2Channel.Server channel = (HTTP2Channel.Server)((HTTP2Stream)stream).getAttachment();
         if (channel != null)
         {
-            channel.onTimeout(failure, (task, timedOut) ->
+            channel.onTimeout(timeout, (task, timedOut) ->
             {
                 if (task != null)
                     offerTask(task, true);

jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java

@@ -14,6 +14,7 @@
 package org.eclipse.jetty.http2.server.internal;
 
 import java.nio.ByteBuffer;
+import java.util.concurrent.TimeoutException;
 import java.util.function.BiConsumer;
 import java.util.function.Supplier;
 
@@ -451,6 +452,18 @@ private boolean isTunnel(MetaData.Request request, MetaData.Response response)
         return MetaData.isTunnel(request.getMethod(), response.getStatus());
     }
 
+    @Override
+    public long getIdleTimeout()
+    {
+        return _stream.getIdleTimeout();
+    }
+
+    @Override
+    public void setIdleTimeout(long idleTimeoutMs)
+    {
+        _stream.setIdleTimeout(idleTimeoutMs);
+    }
+
     @Override
     public void push(MetaData.Request resource)
     {
@@ -558,9 +571,9 @@ public Throwable consumeAvailable()
     }
 
     @Override
-    public void onTimeout(Throwable failure, BiConsumer<Runnable, Boolean> consumer)
+    public void onTimeout(TimeoutException timeout, BiConsumer<Runnable, Boolean> consumer)
     {
-        Runnable task = _httpChannel.onFailure(failure);
+        Runnable task = _httpChannel.onIdleTimeout(timeout);
         boolean idle = !_httpChannel.isRequestHandled();
         consumer.accept(task, idle);
     }