On Fri, 11 Jun 2021 10:03:44 GMT, Aleksei Voitylov <avoity...@openjdk.org> 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