HTTPCLIENT-2379: PerRoutePool.release now decrements the counter first and keeps the entry in the map until the count reaches zero, regardless of whether the connection is still open. The entry is handed back to the parent pool exactly once—on that final release—and any attempt to release an entry that was never leased now raises an IllegalStateException.

Author: arturobernalg

httpclient5/src/main/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPool.java

@@ -288,20 +288,14 @@ PoolEntry<T, C> lease() {
         long release(final PoolEntry<T, C> entry, final boolean reusable) {
             lock.lock();
             try {
-                final C connection = entry.getConnection();
-                if (!reusable || connection == null || !connection.isOpen()) {
-                    entryMap.remove(entry);
-                    return 0;
-                } else {
-                    final AtomicLong counter = entryMap.compute(entry, (e, c) -> {
-                        if (c == null) {
-                            return null;
-                        }
-                        final long count = c.decrementAndGet();
-                        return count > 0 ? c : null;
-                    });
-                    return counter != null ? counter.get() : 0L;
-                }
+                final AtomicLong counter = entryMap.compute(entry, (e, c) -> {
+                    if (c == null) {
+                        return null;
+                    }
+                    final long count = c.decrementAndGet();
+                    return count > 0 ? c : null;
+                });
+                return counter != null ? counter.get() : 0L;
             } finally {
                 lock.unlock();
             }

httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingConnPoolTest.java

@@ -314,38 +314,6 @@ void testReleaseReusableInCacheNotReturnedToPool() throws Exception {
                 Mockito.anyBoolean());
     }
 
-    @Test
-    void testReleaseNonReusableInCacheReturnedToPool() throws Exception {
-        final PoolEntry<String, HttpConnection> poolEntry = new PoolEntry<>(DEFAULT_ROUTE);
-        poolEntry.assignConnection(connection);
-        Mockito.when(connection.isOpen()).thenReturn(true);
-        final H2SharingConnPool.PerRoutePool<String, HttpConnection> routePool = h2SharingPool.getPerRoutePool(DEFAULT_ROUTE);
-        routePool.track(poolEntry);
-        routePool.track(poolEntry);
-
-        h2SharingPool.release(poolEntry, false);
-
-        Mockito.verify(connPool).release(
-                Mockito.same(poolEntry),
-                Mockito.eq(false));
-    }
-
-    @Test
-    void testReleaseReusableAndClosedInCacheReturnedToPool() throws Exception {
-        final PoolEntry<String, HttpConnection> poolEntry = new PoolEntry<>(DEFAULT_ROUTE);
-        poolEntry.assignConnection(connection);
-        Mockito.when(connection.isOpen()).thenReturn(false);
-        final H2SharingConnPool.PerRoutePool<String, HttpConnection> routePool = h2SharingPool.getPerRoutePool(DEFAULT_ROUTE);
-        routePool.track(poolEntry);
-        routePool.track(poolEntry);
-
-        h2SharingPool.release(poolEntry, true);
-
-        Mockito.verify(connPool).release(
-                Mockito.same(poolEntry),
-                Mockito.eq(true));
-    }
-
     /**
      * Same connection can only be released once.
      * Attempting to release it again will throw: IllegalStateException("Pool entry is not present in the set of leased entries")
@@ -372,7 +340,7 @@ void testReleaseNonReusableNotInCacheReturnedToPool() throws Exception {
         }).when(connPool).release(Mockito.eq(poolEntry), Mockito.anyBoolean());
 
         h2SharingPool.release(poolEntry, false);
-        // for reproduce https://issues.apache.org/jira/browse/HTTPCLIENT-2379
+        h2SharingPool.release(poolEntry, false);
         Assertions.assertThrows(IllegalStateException.class, () -> h2SharingPool.release(poolEntry, false));
     }
 

httpclient5/src/test/java/org/apache/hc/client5/http/impl/nio/H2SharingPerRoutePoolTest.java

@@ -98,7 +98,7 @@ void testReleaseNonReusable() {
         pool.track(poolEntry1);
         pool.track(poolEntry1);
 
-        Assertions.assertEquals(0, pool.release(poolEntry1, false));
+        Assertions.assertEquals(2, pool.release(poolEntry1, false)); // 3 → 2
     }
 
     @Test
@@ -114,7 +114,7 @@ void testReleaseConnectionClosed() {
         pool.track(poolEntry1);
 
         Mockito.when(poolEntry1.getConnection().isOpen()).thenReturn(false);
-        Assertions.assertEquals(0, pool.release(poolEntry1, true));
+        Assertions.assertEquals(2, pool.release(poolEntry1, true));  // 3 → 2
     }
 
     @Test
@@ -124,7 +124,7 @@ void testReleaseConnectionMissing() {
         pool.track(poolEntry1);
 
         poolEntry1.discardConnection(CloseMode.IMMEDIATE);
-        Assertions.assertEquals(0, pool.release(poolEntry1, true));
+        Assertions.assertEquals(2, pool.release(poolEntry1, true));  // 3 → 2
     }
 
 }