RFR: JDK-8222545: Safe klass asserts

Roman Kennke rkennke at redhat.com
Wed Apr 17 16:59:09 UTC 2019


>> Various code paths in oopDesc, Klass and their subclasses assert
>> something that fetches the object's _klass field. With upcoming
>> Shenandoah's changes this is not always safe and requires an additional
>> indirection.
>>
>> The trouble here is that we can, for example, call
>> Klass::oop_oop_iterate() with a pre-resolved Klass*, instead of
>> oopDesc::oop_iterate() which would call oopDesc::klass() on its own,
>> which would be racy on some GC internal call paths, but we can't
>> (currently) control some calls to klass() further down the call stack
>> (all in asserts).
>>
>> We'd also like a way to ensure that non-GC calls to klass() are sane.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8222545
>> Webrev:
>> http://cr.openjdk.java.net/~rkennke/JDK-8222545/webrev.00/
>> Testing:
>> hotspot_gc_shenandoah with and without the prototype, hotspot/tier1
>>
>> The change introduces only two ASSERT-level GC-interfaces, and afaict,
>> this with JDK-8222537 will be all that we need for the upcoming
>> elimination of forward pointers in Shenandoah. Notice that one assert in
>> objArrayKlass is strengthened from is_array() to is_objArray(), but that
>> seems only sane in that context.
>>
>> Can I please get reviews?
> 
> This looks very awkward to me. Using:
> 
> Universe::heap()->safe_klass(obj)->is_objArray_klass()
> 
> instead of the obvious:
> 
> obj->is_objArray()
> 
> is very unintuitive. Can this not be handled inside is_objArray (and 
> is_typeArray) ?

Not really. Then it would get exposed to many more code paths, most of 
which don't actually need it/don't want it, and many of which are 
outside of asserts, and rely on the usual klass() with the sanity assert 
there instead. I am open for suggestions, but it would have to be 
restricted to ASSERT code IMO, and ideally with as few as possible GC 
interface additions.

Roman



More information about the hotspot-gc-dev mailing list