Use tri-state return value to make duplicate releases harmless

PerRoutePool.release now signals “not tracked” with the sentinel NOT_TRACKED (-1). H2SharingConnPool.release delegates to the underlying pool only when the result is 0 and silently ignores the sentinel, so a second release no longer triggers IllegalStateException. Tests have been updated to expect the new sentinel and remain green.

Author: arturobernalg

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

@@ -79,6 +79,8 @@ public class H2SharingConnPool<T, C extends HttpConnection> implements ManagedCo
     private final ConcurrentMap<T, PerRoutePool<T, C>> perRouteCache;
     private final AtomicBoolean closed;
 
+    private static final long NOT_TRACKED = -1L;
+
     public H2SharingConnPool(final ManagedConnPool<T, C> pool) {
         this.pool = Args.notNull(pool, "Connection pool");
         this.perRouteCache = new ConcurrentHashMap<>();
@@ -165,27 +167,30 @@ public void release(final PoolEntry<T, C> entry, final boolean reusable) {
             pool.release(entry, reusable);
             return;
         }
-        final T route = entry.getRoute();
-        final PerRoutePool<T, C> perRoutePool = perRouteCache.get(route);
-        if (perRoutePool != null && perRoutePool.getCount(entry) == 0) {
-            return;
-        }
+
+        final PerRoutePool<T, C> perRoutePool = perRouteCache.get(entry.getRoute());
         if (perRoutePool != null) {
-            final long count = perRoutePool.release(entry, reusable);
-            if (count > 0) {
+            final long res = perRoutePool.release(entry, reusable);
+
+            if (res == NOT_TRACKED) {
+                return;
+            }
+            if (res > 0) {
                 if (LOG.isDebugEnabled()) {
                     LOG.debug("Connection {} is being shared for message exchange multiplexing (lease count = {})",
-                            ConnPoolSupport.getId(entry.getConnection()), count);
+                            ConnPoolSupport.getId(entry.getConnection()), res);
                 }
                 return;
             }
         }
         if (LOG.isDebugEnabled()) {
-            LOG.debug("Releasing connection {} back to the pool", ConnPoolSupport.getId(entry.getConnection()));
+            LOG.debug("Releasing connection {} back to the pool",
+                    ConnPoolSupport.getId(entry.getConnection()));
         }
         pool.release(entry, reusable);
     }
 
+
     @Override
     public void setMaxTotal(final int max) {
         pool.setMaxTotal(max);
@@ -291,20 +296,20 @@ 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()) {
+                final AtomicLong counter = entryMap.get(entry);
+                if (counter == null) {
+                    return NOT_TRACKED;
+                }
+                final C conn = entry.getConnection();
+                if (!reusable || conn == null || !conn.isOpen()) {
+                    entryMap.remove(entry);
+                    return 0L;
+                }
+                final long remaining = counter.decrementAndGet();
+                if (remaining == 0) {
                     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;
                 }
+                return remaining;
             } finally {
                 lock.unlock();
             }

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

@@ -89,7 +89,7 @@ void testReleaseReusable() {
         Assertions.assertEquals(2, pool.release(poolEntry1, true));
         Assertions.assertEquals(1, pool.release(poolEntry1, true));
         Assertions.assertEquals(0, pool.release(poolEntry1, true));
-        Assertions.assertEquals(0, pool.release(poolEntry1, true));
+        Assertions.assertEquals(-1, pool.release(poolEntry1, true));
     }
 
     @Test
@@ -103,8 +103,8 @@ void testReleaseNonReusable() {
 
     @Test
     void testReleaseNonPresent() {
-        Assertions.assertEquals(0, pool.release(poolEntry1, true));
-        Assertions.assertEquals(0, pool.release(poolEntry2, true));
+        Assertions.assertEquals(-1, pool.release(poolEntry1, true));
+        Assertions.assertEquals(-1, pool.release(poolEntry2, true));
     }
 
     @Test