On 25/05/2021 7:53 am, Aleksei Voitylov wrote:
On Mon, 24 May 2021 06:24:15 GMT, David Holmes <dhol...@openjdk.org> wrote:
Aleksei Voitylov has updated the pull request incrementally with one additional
commit since the last revision:
fix whitespace
src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 511:
509: if (currentLock.getCounter() == 1) {
510: // unlock and release the object if no other threads are
queued
511: currentLock.unlock();
This isn't atomic so I don't see how it can work as desired. Overall I don't understand
what the semantics of this "counted lock" are intended to be.
Hi David,
The locking strategy is as follows:
CountedLock is a subclass of ReentrantLock that allows exact counting of
threads that intend to acquire the lock object. Each time a thread calls
acquireNativeLibraryLock() with a certain name, either a new CountedLock object
is allocated and assigned 1 as the counter, or an existing CountedLock is
incremented prior to invocation of the lock() method on it. The increment
operation to the lock object is performed in the context of execution of
compute() method, which is executed atomically. This allows to correctly
dispose the CountedLock object when the last thread in the queue leaves
releaseNativeLibraryLock(). The entire remapping function passed to
computeIfPresent() method is executed atomically.
So the counting is trying to be an in-use count so that it can be
deleted when not needed? I'm still not clear on exactly what this
counting is doing (partly because I have trouble reading the lambda
expressions that relate to the lock).
Could you be more specific on what is not performed atomically?
The code I referenced. The counter is not protected by the lock that it
counts. In the code above we hold the lock and check if the counter == 1
before unlocking. But the counter is incremented without the lock held,
so as soon as we have called getCounter() the count could have changed.
David
-----
-------------
PR: https://git.openjdk.java.net/jdk/pull/3976
