On Fri, 28 Aug 2026 08:17:18 GMT, Volkan Yazici <[email protected]> wrote:
>> src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java >> line 1564: >> >>> 1562: setFinalStream(); >>> 1563: return false; >>> 1564: } >> >> This does not look completely right - maybe it should do: >> >> if (timedOut) idleConnectionTimeoutEvent.handle()? > > `Http2Connection.IdleConnectionTimeoutEvent#handle()` re-acquires the state > lock, marks the idle connection for termination, releases its inner lock > acquisition, and tries to terminate the connection using > `connTerminator.idleTimedOut()`, yet the lock is still held by the outer > caller. That is, it will result in terminating the connection while the lock > is still held. This doesn't match what `handle()` is trying to do. Are you > sure about this change? > > FWIW, when I change the code in the way you suggested, the newly added tests > still pass. It's a good point. My worry here is that the connection may be removed from the list of connections without being properly terminated. Also we don't seem to check whether the idleTimeoutEvent has been cancelled. Are we sure that the event cannot be cancelled? The actions performed by the event when it is fired and finds that the connection should be terminated are much more involved than just calling finalStream. And the caller seems to only remove the connection from the list. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32545#discussion_r3879411707
