RFR: 8335181: Incorrect handling of HTTP/2 GOAWAY frames in HttpClient [v6]

Jaikiran Pai jpai at openjdk.org
Tue Aug 13 13:30:57 UTC 2024


On Tue, 13 Aug 2024 13:19:44 GMT, Jaikiran Pai <jpai at openjdk.org> 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 111 additional commits since the last revision:
> 
>  - also decrement streams to allow the HttpClient to close
>  - 8336384: AbstractQueuedSynchronizer.acquire should cancel acquire when failing due to a LinkageError or other errors
>    
>    Reviewed-by: alanb
>  - 8338202: Shenandoah: Improve handshake closure labels
>    
>    Reviewed-by: rkennke, ysr, wkemper
>  - 8334475: UnsafeIntrinsicsTest.java#ZGenerationalDebug assert(!assert_on_failure) failed: Has low-order bits set
>    
>    Reviewed-by: stefank, eosterlund, aboldtch
>  - 8338155: Fix -Wzero-as-null-pointer-constant warnings involving PTHREAD_MUTEX_INITIALIZER
>    
>    Reviewed-by: dholmes, dlong
>  - 8338124: C2 SuperWord: MulAddS2I input permutation still partially broken after JDK-8333840
>    
>    Reviewed-by: chagedorn, thartmann, kvn
>  - 8335628: C2 SuperWord: cleanup: remove SuperWord::longer_type_for_conversion
>    
>    Reviewed-by: chagedorn, kvn
>  - 8338154: Fix -Wzero-as-null-pointer-constant warnings in gtest framework
>    
>    Reviewed-by: ihse, dholmes, jwaters
>  - 8338248: PartialArrayStateAllocator::Impl leaks Arena array
>    
>    Reviewed-by: kbarrett, shade
>  - 8338160: Fix -Wzero-as-null-pointer-constant warnings in management.cpp
>    
>    Reviewed-by: dholmes
>  - ... and 101 more: https://git.openjdk.org/jdk/compare/7365f09b...fb4ecae7

Looks like I did some mistake with a "git merge" command locally when trying to refresh this PR with latest mainline changes. I'll close this one and open a new one with only the relevant commits.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/20442#issuecomment-2286255905


More information about the net-dev mailing list