On Wed, 24 Apr 2024 08:31:14 GMT, Michael McMahon <micha...@openjdk.org> wrote:
>> 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. Yep. Just let me know if there is anything else you need me to do. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/18667#discussion_r1577677619
