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

[Martin Balao]

Hi Lois,

Thanks for your proposal. I'm not a reviewer but it looks good to me too.

Kind regards,
Martin.-

On Mon, Oct 29, 2018 at 4:47 PM, Lois Foltan <lois.foltan at oracle.com> wrote:

> 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>
> 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.w
>>> ebrev.00/
>>>   * http://cr.openjdk.java.net/~mbalao/webrevs/8212937/8212937.w
>>> ebrev.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
>>>
>> 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
>>
>
>
>