On Mon, 28 Nov 2022 15:11:22 GMT, Jaikiran Pai <[email protected]> wrote:

>> Daniel Fuchs 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 five additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'master' into SpecialHeadersTest-8297200
>>  - Update test/jdk/java/net/httpclient/ReferenceTracker.java
>>    
>>    Co-authored-by: Andrey Turbanov <[email protected]>
>>  - Fixed race condition in detecting if selector is still alive
>>  - 8297200
>>  - 8297200
>
> test/jdk/java/net/httpclient/SpecialHeadersTest.java line 478:
> 
>> 476:                 if (!sameClient || client == null) {
>> 477:                     client = newHttpClient("testAsync", sameClient);
>> 478:                     tracker = TRACKER.getTracker(client);
> 
> Should this be outside this `if` block? Otherwise, in its current form, I 
> think this `tracker` can be `null` and then later in this test where we do 
> `TRACKER.check(tracker, ...)`, I believe it will lead to an NPE inside the 
> `check` implementation.

Please disregard this comment. I took a more closer look and what you have here 
is fine, since we only call the `TRACKER.check(tracker,...)` when `sameClient = 
false` in which case this above `if` block would already have set the `tracker`.

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

PR: https://git.openjdk.org/jdk/pull/11294

Reply via email to