On Wed, 12 Nov 2025 09:08:43 GMT, Volkan Yazici <[email protected]> wrote:
>> Jaikiran Pai has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 12 additional >> commits since the last revision: >> >> - rename isErroneousClose() to isAbnormalClose() >> - stackless instance for idle timed out NoError >> - Volkan's suggestion - log message improvement >> - merge latest from master branch >> - mark jdk.internal.net.http.Http2Connection as Closable >> - reduce number of concurrent requests >> - cleanup >> - update test >> - Return false from isOpen() if underlying channel is not open >> - Daniel's review suggestion - stop the scheduler when TubeSubscriber >> errored or completed >> - ... and 2 more: https://git.openjdk.org/jdk/compare/3b9f58c2...1c3f73ba > > src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java > line 234: > >> 232: if (cancelled) { >> 233: if (debug.on()) { >> 234: debug.log("Not initiating idle connection >> close"); > > Suggestion: > > debug.log("Connection is already cancelled, skipping > idle connection close"); I updated the PR with a slightly different message - it's not the connection which is cancelled but it is the idle timeout event which is cancelled. > src/java.net.http/share/classes/jdk/internal/net/http/Http2TerminationCause.java > line 144: > >> 142: */ >> 143: public static Http2TerminationCause idleTimedOut() { >> 144: return new NoError("HTTP/2 connection idle timed out", "idle >> timed out"); > > Any particular reason this is not cached in `NoError.IDLE_TIMED_OUT` in a > similar manner to `NoError.INSTANCE`? I could not see a place where its stack > trace would be of value. You are right, the idle timed out instance could also do away with the stacktrace. I have updated the PR with this change. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2533799723 PR Review Comment: https://git.openjdk.org/jdk/pull/28233#discussion_r2533797755
