RFR: 8326820: Metadata artificially kept alive

Erik Österlund eosterlund at openjdk.org
Tue Jun 18 13:45:18 UTC 2024


On Tue, 18 Jun 2024 13:33:59 GMT, Erik Österlund <eosterlund at 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


More information about the serviceability-dev mailing list