On Wed, 12 Nov 2025 10:14:27 GMT, Volkan Yazici <[email protected]> wrote:
>> Jaikiran Pai has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - mark jdk.internal.net.http.Http2Connection as Closable
>> - reduce number of concurrent requests
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java
> line 889:
>
>> 887: * If the connection hasn't yet been terminated then this method
>> returns an empty Optional.
>> 888: */
>> 889: final Optional<IOException> getTerminationException() {
>
> There is one place this method is used and it does
> `getTerminationException().orElse(null)`. I guess we can drop the `Optional`
> ceremony.
I removed this method in the latest update to this PR.
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java
> line 2134:
>
>> 2132: // that way when Http2Connection.isOpen() returns
>> false in that situation, then this
>> 2133: // getTerminationCause() will return a termination
>> cause.
>> 2134: terminate(Http2TerminationCause.forException(new
>> IOException("channel is not open")));
>
> Terminating the connection in a getter doesn't feel all right. Would you mind
> elaborating on the rationale, the absence of a better alternative, please?
We spoke about this detail offline and I decided to rename the method to
`determineTerminationCause()` instead of calling it `getTerminationCause()`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2541343245
PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2541336328
