Improve connection reuse logic

[agherardi]

This is still a work-in-progress, at this point I'm just soliciting feedback.

The purpose of this change is to improve connection reuse by detecting that the content stream of an HTTP response entity is already at EOF before the stream's read() method returns -1. This is useful when the application does not want to call the close() method of the content stream, since that method can potentially block.

[ok2c]

@agherardi Could you please help me better understand what you are trying to achieve? Why would you want to avoid calling #close method when it is a part of InputStream contract? I also do not quite understand why blocking in #close is a problem given that InputStream is inherently blocking.

[agherardi]

My HTTP stack consists of Jersey https://github.com/jersey/jersey with the Apache httpclient connector and the JacksonJsonProvider. I'm using the stack to talk to a REST-ful server that returns JSON responses, and have Jersey + JacksonJsonProvider automatically deserialize JSON responses to Java POJOs. The approach is similar to the one described here https://dennis-xlc.gitbooks.io/restful-java-with-jax-rs-2-0-en/cn/part1/chapter8/building_and_invoking_requests.html

While testing, I noticed that HTTP and HTTPS connections are not reused - i.e., the stack opens and closes the HTTP/HTTPS connection for every REST call. The underlying problem is that the Jersey httpclient connector closes the CloseableHttpResponse BEFORE closing the response's content input stream.

I can think of a couple of ways to solve this issue. One is to reverse the order in which the Jersey connector closes things - i,e,, first close the input stream, then close the CloseableHttpResponse. I have a merge request out to the Jersey maintainers https://github.com/jersey/jersey/pull/3752 for that.

The other possibility is to modify httpclient along the lines described in this merge request. The idea is to make EofSensorInputStream#checkEOF more proactive in detecting when a response has been consumed and release the connection to the pool without waiting for a close().

[ok2c]

@agherardi OK. I see. While it is not pretty I understand the rationale for the work-around. There are cases when clients might want to consume the payload which has some sort of format delineation and stop reading without getting -1 (end of stream) result that would trigger release of the underlying connection.

Please let me know if you want me to merge the changes or you still intent to do more work on it

[agherardi]

There are cases when clients might want to consume the payload which has some sort of format delineation and stop reading without getting -1 (end of stream) result that would trigger release of the underlying connection.

Yes, that's exactly the point.

Please let me know if you want me to merge the changes or you still intent to do more work on it

Now that I know that this is acceptable, please give me some time to do more work and add some unit tests. Thanks!

[agherardi]

After running some tests, I realized that there isn't a bullet-proof way to detect EOF without blocking when the server uses chunked encoding. The solution I came up with only works if the SessionInputBuffer happens to have the chunk header with size=0 already buffered, and that's not always the case.

Therefore, I believe the best option is for the app - and in my case the Jersey Apache connector https://github.com/jersey/jersey/pull/3752 - to close the entity stream before closing the response.

I'm closing this PR. Thank you for your help.