RFR: 8304701: Request with timeout aborts later in-flight request on HTTP/1.1 cxn [v3]
Daniel Fuchs
dfuchs at openjdk.org
Mon Nov 20 18:03:07 UTC 2023
On Mon, 20 Nov 2023 10:11:52 GMT, Conor Cleary <ccleary at openjdk.org> wrote:
>> **Problem**
>> When using HTTP/1.1 with HttpClient, it was observed that requests configured with timeouts at build time fail with a HttpTimeoutException when they are redirected to a separate URI by a server (status code 3xx on first response). What should happen is that the second request response (so after receiving a 3xx code) clears restarts the timer intially set. However, when `responseTimerEvent` is registered for the first time, it is not unregistered and cleared before starting a second timer.
>>
>> **Solution**
>> This fix addresses the issue by calling `cancelTimer()` in `MultiExchange.java` whenever the `newRequest` reference is set to a non-null value after calling `responseFilters(response)` on the initial response received. This occurs in the case where a status code 3xx is received in the initial response. When `cancelTimer()` is called, it now unreferences `responseTimerEvent` after cancellation operations take place.
>>
>> While the fix for the issue was relatively straight forward, the regression test is less so. I would point to to the comment located in `RedirectTimeoutTest:L119` for an explanation of the testing method.
>
> Conor Cleary has updated the pull request incrementally with one additional commit since the last revision:
>
> 8304701: Increased timeout values again for test stability
test/jdk/java/net/httpclient/RedirectTimeoutTest.java line 69:
> 67: static URI h1Uri, h1RedirectUri, h2Uri, h2RedirectUri, h2WarmupUri, testRedirectURI;
> 68: private static final long TIMEOUT_MILLIS = 1500L;
> 69: private static final long SLEEP_TIME = 750L;
Maybe you should use `jdk.test.lib.Utils.adjustTimeout` here to make sure those timeouts are bumped up when running in higher tiers. 1500 ms is small - so if need be you can bump that again (or is it the old value?).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16689#discussion_r1399560337
More information about the net-dev
mailing list