From b953871c9a5ff4fbca4a2499848f75182dbd9810 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Fri, 12 Jan 2024 11:02:53 +0100 Subject: [PATCH] Backport of #11267. Signed-off-by: Simone Bordet --- .../jetty/http2/client/IdleTimeoutTest.java | 56 +++++++++++++++++++ .../org/eclipse/jetty/http2/HTTP2Session.java | 14 ++++- .../jetty/http3/internal/HTTP3Session.java | 48 ++++++++-------- .../jetty/quic/common/QuicSession.java | 11 ++-- 4 files changed, 99 insertions(+), 30 deletions(-) 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 f6d061daa2a..ae15c47090e 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 @@ -14,7 +14,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; @@ -39,7 +43,10 @@ import org.eclipse.jetty.http2.frames.DataFrame; 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; @@ -53,7 +60,9 @@ import org.hamcrest.Matchers; import org.junit.jupiter.api.Test; import org.slf4j.LoggerFactory; +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; @@ -749,6 +758,53 @@ public class IdleTimeoutTest extends AbstractTest assertTrue(responseLatch.await(5, TimeUnit.SECONDS)); } + @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()); + + 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 768824fe9bf..a734038f3e7 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 @@ -1909,6 +1909,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio { String reason = "idle_timeout"; boolean notify = false; + boolean terminate = false; boolean sendGoAway = false; GoAwayFrame goAwayFrame = null; Throwable cause = null; @@ -1952,11 +1953,22 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio { 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); diff --git a/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/HTTP3Session.java b/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/HTTP3Session.java index d6b04c4b577..c81528e2280 100644 --- a/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/HTTP3Session.java +++ b/jetty-http3/http3-common/src/main/java/org/eclipse/jetty/http3/internal/HTTP3Session.java @@ -216,8 +216,7 @@ public abstract class HTTP3Session extends ContainerLifeCycle implements Session long error = HTTP3ErrorCode.REQUEST_CANCELLED_ERROR.code(); String reason = "go_away"; failStreams(stream -> true, error, reason, true); - terminate(); - outwardDisconnect(error, reason); + terminateAndDisconnect(error, reason); } return CompletableFuture.completedFuture(null); } @@ -494,18 +493,12 @@ public abstract class HTTP3Session extends ContainerLifeCycle implements Session goAwaySent = newGoAwayFrame(false); GoAwayFrame goAwayFrame = goAwaySent; zeroStreamsAction = () -> writeControlFrame(goAwayFrame, Callback.from(() -> - { - terminate(); - outwardDisconnect(HTTP3ErrorCode.NO_ERROR.code(), "go_away"); - })); + terminateAndDisconnect(HTTP3ErrorCode.NO_ERROR.code(), "go_away") + )); } else { - zeroStreamsAction = () -> - { - terminate(); - outwardDisconnect(HTTP3ErrorCode.NO_ERROR.code(), "go_away"); - }; + zeroStreamsAction = () -> terminateAndDisconnect(HTTP3ErrorCode.NO_ERROR.code(), "go_away"); failStreams = true; } } @@ -566,6 +559,7 @@ public abstract class HTTP3Session extends ContainerLifeCycle implements Session public boolean onIdleTimeout() { boolean notify = false; + boolean terminate = false; try (AutoLock ignored = lock.lock()) { switch (closeState) @@ -583,9 +577,8 @@ public abstract class HTTP3Session extends ContainerLifeCycle implements Session case CLOSING: case CLOSED: { - if (LOG.isDebugEnabled()) - LOG.debug("already closed, ignored idle timeout for {}", this); - return false; + terminate = true; + break; } default: { @@ -594,6 +587,14 @@ public abstract class HTTP3Session extends ContainerLifeCycle implements Session } } + if (terminate) + { + if (LOG.isDebugEnabled()) + LOG.debug("already closed, ignored idle timeout for {}", this); + terminateAndDisconnect(HTTP3ErrorCode.NO_ERROR.code(), "idle_timeout"); + return false; + } + boolean confirmed = true; if (notify) confirmed = notifyIdleTimeout(); @@ -650,18 +651,15 @@ public abstract class HTTP3Session extends ContainerLifeCycle implements Session failStreams(stream -> true, error, reason, true); if (goAwayFrame != null) - { - writeControlFrame(goAwayFrame, Callback.from(() -> - { - terminate(); - outwardDisconnect(error, reason); - })); - } + writeControlFrame(goAwayFrame, Callback.from(() -> terminateAndDisconnect(error, reason))); else - { - terminate(); - outwardDisconnect(error, reason); - } + terminateAndDisconnect(error, reason); + } + + private void terminateAndDisconnect(long error, String reason) + { + terminate(); + outwardDisconnect(error, reason); } /** diff --git a/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java b/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java index d23d85d0efe..968e4dd2f3b 100644 --- a/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java +++ b/jetty-quic/quic-common/src/main/java/org/eclipse/jetty/quic/common/QuicSession.java @@ -398,11 +398,14 @@ public abstract class QuicSession extends ContainerLifeCycle public void outwardClose(long error, String reason) { + boolean closed = quicheConnection.close(error, reason); if (LOG.isDebugEnabled()) - LOG.debug("outward closing 0x{}/{} on {}", Long.toHexString(error), reason, this); - quicheConnection.close(error, reason); - // Flushing will eventually forward the outward close to the connection. - flush(); + LOG.debug("outward closing ({}) 0x{}/{} on {}", closed, Long.toHexString(error), reason, this); + if (closed) + { + // Flushing will eventually forward the outward close to the connection. + flush(); + } } private void finishOutwardClose(Throwable failure)