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

[kalinshi(施慧)]

Please help review updated fix again!

Move JFR send allocation event code. From before object setup to post_allocation_notify.

send_allocation_in_new_tlab_event is invoked if object is allocated at current thread's TLAB start.
send_allocation_outside_tlab_event is invoked if object is not in current thread's TLAB.

Not sure why AllocTracer::send_allocation_in_new_tlab_event and AllocTracer::send_allocation_outside_tlab_event not guarded with JFR_ONLY macro. Leave it same with current code.

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

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