RFR (S): 8197844: JVMTI GetLoadedClasses should use the Access API

[coleen.phillimore at oracle.com]

Hi Erik,

I wanted to fix this differently and I don't like that Klass is a friend 
of ClassLoaderData especially.

The patch in jvmtiGetLoadedClasses.cpp was a minimal patch which is easy 
for backporting, but in discussions with Kim and Stefan, I think the 
problem is larger.  When we iterate through ClassLoaderDataGraph we 
don't want any of the loaders to get unlinked if there's a safepoint in 
the function or closure that we call, or else the list will get broken.  
I don't think we do that now but we should guard against it.  I think 
the CLDG iterator should have the barrier on the holder, which is good 
because then it doesn't have to be friends with itself.  We have a bunch 
of ad-hoc iteration of the graph which should be unified into an iterator.

I'd like to work on a different patch if that is ok with you.

Thanks,
Coleen

On 2/15/18 5:04 AM, Erik Österlund wrote:
> Hi,
>
> The JVMTI GetLoadedClasses function retrieves, as the name suggests, 
> all loaded classes. However, to be able to safely return their mirrors 
> in handles during potential concurrent marking, G1 requires an SATB 
> enqueue barrier to shade these mirrors grey that were not part of the 
> snapshot at the beginning, to prevent the classes from being unloaded.
>
> The current solution is to explicitly SATB enqueue the mirror of the 
> retrieved classes with the usual #ifdef blah if (UseG1GC) jazz.
>
> My proposed solution is to instead use the Access API to make the code 
> more modular. In the Access API, this operation corresponds to loading 
> the mirror with ON_PHANTOM_OOP_REF. Also, an observation from my part 
> is that usually when we keep classes from unloading, we keep the 
> klass_holder alive, rather than the mirror. For all Klasses except 
> anonymous instanceKlasses, the holder is the class loader. For 
> anonymous instanceKlasses, the holder is the mirror. As the rest of 
> the code tends to prevent class unloading by keeping the holder alive, 
> I would prefer to keep the holder alive instead of the mirror, unless 
> there is a good reason to keep the mirror alive instead of the holder.
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8197844
>
> Webrev:
> http://cr.openjdk.java.net/~eosterlund/8197844/webrev.00/
>
> This change has made it through hs-tier1-5 and jdk-tier1-3.
>
> Thanks,
> /Erik