On Thu, 4 Dec 2025 12:53:39 GMT, Volkan Yazici <[email protected]> wrote:
>> 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?
Looks OK - out of curiosity, is the try-finally simple hygienic or did you
observe a case where it was required?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2608027125
