Merge pull request #2797 from eclipse/jetty-9.4.x-2796-max_concurrent…

…_streams_local_timeout Fixes #2796 - Max local stream count exceeded when request fails.
jetty · Aug 14, 2018 · 03af18c · 03af18c
2 parents dd428b2 + 35541d0
commit 03af18c
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 13 deletions.
diff --git a/...ent-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java b/...ent-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/...transport/src/test/java/org/eclipse/jetty/http2/client/http/MaxConcurrentStreamsTest.java b/...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.