RFR: 8308310: HttpClient: Avoid logging or locking from within synchronized blocks [v8]

[Jaikiran Pai]

On Wed, 24 May 2023 10:59:02 GMT, Daniel Fuchs <dfuchs at openjdk.org> 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