On Fri, 24 Nov 2023 14:41:48 GMT, Conor Cleary <[email protected]> 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
