RFR 8212937: Parent class loader may not have a referred ClassLoaderData instance when obtained in Klass::class_in_module_of_loader

[Lois Foltan]

On 10/25/2018 11:15 AM, Martin Balao wrote:

> Hi Lois,
>
> Thanks for having a look at this.
>
> Yes, I have a test that triggers this execution path. Unfortunately, 
> this is a security test that I cannot publicly share. What I can tell 
> you, though, is that the parent class loader is a "delegating class 
> loader" which never defined a class and that's the reason why it 
> doesn't have a ClassLoaderData instance created.
>
> I'm not sure if this would be needed in other places where the idiom 
> is not used. The reason is that there might be any other implicit 
> assumptions that makes it impossible. That's why I limited the scope 
> of this changeset to the only case that I'm sure it's possible to trigger.

Hi Martin,

I wrote a test case that reproduces this failure and demonstrates that 
invoking SystemDictionary::register_loader() will not work. With a 
non-product (fastdebug) build, the test case shows that calling 
register_loader is problematic, a SystemDictionary lock is currently 
held and acquiring a ClassLoaderDataGraph lock to create the new 
ClassLoaderData causes an out of order locking fatal error.

Please consider this test case and proposed change:
     http://cr.openjdk.java.net/~lfoltan/bug_jdk8212937/webrev/

Thanks,
Lois

>
> Kind regards,
> Martin.-
>
> On Wed, Oct 24, 2018 at 5:52 PM, Lois Foltan <lois.foltan at oracle.com 
> <mailto:lois.foltan at oracle.com>> wrote:
>
>     On 10/24/2018 3:58 PM, Martin Balao wrote:
>
>         Hi,
>
>         Can I have a review for JDK-8212937 [1]?
>
>         Webrev.00:
>
>           *
>         http://cr.openjdk.java.net/~mbalao/webrevs/8212937/8212937.webrev.00/
>         <http://cr.openjdk.java.net/%7Embalao/webrevs/8212937/8212937.webrev.00/>
>           *
>         http://cr.openjdk.java.net/~mbalao/webrevs/8212937/8212937.webrev.00.zip
>         <http://cr.openjdk.java.net/%7Embalao/webrevs/8212937/8212937.webrev.00.zip>
>
>         By obtaining the ClassLoaderData instance through a call to
>         SystemDictionary::register_loader, we make sure that either an
>         existing
>         instance is obtained or a new one created. This same idiom has
>         already been
>         used in other places where there are no guarantees that a previous
>         ClassLoaderData instance exists for a given class loader. See
>         futher
>         information in JDK-8212937 ticket [1].
>
>         Thanks,
>         Martin.-
>
>         --
>         [1] - https://bugs.openjdk.java.net/browse/JDK-8212937
>         <https://bugs.openjdk.java.net/browse/JDK-8212937>
>
>     Hi Martin,
>
>     I agree that the same idiom is used in other places, but there are
>     also a couple of occurrences of java_lang_ClassLoader::parent in
>     classfile/classLoaderStats.cpp and
>     classfile/classLoaderHierarchyDCmd.cpp that do not.  In order to
>     better understand, do you have a test case that demonstrates this
>     issue?
>
>     Thanks,
>     Lois
>
>