From 86586df0a8a4d9c6b5af9a621ad1adf1b494d39b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Mon, 15 Jan 2024 22:00:57 +0100 Subject: [PATCH] Issue #11259 - HTTP/2 connection not closed after idle timeout when TCP congested. (#11268) * Backport of #11267. Signed-off-by: Simone Bordet --- .../jetty/http2/client/IdleTimeoutTest.java | 56 +++++++++++++++++++ .../org/eclipse/jetty/http2/HTTP2Session.java | 14 ++++- 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/IdleTimeoutTest.java b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/IdleTimeoutTest.java index 3871b3251fc1..5e65cbbf5e41 100644 --- a/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/IdleTimeoutTest.java +++ b/jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/IdleTimeoutTest.java @@ -19,7 +19,11 @@ package org.eclipse.jetty.http2.client; import java.io.IOException; +import java.net.InetSocketAddress; import java.nio.ByteBuffer; +import java.nio.channels.SelectionKey; +import java.nio.channels.SocketChannel; +import java.time.Duration; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -43,7 +47,10 @@ import org.eclipse.jetty.http2.frames.GoAwayFrame; import org.eclipse.jetty.http2.frames.HeadersFrame; import org.eclipse.jetty.http2.frames.ResetFrame; +import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory; import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory; +import org.eclipse.jetty.io.ManagedSelector; +import org.eclipse.jetty.io.SocketChannelEndPoint; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; @@ -57,7 +64,9 @@ import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; +import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -681,6 +690,53 @@ public void onReset(Stream stream, ResetFrame frame) assertThat(((ISession)client).updateSendWindow(0), Matchers.greaterThan(0)); } + @Test + public void testIdleTimeoutWhenCongested() throws Exception + { + long idleTimeout = 1000; + HTTP2CServerConnectionFactory h2c = new HTTP2CServerConnectionFactory(new HttpConfiguration()); + prepareServer(h2c); + server.removeConnector(connector); + connector = new ServerConnector(server, 1, 1, h2c) + { + @Override + protected SocketChannelEndPoint newEndPoint(SocketChannel channel, ManagedSelector selectSet, SelectionKey key) + { + SocketChannelEndPoint endpoint = new SocketChannelEndPoint(channel, selectSet, key, getScheduler()) + { + @Override + public boolean flush(ByteBuffer... buffers) + { + // Fake TCP congestion. + return false; + } + + @Override + protected void onIncompleteFlush() + { + // Do nothing here to avoid spin loop, + // since the network is actually writable, + // as we are only faking TCP congestion. + } + }; + endpoint.setIdleTimeout(getIdleTimeout()); + return endpoint; + } + }; + connector.setIdleTimeout(idleTimeout); + server.addConnector(connector); + server.start(); + + prepareClient(); + client.start(); + + InetSocketAddress address = new InetSocketAddress("localhost", connector.getLocalPort()); + // The connect() will complete exceptionally. + client.connect(address, new Session.Listener.Adapter(), new Promise.Completable<>()); + + await().atMost(Duration.ofMillis(5 * idleTimeout)).until(() -> connector.getConnectedEndPoints().size(), is(0)); + } + private void sleep(long value) { try 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 19e41d12af22..cb0bf1ef989c 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 @@ -1824,6 +1824,7 @@ private boolean onIdleTimeout() { String reason = "idle_timeout"; boolean notify = false; + boolean terminate = false; boolean sendGoAway = false; GoAwayFrame goAwayFrame = null; Throwable cause = null; @@ -1867,11 +1868,22 @@ private boolean onIdleTimeout() { if (LOG.isDebugEnabled()) LOG.debug("Already closed, ignored idle timeout for {}", HTTP2Session.this); - return false; + // Writes may be TCP congested, so termination never happened. + terminate = true; + goAwayFrame = goAwaySent; + if (goAwayFrame == null) + goAwayFrame = goAwayRecv; + break; } } } + if (terminate) + { + terminate(goAwayFrame); + return false; + } + if (notify) { boolean confirmed = notifyIdleTimeout(HTTP2Session.this);