RFR: 8208693: HttpClient: Extend the request timeout's scope to cover the response body
Daniel Fuchs
dfuchs at openjdk.org
Wed Oct 22 12:33:15 UTC 2025
On Wed, 24 Sep 2025 13:24:35 GMT, Volkan Yazici <vyazici at openjdk.org> wrote:
> Currently `HttpRequest::timeout` only applies until the response headers are received. Extend its scope to also cover the consumption of the response body.
>
> ### Review guidelines
>
> 1. Read _"the fix"_ in `MultiExchange`
> 2. Skim through the test server *handler* in `TimeoutResponseTestSupport`
> 3. Review first `TimeoutResponseHeaderTest`, and then `TimeoutResponseBodyTest` (Mind the multiple `@test` blocks!)
src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java line 414:
> 412: return handleNoBody(r, exch);
> 413: }
> 414: return exch.readBodyAsync(responseHandler, this::cancelTimer)
Could we remove the `preTerminationCallback` parameter, and instead have the various subclass of `HttpBodySubscriberWrapper` supply `this.multi::cancelTimer()` to their super class?
If I'm not mistaken all concrete subclasses of `HttpBodySubscriberWrapper` have access to the multi exchange.
That should reduce the amount of changes significantly.
Another possibility is to introduce a noarg protected abstract method in `HttpBodySubscriberWrapper` and have all subclasses implement it.
We could call it for instance `protected abstract void onTerminationNotified();` and explain that it can be used to cancel a timer if needed.
It might need to be called on cancel as well?
If not a comment explaining why might be good to have. If it's not called by cancel then maybe the method suggested above could be changed to `onCompletionNotified`.
src/java.net.http/share/classes/jdk/internal/net/http/common/HttpBodySubscriberWrapper.java line 412:
> 410: if (preTerminationCallback != null) {
> 411: preTerminationCallback.run();
> 412: }
Did you consider adding this to complete() instead? There would then be a single place where this code would appear.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2451899504
PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2451927047
More information about the net-dev
mailing list