From 35541d0c1eff5de6c9dbd2d717a67d87cd411f8f Mon Sep 17 00:00:00 2001
From: Simone Bordet <simone.bordet@gmail.com>
Date: Mon, 13 Aug 2018 11:44:57 +0200
Subject: [PATCH] Fixes #2796 - Max local stream count exceeded when request
 fails.

Now releasing the connection only after the stream has been reset, so
we are sure that the stream has been closed and its count decremented.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
---
 .../client/http/HttpChannelOverHTTP2.java     | 49 +++++++++++++------
 .../client/http/MaxConcurrentStreamsTest.java | 33 +++++++++++++
 2 files changed, 68 insertions(+), 14 deletions(-)

diff --git a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java
index cdc4662fc1a..f9cfa6c2949 100644
--- a/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java
+++ b/jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java
@@ -101,24 +101,22 @@ public class HttpChannelOverHTTP2 extends HttpChannel
         connection.release(this);
     }
 
-    @Override
-    public boolean abort(HttpExchange exchange, Throwable requestFailure, Throwable responseFailure)
-    {
-        Stream stream = getStream();
-        boolean aborted = super.abort(exchange, requestFailure, responseFailure);
-        if (aborted)
-        {
-            if (stream != null)
-                stream.reset(new ResetFrame(stream.getId(), ErrorCode.CANCEL_STREAM_ERROR.code), Callback.NOOP);
-        }
-        return aborted;
-    }
-
     @Override
     public void exchangeTerminated(HttpExchange exchange, Result result)
     {
         super.exchangeTerminated(exchange, result);
-        release();
+        if (result.isSucceeded())
+        {
+            release();
+        }
+        else
+        {
+            Stream stream = getStream();
+            if (stream != null)
+                stream.reset(new ResetFrame(stream.getId(), ErrorCode.CANCEL_STREAM_ERROR.code), new ReleaseCallback());
+            else
+                release();
+        }
     }
 
     @Override
@@ -129,4 +127,27 @@ public class HttpChannelOverHTTP2 extends HttpChannel
                 sender,
                 receiver);
     }
+
+    private class ReleaseCallback implements Callback
+    {
+        @Override
+        public void succeeded()
+        {
+            release();
+        }
+
+        @Override
+        public void failed(Throwable x)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug(x);
+            release();
+        }
+
+        @Override
+        public InvocationType getInvocationType()
+        {
+            return InvocationType.NON_BLOCKING;
+        }
+    }
 }
diff --git a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/MaxConcurrentStreamsTest.java b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/MaxConcurrentStreamsTest.java
index 3bf5ebc6483..28260527805 100644
--- a/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/MaxConcurrentStreamsTest.java
+++ b/jetty-http2/http2-http-client-transport/src/test/java/org/eclipse/jetty/http2/client/http/MaxConcurrentStreamsTest.java
@@ -367,6 +367,39 @@ public class MaxConcurrentStreamsTest extends AbstractTest
         Assert.assertTrue(failures.toString(), failures.isEmpty());
     }
 
+    @Test
+    public void testTwoConcurrentStreamsFirstTimesOut() throws Exception
+    {
+        long timeout = 1000;
+        start(1, new EmptyServerHandler()
+        {
+            @Override
+            protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
+            {
+                if (target.endsWith("/1"))
+                    sleep(2 * timeout);
+            }
+        });
+        client.setMaxConnectionsPerDestination(1);
+
+        CountDownLatch latch = new CountDownLatch(1);
+        client.newRequest("localhost", connector.getLocalPort())
+                .path("/1")
+                .timeout(timeout, TimeUnit.MILLISECONDS)
+                .send(result ->
+                {
+                    if (result.isFailed())
+                        latch.countDown();
+                });
+
+        ContentResponse response2 = client.newRequest("localhost", connector.getLocalPort())
+                .path("/2")
+                .send();
+
+        Assert.assertEquals(HttpStatus.OK_200, response2.getStatus());
+        Assert.assertTrue(latch.await(2 * timeout, TimeUnit.MILLISECONDS));
+    }
+
     private void primeConnection() throws Exception
     {
         // Prime the connection so that the maxConcurrentStream setting arrives to the client.