#7107: handle review comments

jetty · Nov 19, 2021 · 77f953f · 77f953f
1 parent 4039360
commit 77f953f
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 13 deletions.
diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java
@@ -391,11 +391,6 @@ protected boolean deactivate(Connection connection)
 
     @Override
     public boolean remove(Connection connection)
-    {
-        return remove(connection, false);
-    }
-
-    protected boolean remove(Connection connection, boolean force)
     {
         if (!(connection instanceof Attachable))
             throw new IllegalArgumentException("Invalid connection object: " + connection);
@@ -408,14 +403,20 @@ protected boolean remove(Connection connection, boolean force)
             attachable.setAttachment(null);
         if (LOG.isDebugEnabled())
             LOG.debug("Removed ({}) {} {}", removed, holder.entry, pool);
-        if (removed || force)
+        if (removed)
         {
             released(connection);
             removed(connection);
         }
         return removed;
     }
 
+    @Deprecated
+    protected boolean remove(Connection connection, boolean force)
+    {
+        return remove(connection);
+    }
+
     protected void onCreated(Connection connection)
     {
     }

diff --git a/...port/src/test/java/org/eclipse/jetty/http2/client/http/MultiplexedConnectionPoolTest.java b/...port/src/test/java/org/eclipse/jetty/http2/client/http/MultiplexedConnectionPoolTest.java
@@ -95,6 +95,7 @@ public void testMaxDurationConnectionsWithMultiplexedPoolLifecycle() throws Exce
         final int maxDuration = 200;
         AtomicInteger poolCreateCounter = new AtomicInteger();
         AtomicInteger poolRemoveCounter = new AtomicInteger();
+        AtomicReference<Pool<Connection>> poolRef = new AtomicReference<>();
         ConnectionPoolFactory factory = new ConnectionPoolFactory("MaxDurationConnectionsWithMultiplexedPoolLifecycle", destination ->
         {
             int maxConnections = destination.getHttpClient().getMaxConnectionsPerDestination();
@@ -112,6 +113,7 @@ protected void removed(Connection connection)
                     poolRemoveCounter.incrementAndGet();
                 }
             };
+            poolRef.set(pool.getBean(Pool.class));
             pool.setMaxDuration(maxDuration);
             return pool;
         });
@@ -152,21 +154,21 @@ protected void service(String target, org.eclipse.jetty.server.Request jettyRequ
         client = new HttpClient(transport);
         client.start();
 
-        CountDownLatch[] reqClientDoneLatches = new CountDownLatch[] {new CountDownLatch(1), new CountDownLatch(1), new CountDownLatch(1)};
+        CountDownLatch[] reqClientSuccessLatches = new CountDownLatch[] {new CountDownLatch(1), new CountDownLatch(1), new CountDownLatch(1)};
 
-        sendRequest(reqClientDoneLatches, 0);
+        sendRequest(reqClientSuccessLatches, 0);
         // wait until handler is executing
         assertTrue(reqExecutingLatches[0].await(5, TimeUnit.SECONDS));
         LOG.debug("req 0 executing");
 
-        sendRequest(reqClientDoneLatches, 1);
+        sendRequest(reqClientSuccessLatches, 1);
         // wait until handler executed sleep
         assertTrue(reqExecutedLatches[1].await(5, TimeUnit.SECONDS));
         LOG.debug("req 1 executed");
 
         // Now the pool contains one connection that is expired but in use by 2 threads.
 
-        sendRequest(reqClientDoneLatches, 2);
+        sendRequest(reqClientSuccessLatches, 2);
         LOG.debug("req2 sent");
         assertTrue(reqExecutingLatches[2].await(5, TimeUnit.SECONDS));
         LOG.debug("req2 executing");
@@ -175,19 +177,20 @@ protected void service(String target, org.eclipse.jetty.server.Request jettyRequ
 
         // release and wait for req2 to be done before releasing req1
         reqFinishingLatches[2].countDown();
-        assertTrue(reqClientDoneLatches[2].await(5, TimeUnit.SECONDS));
+        assertTrue(reqClientSuccessLatches[2].await(5, TimeUnit.SECONDS));
         reqFinishingLatches[1].countDown();
 
         // release req0 once req1 is done; req 1 should not have closed the response as req 0 is still running
-        assertTrue(reqClientDoneLatches[1].await(5, TimeUnit.SECONDS));
+        assertTrue(reqClientSuccessLatches[1].await(5, TimeUnit.SECONDS));
         reqFinishingLatches[0].countDown();
-        assertTrue(reqClientDoneLatches[0].await(5, TimeUnit.SECONDS));
+        assertTrue(reqClientSuccessLatches[0].await(5, TimeUnit.SECONDS));
 
         // Check that the pool created 2 and removed 2 connections;
         // 2 were removed b/c waiting for req 2 means the 2nd connection
         // expired and has to be removed and closed upon being returned to the pool.
         assertThat(poolCreateCounter.get(), Matchers.is(2));
         assertThat(poolRemoveCounter.get(), Matchers.is(2));
+        assertThat(poolRef.get().size(), Matchers.is(0));
     }
 
     private void sendRequest(CountDownLatch[] reqClientDoneLatches, int i)