RFR: 8266310: deadlock while loading the JNI code [v5]

[David Holmes]

On 25/05/2021 7:53 am, Aleksei Voitylov wrote:
> On Mon, 24 May 2021 06:24:15 GMT, David Holmes <dholmes at 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
>