On Thu, 11 Dec 2025 12:11:46 GMT, Jaikiran Pai <[email protected]> wrote:
>> EunHyunsu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8371903: Use Utils.adjustTimeout() in GoAwayWithErrorTest
>
> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java
> line 1473:
>
>> 1471: numUnprocessed.incrementAndGet();
>> 1472: } else {
>> 1473: stream.connectionClosing(cause.getCloseCause());
>
> Hello @ehs208, I think this should be done a bit differently. Here we are
> merely constructing a `TerminationCause` and then closing the streams with
> that termination cause. But that doesn't guarantee that the connection will
> have been closed with this termination cause. I think what we should do here
> is merely mark the unprocessed streams with closeAsUnprocessed() and then
> just call `close(Http2TerminationCause.forH2Error(errorCode, errorMsg);`. The
> implementation of the `Http2Connection.Terminator.doTerminate()` already has
> the necessary logic to close all these (processed) streams with the
> termination cause. So I think this code should look something like:
>
> final byte[] debugData = frame.getDebugData();
> final String debugInfo = debugData.length > 0
> ? new String(debugData, UTF_8)
> : "";
>
> ...
>
> streams.forEach((id, stream) -> {
> if (id > lastProcessedStream) {
> stream.closeAsUnprocessed();
> numUnprocessed.incrementAndGet();
> }
> });
> ...
> // closes the connection as well as the rest of the (processed) streams
> close(Http2TerminationCause.forH2Error(errorCode, errorMsg));
>
> I haven't tested this myself.
@jaikiran won't the code at line 1483 below (`close(cause)`) ensure that the
connection is actually closed?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28632#discussion_r2610366237
