> 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...
Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: quiet vs quite ------------- Changes: - all: https://git.openjdk.org/jdk/pull/32078/files - new: https://git.openjdk.org/jdk/pull/32078/files/16cb3fa5..cfc7979e Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=32078&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=32078&range=00-01 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/32078.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/32078/head:pull/32078 PR: https://git.openjdk.org/jdk/pull/32078
