Fix #9169 refine idle timeout and failure

Only fail request callback if a failure has not been otherwise notified. Slight optimization for failing idle timeouts by avoiding double lock. Always create a failure if failing the callback.
jetty · Aug 28, 2023 · a746c04 · a746c04
1 parent 61f6444
commit a746c04
Showing 1 changed file with 21 additions and 25 deletions.
diff --git a/...y-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java b/...y-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java
@@ -346,7 +346,6 @@ public Invocable.InvocationType getInvocationType()
     @Override
     public Runnable onIdleTimeout(TimeoutException t)
     {
-        Predicate<TimeoutException> onIdleTimeout;
         try (AutoLock ignored = _lock.lock())
         {
             if (LOG.isDebugEnabled())
@@ -380,33 +379,30 @@ public Runnable onIdleTimeout(TimeoutException t)
                 if (invokeOnContentAvailable != null || invokeWriteFailure != null)
                     return _serializedInvoker.offer(invokeOnContentAvailable, invokeWriteFailure);
 
-                // Otherwise We ask any idle timeout listeners if we should call onFailure or not
-                onIdleTimeout = _onIdleTimeout;
+                // otherwise, if there is an idle timeout listener, we ask if if we should call onFailure or not
+                Predicate<TimeoutException> onIdleTimeout = _onIdleTimeout;
+                if (onIdleTimeout != null)
+                {
+                    return _serializedInvoker.offer(() ->
+                    {
+                        if (onIdleTimeout.test(t))
+                        {
+                            // If the idle timeout listener(s) return true, then we call onFailure and any task it returns.
+                            Runnable task = onFailure(t);
+                            if (task != null)
+                                task.run();
+                        }
+                    });
+                }
+
+                // otherwise, we are going to fail the callback, either directly or by a call to onFailure, so ensure all future IO
+                // operations are failed as well
+                _failure = Content.Chunk.from(t, true);
 
-                // If there is no idle nor failure listener, then we can fail the callback directly without double lock
-                if (onIdleTimeout == null && _onFailure == null && _request != null)
+                // if there is no failure listener, then we can fail the callback directly without a double lock
+                if (_onFailure == null && _request != null)
                     return () -> _request._callback.failed(t);
             }
-            else
-            {
-                onIdleTimeout = null;
-            }
-        }
-
-        // Ask any listener what to do
-        if (onIdleTimeout != null)
-        {
-            Runnable onIdle = () ->
-            {
-                if (onIdleTimeout.test(t))
-                {
-                    // If the idle timeout listener(s) return true, then we call onFailure and any task it returns.
-                    Runnable task = onFailure(t);
-                    if (task != null)
-                        task.run();
-                }
-            };
-            return _serializedInvoker.offer(onIdle);
         }
 
         // otherwise treat as a failure