RFR: 8286066: FillerObject_klass should be loaded as early as possible [v5]

Wed May 4 07:55:21 UTC 2022

On Wed, 4 May 2022 07:18:09 GMT, Jie Fu <jiefu at openjdk.org> wrote:

>> Hi all,
>> 
>> `FillerObject_klass` was loaded too late, which may lead to VM crash due to `FillerObject_klass` is unloaded.
>> We found this bug by running `runtime/cds/appcds/TestEpsilonGCWithCDS.java` on x86_32.
>> 
>> From the following gdb backtrace, `FillerObject_klass` may be used during `vmClasses::resolve_all` (see frame #27 resolving `Exception_klass`).
>> So `FillerObject_klass` should be loaded as soon as possible.
>> 
>> 
>> (gdb) bt
>> #0  0xf66c5506 in vmClasses::check_klass (k=0x0) at /home/jdk/src/hotspot/share/classfile/vmClasses.hpp:55
>> #1  0xf68de7a7 in vmClasses::FillerObject_klass () at /home/jdk/src/hotspot/share/classfile/vmClasses.hpp:87
>> #2  0xf695ce9e in CollectedHeap::fill_with_object_impl (start=0xb2e370c8, words=2, zap=true) at /home/jdk/src/hotspot/share/gc/shared/collectedHeap.cpp:470
>> #3  0xf695b40b in CollectedHeap::fill_with_object (start=0xb2e370c8, words=2, zap=true) at /home/jdk/src/hotspot/share/gc/shared/collectedHeap.cpp:479
>> #4  0xf695bf4e in CollectedHeap::fill_with_object (start=0xb2e370c8, end=0xb2e370d0, zap=true) at /home/jdk/src/hotspot/share/gc/shared/collectedHeap.hpp:289
>> #5  0xf695b4fd in CollectedHeap::fill_with_dummy_object (this=0xf602c250, start=0xb2e370c8, end=0xb2e370d0, zap=true)
>>     at /home/jdk/src/hotspot/share/gc/shared/collectedHeap.cpp:503
>> #6  0xf71d5866 in ThreadLocalAllocBuffer::insert_filler (this=0xf6017cdc) at /home/jdk/src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp:122
>> #7  0xf71d5949 in ThreadLocalAllocBuffer::retire (this=0xf6017cdc, stats=0x0) at /home/jdk/src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp:145
>> #8  0xf71d5995 in ThreadLocalAllocBuffer::retire_before_allocation (this=0xf6017cdc) at /home/jdk/src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp:152
>> #9  0xf6ed49c0 in MemAllocator::allocate_inside_tlab_slow (this=0xf62255c0, allocation=...) at /home/jdk/src/hotspot/share/gc/shared/memAllocator.cpp:309
>> #10 0xf6ed48ea in MemAllocator::allocate_inside_tlab (this=0xf62255c0, allocation=...) at /home/jdk/src/hotspot/share/gc/shared/memAllocator.cpp:278
>> #11 0xf6ed4b47 in MemAllocator::mem_allocate (this=0xf62255c0, allocation=...) at /home/jdk/src/hotspot/share/gc/shared/memAllocator.cpp:350
>> #12 0xf6ed4b9e in MemAllocator::allocate (this=0xf62255c0) at /home/jdk/src/hotspot/share/gc/shared/memAllocator.cpp:363
>> #13 0xf6c1b199 in CollectedHeap::class_allocate (this=0xf602c250, klass=0xaa41bca8, size=26, __the_thread__=0xf6017c10)
>>     at /home/jdk/src/hotspot/share/gc/shared/collectedHeap.inline.hpp:46
>> #14 0xf6c1aead in InstanceMirrorKlass::allocate_instance (this=0xaa41bca8, k=0xaa429d70, __the_thread__=0xf6017c10) at /home/jdk/src/hotspot/share/oops/instanceMirrorKlass.cpp:55
>> #15 0xf6c396b5 in java_lang_Class::create_mirror (k=0xaa429d70, class_loader=..., module=..., protection_domain=..., classData=..., __the_thread__=0xf6017c10)
>>     at /home/jdk/src/hotspot/share/classfile/javaClasses.cpp:1004
>> #16 0xf690c541 in ClassFileParser::fill_instance_klass (this=0xf62257d4, ik=0xaa429d70, changed_by_loadhook=false, cl_inst_info=..., __the_thread__=0xf6017c10)
>>     at /home/jdk/src/hotspot/share/classfile/classFileParser.cpp:5428
>> 
>> 
>>     ....
>> 
>> 
>> #27 0xf724e3ea in vmClasses::resolve (id=vmClassID::Exception_klass_knum, __the_thread__=0xf6017c10) at /home/jdk/src/hotspot/share/classfile/vmClasses.cpp:99
>> 
>> 
>> #28 0xf724e4ed in vmClasses::resolve_until (limit_id=vmClassID::SoftReference_klass_knum, start_id=@0xf6225fa8: vmClassID::Cloneable_klass_knum, __the_thread__=0xf6017c10)
>>     at /home/jdk/src/hotspot/share/classfile/vmClasses.cpp:108
>> #29 0xf724ef3f in vmClasses::resolve_through (last_id=vmClassID::Reference_klass_knum, start_id=@0xf6225fa8: vmClassID::Cloneable_klass_knum, __the_thread__=0xf6017c10)
>>     at /home/jdk/src/hotspot/share/classfile/vmClasses.hpp:64
>> #30 0xf724e75e in vmClasses::resolve_all (__the_thread__=0xf6017c10) at /home/jdk/src/hotspot/share/classfile/vmClasses.cpp:168
>> #31 0xf718845a in SystemDictionary::initialize (__the_thread__=0xf6017c10) at /home/jdk/src/hotspot/share/classfile/systemDictionary.cpp:1655
>> #32 0xf71f9814 in Universe::genesis (__the_thread__=0xf6017c10) at /home/jdk/src/hotspot/share/memory/universe.cpp:346
>> #33 0xf71fb851 in universe2_init () at /home/jdk/src/hotspot/share/memory/universe.cpp:966
>> #34 0xf6c07275 in init_globals () at /home/jdk/src/hotspot/share/runtime/init.cpp:132
>> #35 0xf71cc844 in Threads::create_vm (args=0xf622626c, canTryAgain=0xf62261d3) at /home/jdk/src/hotspot/share/runtime/thread.cpp:2756
>> 
>> 
>> The fix just loads `FillerObject` after `Object_klass`.
>> 
>> Thanks.
>> Best regards,
>> Jie
>
> Jie Fu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Don't modify the original jdk image

Changing my approval after thinking it through a bit more to discuss the options we have.

An alternative fix which seems more comprehensive could be to use `java_lang_Object()` for filling these gaps while the other isn't initialized.

E.g. in 

CollectedHeap::fill_with_object_impl(HeapWord* start, size_t words, bool zap)
{
  assert(words <= filler_array_max_size(), "too big for a single object");

  if (words >= filler_array_min_size()) {
    fill_with_array(start, words, zap);
  } else if (words > 0) {
    assert(words == min_fill_size(), "unaligned size");
    ObjAllocator allocator(vmClasses::FillerObject_klass(), words);
    allocator.initialize(start);
  }
}

instead of direct use of `vmClasses::FillerObject_klass()`, add a helper function that selects it based on some predicate; not sure if `Universe::is_initialized()` (or so) isn't too lenient.

I see that `vmClasses` already has some `XXXX_class_loaded()` methods - maybe add one of these for the filler objects and select on that?

Use of the small filler objects is really rare, additionally it only happens at most once per TLAB/PLAB fill, and is mostly done by the application anyway, so I do not see an issue with such an additional flag.

Another option is to have some global variable containing that klass (either in `Universe` or maybe better in `CollectedHeap`) that initially aliases `java_lang_Object` and after this class loading is complete, set it to the filler object klass (that is then used by `CollectedHeap::fill_with_object_impl`.

This would completely avoid the somewhat brittle guessing about the initialization order of the klasses, and avoids any runtime overhead by checking whether the klass has already been loaded during runtime at the cost of a single global variable. At this time I kind of prefer this second option.

-------------

Changes requested by tschatzl (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8519