[httpcomponents-client] branch master updated: HTTPCLIENT-2221 Closing a classic response/entity allows connection reuse

[ol...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git


The following commit(s) were added to refs/heads/master by this push:
     new 3bd017cb0 HTTPCLIENT-2221 Closing a classic response/entity allows connection reuse
3bd017cb0 is described below

commit 3bd017cb0a772f0c33d22dc1f8fd40012d33dab9
Author: Carter Kozak <ck...@apache.org>
AuthorDate: Fri Jun 10 14:31:04 2022 -0400

    HTTPCLIENT-2221 Closing a classic response/entity allows connection reuse
    
    Previously, a partially consumed response body closed via
    CloseableHttpResponse.close or HttpEntity.close would fully consume
    remaining bytes (via close), however it would not release the
    connection for reuse.
    If CloseableHttpResponse.close was called, it would follow the close
    with a discard/disconnect, however if only the entity was closed,
    the connection would remain in a checked-out (leaked) state.
    
    Now, we take advantage of the fact that closing a response stream
    on any reusable connection is required to drain bytes on closure.
    Failures are detected by writeTo and the stream returned by
    getContent, so we can be confident that we will not return a
    broken connection to the pool.
---
 .../hc/client5/testing/sync/TestConnectionReuse.java       | 14 ++++++++++++--
 .../hc/client5/http/impl/classic/ResponseEntityProxy.java  | 14 ++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionReuse.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionReuse.java
index 2f04090a5..05c0a8454 100644
--- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionReuse.java
+++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestConnectionReuse.java
@@ -84,6 +84,8 @@ public class TestConnectionReuse extends LocalServerTestBase {
             }
         }
 
+        // Expect leased connections to be returned
+        Assertions.assertEquals(0, this.connManager.getTotalStats().getLeased());
         // Expect some connection in the pool
         Assertions.assertTrue(this.connManager.getTotalStats().getAvailable() > 0);
     }
@@ -120,6 +122,8 @@ public class TestConnectionReuse extends LocalServerTestBase {
             }
         }
 
+        // Expect leased connections to be returned
+        Assertions.assertEquals(0, this.connManager.getTotalStats().getLeased());
         // Expect some connection in the pool
         Assertions.assertTrue(this.connManager.getTotalStats().getAvailable() > 0);
     }
@@ -166,6 +170,8 @@ public class TestConnectionReuse extends LocalServerTestBase {
             }
         }
 
+        // Expect leased connections to be returned
+        Assertions.assertEquals(0, this.connManager.getTotalStats().getLeased());
         // Expect zero connections in the pool
         Assertions.assertEquals(0, this.connManager.getTotalStats().getAvailable());
     }
@@ -197,8 +203,10 @@ public class TestConnectionReuse extends LocalServerTestBase {
             }
         }
 
-        // Expect zero connections in the pool
-        Assertions.assertEquals(0, this.connManager.getTotalStats().getAvailable());
+        // Expect leased connections to be returned
+        Assertions.assertEquals(0, this.connManager.getTotalStats().getLeased());
+        // Expect some connections in the pool
+        Assertions.assertTrue(this.connManager.getTotalStats().getAvailable() > 0);
     }
 
     @Test
@@ -242,6 +250,8 @@ public class TestConnectionReuse extends LocalServerTestBase {
             return null;
         });
 
+        // Expect leased connections to be returned
+        Assertions.assertEquals(0, this.connManager.getTotalStats().getLeased());
         Assertions.assertEquals(1, this.connManager.getTotalStats().getAvailable());
     }
 
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ResponseEntityProxy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ResponseEntityProxy.java
index 9ea2d04d9..23a720ecb 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ResponseEntityProxy.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ResponseEntityProxy.java
@@ -174,4 +174,18 @@ class ResponseEntityProxy extends HttpEntityWrapper implements EofSensorWatcher
             }
     }
 
+    @Override
+    public void close() throws IOException {
+        try {
+            // HttpEntity.close will close the underlying resource. Closing a reusable request stream results in
+            // draining remaining data, allowing for connection reuse.
+            super.close();
+            releaseConnection();
+        } catch (final IOException | RuntimeException ex) {
+            discardConnection();
+            throw ex;
+        } finally {
+            cleanup();
+        }
+    }
 }