On Wed, 19 May 2021 18:47:52 GMT, Jorn Vernee <jver...@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
