RFR: 8324209: Check implementation of Expect: 100-continue in the java.net.http.HttpClient [v3]

[Jaikiran Pai]

On Mon, 12 Aug 2024 14:22:13 GMT, Darragh Clarke <dclarke at openjdk.org> wrote:

>> Currently `HttpClient` will timeout if a server doesn't respond to a request which includes `Expect: 100-Continue`
>> 
>> Section 10.1.1 of [rfc 9110](https://datatracker.ietf.org/doc/html/rfc9110#name-expect) states that
>> 
>> a client SHOULD NOT wait for an indefinite period before sending the content.
>> 
>> 
>> 
>> This PR changes `HttpClient` to wait for a maximum of 5 seconds for a server response, this will be shorter if a timeout is set. If no response is received, the message will be sent regardless. 
>> This should bring `HttpClient` in line with how [HttpUrlConnection](https://github.com/DarraghClarke/jdk/blob/61386c199a3b29457c002ad31a23990b7f6f88fd/src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java#L1305) treats expect continue timeouts.
>> 
>> This is done using `orTimeout` in the `expectContinue` method , though there is some changes in `streams.java` where it was possible for race conditions to cause timeouts where `CompleteableFuture`s were removed from `response_cfs` prematurely or in some cases not removed at all.
>> 
>> I've tested this against tiers 1-3 and it appears to be stable.
>
> Darragh Clarke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove duplicate method

src/java.net.http/share/classes/jdk/internal/net/http/Exchange.java line 493:

> 491:                         exchImpl.setExpectTimeoutRaised(true);
> 492:                         CompletableFuture<Response> cf =
> 493:                                 exchImpl.sendBodyAsync()

When a timeout occurs due to `orTimeout()` step of a CompletableFuture pipeline, the subsequent stages in that pipeline get invoked on a thread that's internal to the `java.util.concurrent` code and isn't the one that's part of the executor that's configured on our HttpClient.
Looking at `sendBodyAsync()` which gets invoked if there's a timeout, I suspect it's OK if that method gets invoked through this internal thread. Daniel, do you see any issues if this internal thread ends up calling the `sendBodyAsync()` here?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20525#discussion_r1716671574