On Mon, 1 Dec 2025 10:41:11 GMT, Daniel Fuchs <[email protected]> wrote:
>> Volkan Yazici has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Introduce dangling `firstValueAsLong("content-length")` calls back
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java line
> 338:
>
>> 336: } catch (ProtocolException pe) {
>> 337: cf.completeExceptionally(pe);
>> 338: return cf;
>
> We're not going to read the body if we return here, so I think we should
> close the connection with the ProtocolException `pe` before returning the
> completable future `cf`. Otherwise - the connection will remained unclosed
> and out of the pool until we close the client.
Doh! Very good catch. I've fixed this in 024e355f034. I've copied the
`errorHandler` we pass to `ResponseSubscribers::getBodyAsync`. Note that I've
slightly changed the `errorHandler` to have a `finally` block:
try {
subscriber.onError(error);
cf.completeExceptionally(error);
} finally {
asyncReceiver.setRetryOnError(false);
asyncReceiver.onReadError(error);
}
This is what the `executor.execute(() -> ...)` block does too, which I presume
to guard against misbehaving `subscriber::onError` and
`cf::completeExceptionally`.
@dfuch, I'm not resolving this conversation yet. Would you mind reviewing
024e355f034 and letting me know if it is okay, please?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2588964307
