On Wed, 24 May 2023 10:59:02 GMT, Daniel Fuchs <[email protected]> wrote:
>> src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java
>> line 476:
>>
>>> 474: }
>>> 475:
>>> 476: // should only be called while holding the ConnectionPool
>>> stateLock.
>>
>> Hello Daniel, all these methods which say a lock needs to be held when they
>> are called, should we add a `assert stateLock.isHeldByCurrentThread();` to
>> make it verifiable? I understand we didn't have a similar assert when the
>> comment said the method needs to be called while holding the monitor, but
>> since we are changing this part now, perhaps we can add those asserts?
>
> The `stateLock` variable is not available from within the method:
>
> Non-static field 'stateLock' cannot be referenced from a static context
Thank you, that wasn't clearly noticable in the diff.
>> src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java
>> line 150:
>>
>>> 148:
>>> 149: public void tryRelease() {
>>> 150: if (STATE.compareAndSet(this, (byte)0x01, (byte)0x03)) {
>>
>> Nit - a space is missing between the cast and the value for the second and
>> third parameters.
>
> OK - will do - though I usually don't put a space there.
It's fine with me in the current form if this wasn't an oversight.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14038#discussion_r1203912652
PR Review Comment: https://git.openjdk.org/jdk/pull/14038#discussion_r1203918122
