On Fri, 11 Jun 2021 10:03:44 GMT, Aleksei Voitylov <[email protected]>
wrote:
>> Please review this PR which fixes the deadlock in ClassLoader between the
>> two lock objects - a lock object associated with the class being loaded, and
>> the ClassLoader.loadedLibraryNames hash map, locked during the native
>> library load operation.
>>
>> Problem being fixed:
>>
>> The initial reproducer demonstrated a deadlock between the JarFile/ZipFile
>> and the hash map. That deadlock exists even when the ZipFile/JarFile lock is
>> removed because there's another lock object in the class loader, associated
>> with the name of the class being loaded. Such objects are stored in
>> ClassLoader.parallelLockMap. The deadlock occurs when JNI_OnLoad() loads
>> exactly the same class, whose signature is being verified in another thread.
>>
>> Proposed fix:
>>
>> The proposed patch suggests to get rid of locking loadedLibraryNames hash
>> map and synchronize on each entry name, as it's done with class names in see
>> ClassLoader.getClassLoadingLock(name) method.
>>
>> The patch introduces nativeLibraryLockMap which holds the lock objects for
>> each library name, and the getNativeLibraryLock() private method is used to
>> lazily initialize the corresponding lock object. nativeLibraryContext was
>> changed to ThreadLocal, so that in any concurrent thread it would have a
>> NativeLibrary object on top of the stack, that's being currently
>> loaded/unloaded in that thread. nativeLibraryLockMap accumulates the names
>> of all native libraries loaded - in line with class loading code, it is not
>> explicitly cleared.
>>
>> Testing: jtreg and jck testing with no regressions. A new regression test
>> was developed.
>
> Aleksei Voitylov has updated the pull request incrementally with one
> additional commit since the last revision:
>
> address review comments
Thanks for making the change. I have a few minor comments and looking good.
I think the synopsis of the JBS issue should be updated to reflect this
problem. What about "deadlock between System::loadLibrary and JNI FindClass
loading another class"?
test/jdk/java/lang/ClassLoader/loadLibraryDeadlock/LoadLibraryDeadlock.java
line 44:
> 42: try {
> 43: // an instance of unsigned class that loads a native
> library
> 44: Class c1 = Class.forName("Class1");
nit: `s/Class/Class<?>/ ` avoid raw type (same in line 58)
test/jdk/java/lang/ClassLoader/loadLibraryUnload/LoadLibraryUnload.java line 67:
> 65: public Class<?> loadClass(String name) throws
> ClassNotFoundException {
> 66: synchronized (getClassLoadingLock(name)) {
> 67: Class clazz = findLoadedClass(name);
nit: `s/Class/Class<?>/`
test/jdk/java/lang/ClassLoader/loadLibraryUnload/p/Class1.java line 40:
> 38: System.loadLibrary("loadLibraryUnload");
> 39: System.out.println("Native library loaded from Class1.");
> 40: } catch (Exception ignore) {
should this exception just be thrown?
-------------
PR: https://git.openjdk.java.net/jdk/pull/3976
