From 56d3a6ef5da40438ea25b304563ccc03e1555e5b Mon Sep 17 00:00:00 2001
From: Simone Bordet <simone.bordet@gmail.com>
Date: Mon, 8 Jun 2020 16:15:41 +0200
Subject: [PATCH] Issue #4855 - Occasional h2spec failures on CI

Updates after review.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
---
 .../http2/server/HttpTransportOverHTTP2.java  | 26 ++++++++-----------
 .../jetty/server/HttpChannelState.java        |  6 ++---
 2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java
index 540ab9bfa1e7..dae9502f6b59 100644
--- a/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java
+++ b/jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java
@@ -382,10 +382,10 @@ private class TransportCallback implements Callback
         private boolean _commit;
         private Throwable _failure;
 
-        private void reset()
+        private void reset(boolean failed)
         {
             assert Thread.holdsLock(this);
-            _state = State.IDLE;
+            _state = failed ? State.FAILED : State.IDLE;
             _callback = null;
             _commit = false;
             _failure = null;
@@ -453,26 +453,25 @@ private void pending()
                         callback = _callback;
                         commit = _commit;
                         failure = null;
-                        reset();
+                        reset(false);
                         break;
                     }
                     case FAILING:
                     {
                         // The send already completed with a failure, but
                         // the call to failed() was delayed, so call it now.
-                        _state = State.FAILED;
                         callback = _callback;
                         commit = _commit;
                         failure = _failure;
+                        reset(true);
                         break;
                     }
                     default:
                     {
-                        _state = State.FAILED;
-                        _failure = new IllegalStateException("Invalid transport state: " + _state);
                         callback = _callback;
                         commit = _commit;
-                        failure = _failure;
+                        failure = new IllegalStateException("Invalid transport state: " + _state);
+                        reset(true);
                         break;
                     }
                 }
@@ -502,7 +501,7 @@ public void succeeded()
                     {
                         callback = _callback;
                         commit = _commit;
-                        reset();
+                        reset(false);
                         break;
                     }
                     default:
@@ -535,10 +534,9 @@ public void failed(Throwable failure)
                     case IDLE:
                     case PENDING:
                     {
-                        _state = State.FAILED;
-                        _failure = failure;
                         callback = _callback;
                         commit = _commit;
+                        reset(true);
                         break;
                     }
                     default:
@@ -563,10 +561,9 @@ private boolean idleTimeout(Throwable failure)
                     case PENDING:
                     {
                         // The send was started but idle timed out, fail it.
-                        _state = State.FAILED;
-                        _failure = failure;
                         callback = _callback;
                         timeout = true;
+                        reset(true);
                         break;
                     }
                     case IDLE:
@@ -587,13 +584,12 @@ private boolean idleTimeout(Throwable failure)
                     default:
                     {
                         // Should not happen, but just in case.
-                        _state = State.FAILED;
-                        _failure = new IllegalStateException("Invalid transport state: " + _state, failure);
                         callback = _callback;
                         if (callback == null)
                             callback = Callback.NOOP;
                         timeout = true;
-                        failure = _failure;
+                        failure = new IllegalStateException("Invalid transport state: " + _state, failure);
+                        reset(true);
                         break;
                     }
                 }
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
index 4cf983bb0128..60d494614b19 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
@@ -817,14 +817,12 @@ protected void onError(Throwable th)
         // check the actions of the listeners
         synchronized (this)
         {
-            // If we are still async and nobody has called sendError
             if (_requestState == RequestState.ASYNC && !_sendError)
             {
-                // Then the listeners did not invoke API methods
-                // and the container must provide a default error dispatch.
+                // The listeners did not invoke API methods and the
+                // container must provide a default error dispatch.
                 sendError(th);
             }
-            // Otherwise the listeners have called AsyncContext.complete().
             else if (_requestState != RequestState.COMPLETE)
             {
                 LOG.warn("unhandled in state " + _requestState, new IllegalStateException(th));