Issue #8211 - fix ISE from HttpChannelState if failure during process

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;
@@ -49,6 +50,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;
 
@@ -384,6 +386,77 @@ public boolean onIdleTimeout(Stream stream, Throwable x)
         assertFalse(((HTTP2Session)session).isDisconnected());
     }
 
+    @Test
+    public void testForgivingFailedClientEnforcingStreamIdleTimeout() throws Exception
+    {
+        final int idleTimeout = 1000;
+        AtomicReference<Throwable> 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() {});
+
+        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()
+        {
+            @Override
+            public void onDataAvailable(Stream stream)
+            {
+                Stream.Data data = stream.readData();
+                data.release();
+                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
     {

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

@@ -663,8 +663,9 @@ public void run()
             {
                 failure = x;
             }
+
             if (failure != null)
-                request._callback.failed(failure);
+                _request._callback.ensureCompleted(failure);
 
             HttpStream stream;
             boolean completeStream;
@@ -1299,6 +1300,7 @@ private static class ChannelCallback implements Callback
 
         private final ChannelRequest _request;
         private Throwable _completedBy;
+        private boolean _completed = false;
 
         private ChannelCallback(ChannelRequest request)
         {
@@ -1317,6 +1319,10 @@ public void succeeded()
             boolean completeStream;
             try (AutoLock ignored = _request._lock.lock())
             {
+                if (_completed)
+                    return;
+                _completed = true;
+
                 lockedOnComplete();
                 httpChannelState = _request._httpChannel;
                 completeStream = httpChannelState._processState == ProcessState.PROCESSED && httpChannelState._writeState == WriteState.LAST_WRITE_COMPLETED;
@@ -1377,6 +1383,10 @@ public void failed(Throwable failure)
             boolean completeStream;
             try (AutoLock ignored = _request._lock.lock())
             {
+                if (_completed)
+                    return;
+                _completed = true;
+
                 lockedOnComplete();
 
                 httpChannelState = _request._httpChannel;
@@ -1439,6 +1449,25 @@ private void lockedOnComplete()
             httpChannelState._completed = true;
         }
 
+        public void ensureCompleted(Throwable failure)
+        {
+            HttpChannelState httpChannel = _request._httpChannel;
+            if (httpChannel != null)
+            {
+                try (AutoLock ignored = httpChannel._lock.lock())
+                {
+                    if (!httpChannel._completed)
+                    {
+                        failed(failure);
+                        return;
+                    }
+                }
+            }
+
+            if (LOG.isDebugEnabled())
+                LOG.debug("Process failed", failure);
+        }
+
         @Override
         public InvocationType getInvocationType()
         {

jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/ExceptionUtil.java

@@ -145,6 +145,16 @@ public static <T extends Throwable> void ifExceptionThrowAllAs(Class<T> type, Th
         throw as(type, throwable);
     }
 
+    /** Check if two {@link Throwable}s are associated.
+     * @param t1 A Throwable or null
+     * @param t2 Another Throwable or null
+     * @return true iff the exceptions are associated by being the same instance, sharing a cause or one suppressing the other.
+     */
+    public static boolean areAssociated(Throwable t1, Throwable t2)
+    {
+        return !areNotAssociated(t1, t2);
+    }
+
     /** Check if two {@link Throwable}s are associated.
      * @param t1 A Throwable or null
      * @param t2 Another Throwable or null