Fix for #1286 #1287
Fix for #1286 #1287
Conversation
When feign is done with the response, also invoke close on http response (if closeable)
| @@ -233,6 +231,8 @@ public Reader asReader(Charset charset) throws IOException { | |||
| @Override | |||
| public void close() throws IOException { | |||
| EntityUtils.consume(entity); | |||
| if (httpResponse instanceof CloseableHttpResponse) | |||
| ((CloseableHttpResponse) httpResponse).close(); | |||
There was a problem hiding this comment.
could this close method call be enclosed in a finally block? that would ensure the socket is released even in the event of an exception when consuming the entity.
It is also the approach recommended here - https://hc.apache.org/httpcomponents-client-ga/quickstart.html
quoted text from the document
// In order to ensure correct deallocation of system resources
// the user MUST call CloseableHttpResponse#close() from a finally clause.
try{
EntityUtils.consume(entity1);
} finally {
response1.close();
}
- Additionally it would also not show up as a potential issue in static code analyzers like Fortify, which expect to see a finally block which closes the connection.
There was a problem hiding this comment.
hi @velo you reckon, this finally block is something we could incorporate in here?
There was a problem hiding this comment.
That would be really great. Thank you @velo
httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java
Outdated
Show resolved
Hide resolved
kdavisk6
added
the
waiting for feedback
final HttpEntity entity = httpResponse.getEntity();
if (entity == null) {
return null;
}apache response won't be closed in case of missing entity. |
Co-authored-by: Austin Pio <[email protected]>
* Fix for #1286 When feign is done with the response, also invoke close on http response (if closeable) * Update httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java Co-authored-by: Austin Pio <[email protected]> * Update ApacheHttpClient.java * Use finally block to close http5 client * Use finally block to close http5 client Co-authored-by: Kevin Davis <[email protected]> Co-authored-by: Austin Pio <[email protected]>
* Fix for #1286 When feign is done with the response, also invoke close on http response (if closeable) * Update httpclient/src/main/java/feign/httpclient/ApacheHttpClient.java Co-authored-by: Austin Pio <[email protected]> * Update ApacheHttpClient.java * Use finally block to close http5 client * Use finally block to close http5 client Co-authored-by: Kevin Davis <[email protected]> Co-authored-by: Austin Pio <[email protected]>
When feign is done with the response, also invoke close on http response (if closeable)
#1286