On Wed, 29 Oct 2025 14:27:43 GMT, Volkan Yazici <[email protected]> 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!)
>
> Volkan Yazici has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Replace wrapper's `preTerminationCallback` argument with a method to be 
> extended

src/java.net.http/share/classes/jdk/internal/net/http/common/HttpBodySubscriberWrapper.java
 line 79:

> 77:      * A callback to be invoked before termination, whether due to the
> 78:      * completion or failure of the subscriber, or cancellation of the
> 79:      * subscription.

I'd suggest to also say that when a subscription is cancelled, onTermination() 
is called before onCancel().

src/java.net.http/share/classes/jdk/internal/net/http/common/HttpBodySubscriberWrapper.java
 line 102:

> 100:                 } finally {
> 101:                     if (markCancelled()) {
> 102:                         onTermination();

I was  wondering if it would be better to have the subclasses call the 
appropriate method from `onCancel()` rather than have both `cancel()` and 
`complete()` call `onTermination()`. 
However - thinking about it again, I believe that calling onTermination() from 
cancel() is the right call. 

FWIW - we should also make the private class `SubscriptionWrapper` final.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2478386175
PR Review Comment: https://git.openjdk.org/jdk/pull/27469#discussion_r2478212917

Reply via email to