On Wed, 24 May 2023 10:35:16 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 pull request now contains 11 commits: >> >> - Merge branch 'master' into HttpClient-Logging-8308310 >> - Merge branch 'master' into HttpClient-Logging-8308310 >> - Fix whitespace >> - make stateLock final >> - Add debug traces to ExpectContinueTest.java >> - failedRef should be final >> - Align parameters >> - Update >> src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java >> >> Co-authored-by: Andrey Turbanov <[email protected]> >> - Update >> src/java.net.http/share/classes/jdk/internal/net/http/ConnectionPool.java >> >> Co-authored-by: Andrey Turbanov <[email protected]> >> - Update test/jdk/java/net/httpclient/AuthFilterCacheTest.java >> >> Co-authored-by: Andrey Turbanov <[email protected]> >> - ... and 1 more: https://git.openjdk.org/jdk/compare/c0c4d771...c5d2f1f2 > > 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 ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/14038#discussion_r1203899099
