RFR: 8335181: Incorrect handling of HTTP/2 GOAWAY frames in HttpClient
Can I please get a review of this change which fixes the issue noted in https://bugs.openjdk.org/browse/JDK-8335181? As noted in that issue, the current implementation in the `java.net.http.HttpClient` doesn't correctly handle an incoming GOAWAY frame. The HTTP3 RFC https://www.rfc-editor.org/rfc/rfc9113#name-goaway notes the specifics on what the expectations are when an endpoint receives a GOAWAY frame from the peer. Before the changes proposed in this PR, the HttpClient implementation would (incorrectly) shutdown the connection and abort requests when a GOAWAY frame was received. The changes in this PR fixes that by retrying relevant unprocessed requests (if any) and not initiating any new streams on the connection. A new test has been introduced to exercise this detail. The test continues to pass along with other existing tests. tier testing as well as a repeated testing (with test-repeat 50) is currently in progress with this change. - Commit messages: - 8335181: Incorrect handling of HTTP/2 GOAWAY frames in HttpClient Changes: https://git.openjdk.org/jdk/pull/20442/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20442&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8335181 Stats: 554 lines in 8 files changed: 518 ins; 0 del; 36 mod Patch: https://git.openjdk.org/jdk/pull/20442.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20442/head:pull/20442 PR: https://git.openjdk.org/jdk/pull/20442
Re: RFR: 8335181: Incorrect handling of HTTP/2 GOAWAY frames in HttpClient [v2]
> Can I please get a review of this change which fixes the issue noted in > https://bugs.openjdk.org/browse/JDK-8335181? > > As noted in that issue, the current implementation in the > `java.net.http.HttpClient` doesn't correctly handle an incoming GOAWAY frame. > The HTTP3 RFC https://www.rfc-editor.org/rfc/rfc9113#name-goaway notes the > specifics on what the expectations are when an endpoint receives a GOAWAY > frame from the peer. > > Before the changes proposed in this PR, the HttpClient implementation would > (incorrectly) shutdown the connection and abort requests when a GOAWAY frame > was received. The changes in this PR fixes that by retrying relevant > unprocessed requests (if any) and not initiating any new streams on the > connection. > > A new test has been introduced to exercise this detail. The test continues to > pass along with other existing tests. tier testing as well as a repeated > testing (with test-repeat 50) is currently in progress with this change. Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: add code comment to a test class - Changes: - all: https://git.openjdk.org/jdk/pull/20442/files - new: https://git.openjdk.org/jdk/pull/20442/files/2fd3c89e..54ce899b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=20442&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=20442&range=00-01 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/20442.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/20442/head:pull/20442 PR: https://git.openjdk.org/jdk/pull/20442
Re: RFR: 8335181: Incorrect handling of HTTP/2 GOAWAY frames in HttpClient [v2]
On Fri, 2 Aug 2024 11:24:07 GMT, Jaikiran Pai wrote: >> Can I please get a review of this change which fixes the issue noted in >> https://bugs.openjdk.org/browse/JDK-8335181? >> >> As noted in that issue, the current implementation in the >> `java.net.http.HttpClient` doesn't correctly handle an incoming GOAWAY >> frame. The HTTP3 RFC https://www.rfc-editor.org/rfc/rfc9113#name-goaway >> notes the specifics on what the expectations are when an endpoint receives a >> GOAWAY frame from the peer. >> >> Before the changes proposed in this PR, the HttpClient implementation would >> (incorrectly) shutdown the connection and abort requests when a GOAWAY frame >> was received. The changes in this PR fixes that by retrying relevant >> unprocessed requests (if any) and not initiating any new streams on the >> connection. >> >> A new test has been introduced to exercise this detail. The test continues >> to pass along with other existing tests. tier testing as well as a repeated >> testing (with test-repeat 50) is currently in progress with this change. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > add code comment to a test class I'm investigating an intermittent failure that happened once with this change. I will update this PR once I have additional details. - PR Comment: https://git.openjdk.org/jdk/pull/20442#issuecomment-2265459748
