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