RFR (M) 8201556: Disallow reading oops in ClassLoaderData if unloading
Kim Barrett
kim.barrett at oracle.com
Tue Apr 17 19:50:11 UTC 2018
> On Apr 17, 2018, at 3:37 PM, coleen.phillimore at oracle.com wrote:
>
>
>
> On 4/17/18 2:20 PM, Kim Barrett wrote:
>>> On Apr 17, 2018, at 12:02 PM, coleen.phillimore at oracle.com wrote:
>>>
>>> Summary: Move class_loader oop to an OopHandle, and assert that holder is alive when getting class_loader.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/8201556.01/webrev
>>> bug link https://bugs.openjdk.java.net/browse/JDK-8201556
>>>
>>> Tested with mach5 tiers 1-7.
>>>
>>> Thanks,
>>> Coleen
>> ------------------------------------------------------------------------------
>> src/hotspot/share/classfile/classLoaderData.hpp
>> 86 static ClassLoaderData* add_to_graph(Handle class_laoder, bool anonymous);
>>
>> s/laoder/loader/
>
> Thanks!
>>
>> ------------------------------------------------------------------------------
>> src/hotspot/share/classfile/classLoaderData.hpp
>> 411 Klass* class_loader_klass() const { return _class_loader_klass; }
>> 412 Symbol* class_loader_name() const { return _class_loader_name; }
>>
>> Maybe assert that the member being read has been set, since the
>> initialization happens sometime after construction. Or will that
>> maybe not work because of the annoying null-class-loader?
>
> It's null if the class loader is null, so I can't really assert it.
>>
>> ------------------------------------------------------------------------------
>> src/hotspot/share/classfile/classLoaderData.cpp
>> 942 p2i(this), p2i((void *)_class_loader.ptr_raw()), loader_name());
>>
>> Why is there a (void*) cast on the class loader ptr?
>
> It was there. I've removed the cast and am recompiling it.
>>
>> ------------------------------------------------------------------------------
>> src/hotspot/share/oops/klass.hpp
>> 640 bool is_loader_alive(BoolObjectClosure* is_alive) const { return !class_loader_data()->is_unloading(); }
>>
>> Is this just so you can defer updating is_loader_alive callers to
>> another RFE?
>
> Yes, see (please review!) this RFR:
>
> RFR (M) 8201537: Remove is_alive closure from Klass::is_loader_alive()
>
> This one should be first, and I'll merge these two lines to remove the BoolObjectClosure.
>
> Thanks!
> Coleen
>> ------------------------------------------------------------------------------
Based on those answers, looks good.
More information about the hotspot-dev
mailing list