RFR: 8309118: HttpClient: Add more tests for 100 ExpectContinue with HTTP/2 [v3]

[Conor Cleary]

On Mon, 9 Oct 2023 13:35:24 GMT, Daniel Fuchs <dfuchs at openjdk.org> 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 584:
> 
>> 582:     void incoming_reset(ResetFrame frame) {
>> 583:         Log.logTrace("Received RST_STREAM on stream {0}", streamid);
>> 584:         Flow.Subscriber<?> subscriber = responseSubscriber == null ? pendingResponseSubscriber : responseSubscriber;
> 
> Suggestion:
> 
>         Flow.Subscriber<?> subscriber = responseSubscriber;
>         if (subscriber == null) subscriber = pendingResponseSubscriber;
> 
> (avoids reading volatile twice)

Ah yes, I see I think. In either case of the conditional expression either `responseSubscriber` and `pendingResponseSubscriber` are read or `responseSubscriber` is read and `responseSubscriber` is read. I was confused for a bit because I thought only one operand is evaluated. But actually its the case that `responseSubscriber` is read twice when `responseSubscriber == null` evaluates to false.

Good suggestion!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15664#discussion_r1364079499