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

[Daniel Fuchs]

On Mon, 22 Apr 2024 13:05:31 GMT, robert engels <duke at openjdk.org> wrote:

>> also, I reviewed the semantics of the buffered output stream. It is fully synchronized, so flushing here is fine and cannot corrupt the data to the client. But still, I don't think it is needed as it doesn't really solve anything if the api conditions are more clearly specified.
>
>> We don't want to allow it; we want to fix the broken tests, and add a release note so that the users know how to recognize that their code is broken and know how to fix it.
>> 
>> This should be fine in a new JDK version, where the users are expected to invest some effort in upgrading. Do you plan to backport this change to older releases? If you do, it would make sense to add a system property to revert to the old (slow) behavior. This would allow the users to use the updated version without recompiling their code.
> 
> Hi Daniel. I think there is still some disconnect - I apologize for not being clear. I don't think there will be anything to fix even if back-ported. The only thing to do would be to inform the users they no longer needs to add the 'no delay' option to achieve better network performance. Any existing client code must be closing the stream or exchange - or the code would not work - the clients would hang.
> 
> The only problem with the existing JDk impl is that there is very slow performance when doing so. This PR fixes the performance issue - it doesn't change the semantics of the communication. 
> 
> The only reason that a few test cases are breaking with this PR is that they are written incorrectly - but it was never discovered because it was launching the test in a new jvm and only performing a single call.
> 
> At least that is the way I see it - but there's been a lot of discussion so maybe I've missed something.

I can see that a bad handler could call `sendResponse(500, 0);` instead of `sendResponse(500, -1)`, and forget to close the body output stream or the exchange.  A client might just not read the body if it receives 500. Before the fix, the server will flush the headers and the client will receive them and everything would work. After the fix, the headers will not be flushed (as a chunked body is expected), and everything will hang (or eventually timeout).

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

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