On Fri, 4 Nov 2022 02:07:25 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> This change adds an option **EnableWaitForParallelLoad** to enable the >> legacy behavior where the VM will manage synchronization for multiple >> threads loading the same class using a non-parallel capable class loader >> that have released the class loader lock. The VM will break the class >> loader lock for parallel threads trying to load the class, and wait for the >> first thread that initiated loading the class to complete. The subsequent >> threads will use the result of the first thread, rather than get a >> LinkageError: duplicate class definition for loading the class without >> synchronization. >> Releasing the class loader lock was a common workaround for class loaders >> that used a non-hierarchical delegation scheme to avoid deadlock, before >> parallel capable class loading was added. >> https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/lang/ClassLoader.html#registerAsParallelCapable() >> >> Tested with tier1-6 and internal applications. > > src/hotspot/share/classfile/placeholders.cpp line 137: > >> 135: assert(action != PlaceholderTable::LOAD_INSTANCE || seen == NULL, >> 136: "Only one LOAD_INSTANCE allowed at a time"); >> 137: > > Unclear why this is removed? If disabling the new flag causes this fail then > shouldn't the new flag form part of the condition? If you want, it seemed unnecessary clutter for the assert. > src/hotspot/share/classfile/systemDictionary.cpp line 597: > >> 595: double_lock_wait(current, lockObject); >> 596: } else { >> 597: return NULL; > > Not clear why we return NULL here rather than just falling through and > retrying? If we don't return we'll actually do a wait and will hang. Worse, we'll be in a tight loop holding both the ClassLoader and SystemDictionary_lock, that the other thread can't get. ------------- PR: https://git.openjdk.org/jdk/pull/10832
