RFR(S): 8251860: ClassLoaderData::loaded_classes_do fails with "assert(ZAddress::is_marked(addr)) failed: Should be marked"
calvin.cheung at oracle.com
calvin.cheung at oracle.com
Fri Aug 28 17:42:51 UTC 2020
Hi Coleen,
Thanks for your review.
I've re-implemented the fix with your suggestions.
Here's an updated webrev:
http://cr.openjdk.java.net/~ccheung/jdk16/8251860/webrev.01/
On 8/26/20 3:24 PM, Coleen Phillimore wrote:
>
> 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.
With the ClassLoaderDataGraph::loaded_cld_do approach, some klasses in
the _klasses list may already in the link state. So I have to account
for that and not link those klasses again. It complicates the fix a
little bit.
>
> 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.
Yes, it is possible. The updated patch will check if the first klass in
the _klass list of each CLD hasn't been linked, then it will loop back
again to go through all the CLD's _klasses lists.
>
> 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);
> }
> }
I've removed the above.
thanks,
Calvin
>
> 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
>
More information about the hotspot-runtime-dev
mailing list