From 5253641b122fee5ad14cc426369588de42ba6ca8 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 28 Jun 2022 16:28:53 +1000 Subject: [PATCH] Issue #8211 - fix ISE from HttpChannelState if failure during process Signed-off-by: Lachlan Roberts --- .../jetty/http2/tests/IdleTimeoutTest.java | 72 +++++++++++++++++++ .../server/internal/HttpChannelState.java | 14 +++- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/IdleTimeoutTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/IdleTimeoutTest.java index 814e1f8dd0eb..37e97152784a 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/IdleTimeoutTest.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/IdleTimeoutTest.java @@ -17,6 +17,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import org.eclipse.jetty.http.HttpFields; @@ -50,6 +51,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; @@ -378,6 +380,76 @@ public boolean onIdleTimeout(Stream stream, Throwable x) assertFalse(((HTTP2Session)session).isDisconnected()); } + @Test + public void testForgivingFailedClientEnforcingStreamIdleTimeout() throws Exception + { + final int idleTimeout = 1000; + AtomicReference failure = new AtomicReference<>(); + AtomicBoolean failedDidNotThrow = new AtomicBoolean(false); + start(new Handler.Processor() + { + @Override + public void process(Request request, Response response, Callback callback) + { + sleep(2 * idleTimeout); + try + { + callback.succeeded(); + } + catch (Throwable t) + { + failure.set(t); + callback.failed(t); + failedDidNotThrow.set(true); + } + } + }); + + Session session = newClientSession(new Session.Listener.Adapter()); + + final CountDownLatch dataLatch = new CountDownLatch(1); + final CountDownLatch timeoutLatch = new CountDownLatch(1); + MetaData.Request metaData = newRequest("GET", HttpFields.EMPTY); + HeadersFrame requestFrame = new HeadersFrame(metaData, null, true); + session.newStream(requestFrame, new Promise.Adapter<>() + { + @Override + public void succeeded(Stream stream) + { + stream.setIdleTimeout(idleTimeout); + } + }, new Stream.Listener.Adapter() + { + @Override + public void onData(Stream stream, DataFrame frame, Callback callback) + { + callback.succeeded(); + dataLatch.countDown(); + } + + @Override + public boolean onIdleTimeout(Stream stream, Throwable x) + { + assertThat(x, Matchers.instanceOf(TimeoutException.class)); + timeoutLatch.countDown(); + return true; + } + }); + + assertTrue(timeoutLatch.await(5, TimeUnit.SECONDS)); + // We must not receive any DATA frame. + assertFalse(dataLatch.await(2 * idleTimeout, TimeUnit.MILLISECONDS)); + // Stream must be gone. + assertTrue(session.getStreams().isEmpty()); + // Session must not be closed, nor disconnected. + assertFalse(session.isClosed()); + assertFalse(((HTTP2Session)session).isDisconnected()); + + // Callback.succeeded() should have thrown an exception, but Callback.failed() should not have thrown. + assertNotNull(failure.get()); + assertTrue(failedDidNotThrow.get()); + } + @Test public void testServerEnforcingStreamIdleTimeout() throws Exception { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java index 511b419d1198..8cc560b0950d 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java @@ -663,7 +663,15 @@ public void run() failure = x; } if (failure != null) - request._callback.failed(failure); + { + try (AutoLock ignored = _lock.lock()) + { + if (request._httpChannel != null && !request._httpChannel._completed) + request._callback.failed(failure); + else if (LOG.isDebugEnabled()) + LOG.debug("Process failed", failure); + } + } HttpStream stream; boolean completeStream; @@ -1364,6 +1372,9 @@ public void failed(Throwable failure) boolean completeStream; try (AutoLock ignored = _request._lock.lock()) { + request = _request; + if (request._httpChannel != null && request._httpChannel._error != null && TypeUtil.isAssociated(request._httpChannel._error.getCause(), failure)) + return; lockedOnComplete(); httpChannelState = _request._httpChannel; @@ -1373,7 +1384,6 @@ public void failed(Throwable failure) // Verify whether we can write an error response. writeErrorResponse = !httpChannelState._stream.isCommitted(); stream = httpChannelState._stream; - request = _request; // Consume any input. Throwable unconsumed = stream.consumeAvailable();