RFR: 8326498: java.net.http.HttpClient connection leak using http/2 [v5]

[Jaikiran Pai]

On Mon, 17 Nov 2025 15:40:30 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix typo in test code comment
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line 2022:
> 
>> 2020:         if (debug.on()) {
>> 2021:             debug.log("sending GOAWAY " + goAway);
>> 2022:         }
> 
> Suggestion:
> 
>         if (Log.trace()) {
>             Log.logTrace("{0} sending GOAWAY {1}", connection, goAway);
>         } else if (debug.on()) {
>             debug.log("sending GOAWAY " + goAway);
>         }

Done, updated the PR with this change.

> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line 2083:
> 
>> 2081:                         ", GOAWAY sent=" + goAwaySent.get();
>> 2082:                 debug.log("Closing connection (" + stateStr + ") due to: " + tc);
>> 2083:             }
> 
> The message logged for debug is sufficiently different from the one logged with Log that we should consider actually logging both without fearing the repetition.
> Either that, or consider logging the same message in both cases, in which case the else could stay.
> 
> Suggestion:
> 
>             if (Log.errors()) {
>                 Log.logError("Closing connection due to: {0}", tc);
>             }
>             if (debug.on()) {
>                 final String stateStr = "Abnormal close=" + tc.isAbnormalClose() +
>                         ", has active streams=" + isActive() +
>                         ", GOAWAY received=" + goAwayRecvd.get() +
>                         ", GOAWAY sent=" + goAwaySent.get();
>                 debug.log("Closing connection (" + stateStr + ") due to: " + tc);
>             }

I decided to use the same level of information in both these logs. While at it, it felt odd to use `Log.logError()` to log a connection being closed, but it looks like we don't have a more specific/appropriate log method for this case. So I let it stay `Log.logError()` for now.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2541345597
PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2541351458