RFR: 8304701: Request with timeout aborts later in-flight request on HTTP/1.1 cxn [v4]

Daniel Fuchs dfuchs at openjdk.org
Fri Nov 24 16:08:06 UTC 2023


On Fri, 24 Nov 2023 14:41:48 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: Added adjustTimeout to make us of timeout factor

Marked as reviewed by dfuchs (Reviewer).

Thanks for adjusting the timeout. If the test is stable with the new parameters then it looks good to me.

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

PR Review: https://git.openjdk.org/jdk/pull/16689#pullrequestreview-1748174115
PR Comment: https://git.openjdk.org/jdk/pull/16689#issuecomment-1825861677


More information about the net-dev mailing list