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     | 47 ++++++++++++++-----
 .../client/http/MaxConcurrentStreamsTest.java | 33 +++++++++++++
 2 files changed, 67 insertions(+), 13 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 cdc4662fc1ac..f9cfa6c2949c 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
@@ -102,23 +102,21 @@ public void release()
     }
 
     @Override
-    public boolean abort(HttpExchange exchange, Throwable requestFailure, Throwable responseFailure)
+    public void exchangeTerminated(HttpExchange exchange, Result result)
     {
-        Stream stream = getStream();
-        boolean aborted = super.abort(exchange, requestFailure, responseFailure);
-        if (aborted)
+        super.exchangeTerminated(exchange, result);
+        if (result.isSucceeded())
+        {
+            release();
+        }
+        else
         {
+            Stream stream = getStream();
             if (stream != null)
-                stream.reset(new ResetFrame(stream.getId(), ErrorCode.CANCEL_STREAM_ERROR.code), Callback.NOOP);
+                stream.reset(new ResetFrame(stream.getId(), ErrorCode.CANCEL_STREAM_ERROR.code), new ReleaseCallback());
+            else
+                release();
         }
-        return aborted;
-    }
-
-    @Override
-    public void exchangeTerminated(HttpExchange exchange, Result result)
-    {
-        super.exchangeTerminated(exchange, result);
-        release();
     }
 
     @Override
@@ -129,4 +127,27 @@ public String toString()
                 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 3bf5ebc64832..282605278052 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 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r
         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.