On Wed, 29 Jul 2026 06:51:55 GMT, Jaikiran Pai <[email protected]> wrote:
> Can I please get a review of this change which proposes to fix the issue > reported in https://bugs.openjdk.org/browse/JDK-8385131? > > HTTP/2 specifies a `GOAWAY` frame that can be sent/received on a connection. > When a server sends a `GOAWAY` frame, it's an indication that the connection > will no longer be used for any subsequent requests. The `GOAWAY` may be sent > when there are active streams on the connection or when the connection is > idle without any active streams. > > The current implementation in the JDK's `HttpClient`, when it receives the > `GOAWAY` frame, will mark the connection as no longer usable for any new > requests issued through the HttpClient instance. In compliance with the > specification, the `HttpClient` will also mark any active streams as > "unprocessed" (so that they can be tried on a new connection) if those active > streams have an id higher than the last processed stream noted in the > `GOAWAY` frame. For all other active streams (if any), the `HttpClient`, in > compliance with the specification, will allow them to reach their completion. > > The HTTP/2 stream is represented by the `jdk.internal.net.http.Stream` class > in the JDK. Whenever an active `Stream` completes execution and is being > closed, it will check if the connection is marked as no longer usable for > newer requests and whether the current `Stream` is the last one being closed. > If it is, then it goes ahead and closes the connection as well. And that's > the right thing to do. This takes care of closing the connection for the case > where a `GOAWAY` was received by the `HttpClient` when one or more > `Stream`(s) were still active - once the last active `Stream` is closed, the > connection gets closed too. > > What's missing in the current implementation is the case where the > `HttpClient` receives a `GOAWAY` when there are no more active streams on the > connection. The current implementation merely marks the connection as no > longer usable for subsequent new requests, but doesn't close it. And since > there are no active `Stream`(s), the connection close logic in the `Stream` > class will not close the connection either. That then leaves the connection > idle, and the `HttpClient`'s idle connection manager is then responsible for > closing the connection when it reaches the idle timeout duration. Idle > timeout durations are configurable by the application and have a default in > the JDK's implementation. Typically those timeouts are in higher seconds and > sometimes even in minutes. That then means that these HTTP/2 con... Looks good. test/jdk/java/net/httpclient/http2/H2GoAwayPromptConnectionClose.java line 110: > 108: } > 109: > 110: private static void closeQuitely(final AutoCloseable closeable) { Suggestion: private static void closeQuietly(final AutoCloseable closeable) { ------------- Marked as reviewed by djelinski (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/32078#pullrequestreview-4806336301 PR Review Comment: https://git.openjdk.org/jdk/pull/32078#discussion_r3672790032
