On Wed, 12 Nov 2025 09:30:09 GMT, Volkan Yazici <[email protected]> wrote:
>> Jaikiran Pai has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> trivial debug log check change
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java
> line 2056:
>
>> 2054: private void doTerminate() {
>> 2055: final Http2TerminationCause tc = terminationCause.get();
>> 2056: assert tc != null : "missing termination cause";
>
> `doTerminate()` can receive the termination cause from `terminate()`, which
> would render these two lines redundant.
Actually, this one was intentional. I wanted to avoid introducing a
`doTerminate(cause)` (even if `private`) which could then be invoked from
arbitrary places with different causes.
In doTerminate() I always want it to use the cause that's already set and
setting of that cause must always happen in `terminate(cause)`.
terminate(cause) itself is allowed to be called from numerous places and it has
the necessary code to synchronize in a way that allows setting the cause just
once and then calling doTerminate() just once.
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java
> line 2076:
>
>> 2074: Log.logError("Closing connection due to: {0}", tc);
>> 2075: } else {
>> 2076: if (debug.on()) {
>
> You can consider simplifying this as `else if (debug.on())`.
Good catch. Fixed in the updated PR.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2533823365
PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2533830866
