On Tue, 18 Jun 2024 13:33:59 GMT, Erik Österlund <eosterl...@openjdk.org> wrote:
>> ClassLoaderDataGraph provides APIs for walking different metadata. All the >> iterators which are not designed to be used by the GC also keep the holder >> of the CLDs alive and by extensions keeps all metadata alive. This is >> problematic for concurrent GC as it keeps otherwise unreachable classes from >> being unloaded and the respective metadata freed. >> >> This patch changes the default iteration behaviour to not keep the holder >> alive, with the exception of `loaded_classes_do` (renamed >> `loaded_classes_do_keepalive`) and `modules_do` (renamed >> `modules_do_keepalive`) which is used by jvmti APIs that requires that the >> holder is kept alive. >> >> All other uses consumes all the metadata it queries during its safepoint or >> before releasing the `ClassLoaderDataGraph_lock`. >> >> Before this change some jcmd, new jfr chunks and some jfr events, all of >> which consumed these APIs, could cause class unloading to not occur. >> >> Been running our internal stress test in an even more stressful mode which >> without this patch reproduces the metaspace OOME >> [JDK-8326005](https://bugs.openjdk.org/browse/JDK-8326005) consistently >> within a few hours. And after this patch it does not. >> >> Currently running tier1-tier8 testing. > > src/hotspot/share/prims/jvmtiGetLoadedClasses.cpp line 108: > >> 106: // and collect them using the LoadedClassesClosure >> 107: MutexLocker mcld(ClassLoaderDataGraph_lock); >> 108: ClassLoaderDataGraph::loaded_classes_do_keepalive(&closure); > > This one looks like it might not be safe to exposes without keeping the > classes alive. Looks like we fetch the mirror with Klass::java_mirror() and not Klass::java_mirror_no_keepalive(). Its tempting to think that java_mirror will, unlike its evil twin function, keep the mirror alive. However, this maps to _java_mirror.resolve() vs _java_mirror.peek(). The difference between these was only a thing in single generation ZGC as the _java_mirror is an OopHandle with strong references. Single generation ZGC was the only collector that needed to keep oops alive with strong reference loads - no other collector does that. In summary, unless you use single generation ZGC, we don't seem to keep the mirrors alive that we expose from here. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/19769#discussion_r1644490774
