回复: [8u] RFR: Fix JFR crash in WriteObjectSampleStacktrace with object has uninitialized klass(Internet mail)

[kalinshi(施慧)]

Thanks for your comments and proposed fix!

Current fix is trying to make minimal impact on 8u. 
If common_mem_allocate_noinit is success and object is also possible allocated from TLAB fast path, no need record allocation event in current code.

"tlab" variable should be a int value records three different allocation paths. 1) fast path tlab allocation 2) slow path tlab allocation 3) not tlab allocation

Regards
Hui
-----邮件原件-----
发件人: Liu, Xin <xxinliu at amazon.com> 
发送时间: 2020年11月3日 11:12
收件人: kalinshi(施慧) <kalinshi at tencent.com>; jdk8u-dev at openjdk.java.net
主题: Re: [8u] RFR: Fix JFR crash in WriteObjectSampleStacktrace with object has uninitialized klass(Internet mail)

Hello, Hui,

Thank you for taking time to fix it.  The following is my personal option. We still need reviewer to make decision. 

I think the safest approach is to postpone those two events after proper obj initialization.  That aligns up the behavior of newer jdks and avoids redundant stores of klasses, doesn't it? 
The key is a variable to record if the allocation happens in or out of tlab.
https://github.com/openjdk/jdk/blob/master/src/hotspot/share/gc/shared/memAllocator.cpp#L49 

It doesn't necessarily mean you have to backport the class MemAllocator::Allocation to jdk8u.  One idea is to extend common_mem_allocate_init  with another output argument. 
HeapWord* CollectedHeap::common_mem_allocate_init(KlassHandle klass, size_t size, TRAPS) {
  bool tlab = true;
  HeapWord* obj = common_mem_allocate_noinit(klass, size, tlab, CHECK_NULL);
  init_obj(obj, size);
  if (obj != NULL) {
     if (tlab) send_allocation_in_new_tlab_event ()
     else  send_allocation_outside_tlab_event()
  }
  return obj;
}

thanks,
--lx

On 11/1/20, 10:38 PM, "jdk8u-dev on behalf of kalinshi(施慧)" <jdk8u-dev-retn at openjdk.java.net on behalf of kalinshi at tencent.com> wrote:

    CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



    Hi All,

    Please help review crash fix in 8u JFR.

    JFR ObjectSample's _object field might have uninitialized klass when it is iterated in WriteObjectSampleStacktrace. Because ObjectSample is created before post_allocation_setup.
    This problem doesn't exist in 11 or master, as ObjectSample is created in MemAllocator::Allocation destruction method (notify_allocation_jfr_sampler), its klass is initialized.

    Detail reproduce and analysis in JBS. Before this patch, debug version crash 17 time sin 12 hours, with this patch, no crash reproduced.

    JBS: https://bugs.openjdk.java.net/browse/JDK-8255717
    Webrev http://cr.openjdk.java.net/~hshi/8255717/webrev/

    Regards
    Hui