Fixes #12227 - Improve HttpConnection buffer recycling.

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

@@ -488,11 +488,12 @@ void parseAndFillForContent()
             if (!_parser.inContentState())
             {
                 // The request is complete, and we are going to re-enter onFillable(),
-                // because either A) the request/response was completed synchronously
-                // so the onFillable() thread will loop, or B) the request/response
+                // because either A: the request/response was completed synchronously
+                // so the onFillable() thread will loop, or B: the request/response
                 // was completed asynchronously, and the HttpStreamOverHTTP1 dispatches
                 // a call to onFillable() to process the next request.
-                // Therefore, there is no need to release the request buffer here.
+                // Therefore, there is no need to release the request buffer here,
+                // also because the buffer may contain pipelined requests.
                 break;
             }
 
@@ -614,19 +615,6 @@ public void onOpen()
             getExecutor().execute(this);
     }
 
-    @Override
-    public void onClose(Throwable cause)
-    {
-        // TODO: do we really need to do this?
-        //  This event is fired really late, sendCallback should already be failed at this point.
-        //  Revisit whether we still need IteratingCallback.close().
-        if (cause == null)
-            _sendCallback.close();
-        else
-            _sendCallback.abort(cause);
-        super.onClose(cause);
-    }
-
     @Override
     public void run()
     {

jetty-core/jetty-tests/jetty-test-client-transports/src/test/java/org/eclipse/jetty/test/client/transport/HttpClientIdleTimeoutTest.java

@@ -15,6 +15,8 @@
 
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
 
 import org.eclipse.jetty.client.ContentResponse;
 import org.eclipse.jetty.client.StringRequestContent;
@@ -26,6 +28,8 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
+import static org.awaitility.Awaitility.await;
+import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -38,17 +42,17 @@ public class HttpClientIdleTimeoutTest extends AbstractTest
     public void testClientIdleTimeout(Transport transport) throws Exception
     {
         long serverIdleTimeout = idleTimeout * 2;
-        CountDownLatch serverIdleTimeoutLatch = new CountDownLatch(1);
+        AtomicReference<Callback> serverCallbackRef = new AtomicReference<>();
         start(transport, new Handler.Abstract()
         {
             @Override
             public boolean handle(Request request, Response response, Callback callback)
             {
                 // Do not succeed the callback if it's a timeout request.
-                if (!Request.getPathInContext(request).equals("/timeout"))
-                    callback.succeeded();
+                if (Request.getPathInContext(request).equals("/timeout"))
+                    request.addFailureListener(x -> serverCallbackRef.set(callback));
                 else
-                    request.addFailureListener(x -> serverIdleTimeoutLatch.countDown());
+                    callback.succeeded();
                 return true;
             }
         });
@@ -75,25 +79,26 @@ public boolean handle(Request request, Response response, Callback callback)
         assertEquals(HttpStatus.OK_200, response.getStatus());
 
         // Wait for the server's idle timeout to trigger to give it a chance to clean up its resources.
-        assertTrue(serverIdleTimeoutLatch.await(2 * serverIdleTimeout, TimeUnit.MILLISECONDS));
+        Callback callback = await().atMost(2 * serverIdleTimeout, TimeUnit.MILLISECONDS).until(serverCallbackRef::get, notNullValue());
+        callback.failed(new TimeoutException());
     }
 
     @ParameterizedTest
     @MethodSource("transports")
     public void testRequestIdleTimeout(Transport transport) throws Exception
     {
         long serverIdleTimeout = idleTimeout * 2;
-        CountDownLatch serverIdleTimeoutLatch = new CountDownLatch(1);
+        AtomicReference<Callback> serverCallbackRef = new AtomicReference<>();
         start(transport, new Handler.Abstract()
         {
             @Override
             public boolean handle(Request request, Response response, Callback callback) throws Exception
             {
                 // Do not succeed the callback if it's a timeout request.
-                if (!Request.getPathInContext(request).equals("/timeout"))
-                    callback.succeeded();
+                if (Request.getPathInContext(request).equals("/timeout"))
+                    request.addFailureListener(x -> serverCallbackRef.set(callback));
                 else
-                    request.addFailureListener(x -> serverIdleTimeoutLatch.countDown());
+                    callback.succeeded();
                 return true;
             }
         });
@@ -120,7 +125,8 @@ public boolean handle(Request request, Response response, Callback callback) thr
         assertEquals(HttpStatus.OK_200, response.getStatus());
 
         // Wait for the server's idle timeout to trigger to give it a chance to clean up its resources.
-        assertTrue(serverIdleTimeoutLatch.await(2 * serverIdleTimeout, TimeUnit.MILLISECONDS));
+        Callback callback = await().atMost(2 * serverIdleTimeout, TimeUnit.MILLISECONDS).until(serverCallbackRef::get, notNullValue());
+        callback.failed(new TimeoutException());
     }
 
     @ParameterizedTest