On Mon, 9 Oct 2023 13:32:17 GMT, Daniel Fuchs <[email protected]> wrote:
>> Conor Cleary has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains ten additional
>> commits since the last revision:
>>
>> - 8309118: Fixed error in handleReset ReentrantLock
>> - 8309118: Updates to Stream cancelImpl
>> - 8309118: Refactored test, updated incoming_reset
>> - Merge branch 'master' into JDK-8309118
>> - 8309118: Cleanup identifiers and whitespace
>> - 8309118: Removed unused try-with-resources
>> - 8309118: Improve comments and annotations
>> - 8309118: Remove local test timeout value
>> - 8309118: Add more tests for 100 ExpectContinue with HTTP/2
>
> src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 193:
>
>> 191: if (frame instanceof ResetFrame rf) {
>> 192: inputQ.remove();
>> 193: if (endStreamReceived()) {
>
> Suggestion:
>
> if (endStreamReceived() && rf.getErrorCode() ==
> ResetFrame.NO_ERROR) {
So if we have received an end stream but receive an error code, do we want to
complete exceptionally?
The suggestion seems correct, just checking 👍
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15664#discussion_r1364033231
