On Wed, 29 Jul 2026 10:12:23 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 tha... > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > quiet vs quite Marked as reviewed by djelinski (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/32078#pullrequestreview-4808705982
