RFR: 6968351: httpserver clashes with delayed TCP ACKs for low Content-Length [v14]

[Michael McMahon]

On Tue, 23 Apr 2024 19:31:03 GMT, robert engels <duke at openjdk.org> wrote:

>> The PR has been updated to fix the tests.
>
> By the way, as an x-Googler I am familiar with Hyrum and have seen the link. 
> 
> Since the api allows for async handling - validating that an arbitrary handler closes the stream doesn't seem possible other than by testing that the client receives the data it expects, and that the connection does not hang when keep alive with multiple requests is enabled. So I don't think runtime checks could have caught it either.
> 
> If someone wrote a handler that relied on the fact that headers were sent "early", and/or that streams were flushed even though the api specification states the handler must close the stream - they would be coding to the sun implementation - and if they lacked access to the source code they would never be able to determine that is what was happening.
> 
> The sun tests in this regard are incorrectly testing/expecting behavior that the published api specifically prohibits.

> If the handler sends headers(code,0) and doesn't close the stream, the connection is dead. If what you are saying is that the old behavior was to send the headers at this point, the client would see it and terminate the connection - that feels like a lot of ifs.
> 
> but as I mentioned, adding the flush after the exchange handler chain call is safe and would catch this scenario - even if the handler returns the data synchronously - as the buffered output stream at the top of the stream is fully synchronized.

Yes, the connection is dead, but what I'm saying is sometimes that doesn't matter, because the client (if it is HttpURLConnection) is going to close it anyway. I agree this is not a common scenario we need to worry about, and a simple release note flag should be enough to cover it. So, I don't think we are disagreeing on anything substantive here.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1577503119