On Tue, 11 May 2021 13:28:16 GMT, Alan Bateman <al...@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. > > src/java.base/share/classes/jdk/internal/loader/NativeLibraries.java line 478: > >> 476: >> 477: // thread local native libraries stack >> 478: private static final ThreadLocal<Deque<NativeLibraryImpl>> >> nativeLibraryThreadContext = > > I would prefer not see thread locals introduced here. We've put a lot of > effort into recent releases to eliminate the use of TLs from java.base. I will update the PR to avoid ThreadLocal and have ArrayDeque deallocated once its last element has been popped out. ------------- PR: https://git.openjdk.java.net/jdk/pull/3976
