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.
This commit is contained in:
Carter Kozak 2022-06-10 14:31:04 -04:00 committed by Oleg Kalnichevski
parent 8dbaf131f5
commit 3bd017cb0a
2 changed files with 26 additions and 2 deletions

View File

@ -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());
}

View File

@ -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();
}
}
}