RFR(S): 8251860: ClassLoaderData::loaded_classes_do fails with "assert(ZAddress::is_marked(addr)) failed: Should be marked"

[Coleen Phillimore]

Hi Calvin,

I think you should change this to call 
ClassLoaderDataGraph::loaded_cld_do(&collect_cld) and collect 
ClassLoaderData in the GrowableArray, since you then walk through them 
again to link the classes in the class loader data.

Can linking one klass cause a new class to be added to a _klasses list 
of a CLD that you've already walked?  It seems like you need the do() 
loop in the current code wrapped around walking the clds, then link the 
classes until no progress.  You can fast exit when you find a klass 
linked, I believe, since the classes are added to the front of the 
_klasses list.

Also with this change, you can remove locked_loaded_classes_do() because 
it was added for CDS and apparently the comment is wrong.

// This case can block but cannot do unloading (called from CDS)
void ClassLoaderDataGraph::unlocked_loaded_classes_do(KlassClosure* 
klass_closure) {
   for (ClassLoaderData* cld = _head; cld != NULL; cld = cld->next()) {
     cld->loaded_classes_do(klass_closure);
   }
}

Thanks,
Coleen

On 8/26/20 5:35 PM, calvin.cheung at oracle.com wrote:
> bug: https://bugs.openjdk.java.net/browse/JDK-8251860
>
> webrev: http://cr.openjdk.java.net/~ccheung/jdk16/8251860/webrev.00/
>
> Please refer to the bug report for an analysis of the crash.
>
> The proposed fix is to acquire the ClassLoaderDataGraph_lock before 
> calling ClassLoaderDataGraph::loaded_classes_do(&link_closure). The 
> link_closure will not do the actual linking but just to store the 
> InstanceKlass'es in an array and increment the keep_alive counter of 
> the class_loader_data of each InstanceKlass. After the call to 
> ClassLoaderDataGraph::loaded_classes_do, each InstanceKlass in the 
> array will be linked and the keep_alive counter of the 
> class_loader_data will be decremented.
>
> Testing:
>     - tested about 40 times on the MacOS host where the crash was seen
>        (before the fix, the crash was seen about once in 20 runs)
>     - Tier1 - 4.
> thanks,
> Calvin