On Mon, 22 Apr 2024 12:44:58 GMT, robert engels <d...@openjdk.org> wrote:

>> src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 851:
>> 
>>> 849:                     uc.doFilter (new HttpExchangeImpl (tx));
>>> 850:                 }
>>> 851:                 tx.getResponseBody().flush();
>> 
>> I admire the creativity, but I don't think we can do that here. At this 
>> point the stream can be owned by another thread, either because the response 
>> was completed and we are now handling another request, or because the user 
>> decided to handle the request on a separate thread started from `handle`. In 
>> either case, flushing here could lead to concurrency issues like sending the 
>> wrong data to the user.
>
> I reverted the commit. I think my comment that the TcpNoDelayNotRequired 
> hangs the client connection if the stream is not closed - in the existing JDK 
> code - shows that the concern regarding "existing code" is not valid. I think 
> it only these test cases that exhibit the bad behavior and it was never 
> detected because the tests are run in isolation in a new jvm each time.

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.

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

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

Reply via email to