On Tue, 7 Mar 2023 15:05: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 three additional
>> commits since the last revision:
>>
>> - 8293786: Server-side reset implemented, updated comments on incoming_reset
>> - Merge remote-tracking branch 'upstream/master' into JDK-8293786
>> - 8293786: HttpClient will not send more than 64 kb of data from the 2nd
>> request in http2
>
> src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 560:
>
>> 558: // response to be read before the Reset is handled in
>> the case where the client's
>> 559: // input stream is partially consumed or not consumed
>> at all by the server.
>> 560: requestBodyCF.complete(null);
>
> Doesn't seem right: if the error code != NO_ERROR we should probably complete
> the requestBodyCf exceptionally?
There is now a `completeExceptionally(new IOException("..."))` call is if
`frame.ErrorCode() != NO_ERROR`. Otherwise, `complete(null)` is called.
> test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http2/BodyInputStream.java
> line 44:
>
>> 42: final Queue<Http2Frame> q;
>> 43: final int streamid;
>> 44: boolean closed;
>
> Maybe `closed` should be volatile as well
Agreed, this is the case now
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12694#discussion_r1160711042
PR Review Comment: https://git.openjdk.org/jdk/pull/12694#discussion_r1160709911
