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

[Aleksei Voitylov]

On Wed, 19 May 2021 18:47:52 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Aleksei Voitylov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fix trailing whitespace
>
> src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 262:
> 
>> 260:         } finally {
>> 261:             releaseNativeLibraryLock(name);
>> 262:         }
> 
> The new locking scheme looks incorrect to me. It now seems possible for 2 different class loaders in 2 different threads to load the same library (which should not be possible).
> 
> Thread 1:
>   - acquires name lock
>   - checks library name is already in `loadedLibraryNames` (it's not)
>   - release name lock
>   - start loading library
>   
> Then thread 2 comes in and does the same
>   
> Then again thread 1 finishes loading and:
>  - acquires name lock
>  - puts library name in `loadedLibraryNames`
>  - puts library name in it's own `libraries`
>  - release lock
>  
> Then thread 2 comes in and does the same again (although adding the name to `loadedLibraryNames` will silently return `false`).
> 
> It seems that this scenario is possible, and the library will be loaded by 2 different class loaders, each with their own `lib` object. (If there's a race, there has to be a loser as well, which would need to discard their work when finding out they lost)
> 
> You might be able to stress this by checking if `loadedLibraryNames.add(name);` returns `true`, and throwing an exception if not.
> 
> I think a possible solution would be to claim the library name up front for a particular loader. Then 2 threads can still race to load the same library for the same class loader, but 2 threads with 2 different class loaders racing to load the library should not be possible.
> 
> Actually, you might be able to prevent a race on JNI_OnLoad altogether by claiming the library name for a particular thread upfront as well (e.g. using a `ConcurrentHashMap<String, Thread>`).

Thank you, the previous version of the PR suffered from other problems as well. 

The PR is now reverted to a scheme with a lock held on library name, thus such a situation is no longer possible.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3976