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 connections that received a `GOAWAY` when there was no active stream on the connection, will be marked as unusable and stay idle for several seconds/minutes before they actually get closed by the idle timeout management in the `HttpClient`. This can be improved and the connection can be promptly closed. The change in this PR addresses this issue. When the `GOAWAY` is received, the `HttpClient` implementation continues to mark the connection as unusable for subsequent requests and will now additionally check if there are no more active streams on that connection. If there aren't any, then it goes ahead and closes the connection, with a termination cause represented by the incoming `GOAWAY` frame. A new test has been introduced which reproduces the issue and verifies the fix. I have run these changes with a test repeat of 50 and even run tier2, all tests including the new one continue to pass. --------- - [x] I confirm that I make this contribution in accordance with the [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). ------------- Commit messages: - test code comment - trivial test rename - 8385131: HTTP/2 connection not closed after receiving GOAWAY frame when no streams are active - 8385131: working testcase Changes: https://git.openjdk.org/jdk/pull/32078/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=32078&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8385131 Stats: 523 lines in 2 files changed: 522 ins; 0 del; 1 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
