RFR: 8214499: SA should follow 8150689

Jini George jini.george at oracle.com
Fri Nov 30 07:50:14 UTC 2018 

Yes, Yasumasa. You can push the fix. Thanks!

- Jini.

On 11/30/2018 12:36 PM, Yasumasa Suenaga wrote:
> Hi Jini,
> 
> On 2018/11/30 15:15, Jini George wrote:
>>
>> The change looks good to me, Yasumasa. (One minor nit: line 110: 2 
>> spaces instead of 4 to align with the rest of the file).
> 
> Thanks!
> I will fix it.
> 
> 
>> Would the second part of this comment,
>>
>> 126     // Print out all monitors that we have locked, or are trying 
>> to lock,
>> 127     // including re-locking after being notified or timing out in 
>> a wait().
>>
>> continue to be valid now ? (here and in vframe.cpp) ?
> 
> "continue" is also available in vframe.cpp:
>    
> http://hg.openjdk.java.net/jdk/jdk/file/7d3391e9df19/src/hotspot/share/runtime/vframe.cpp#l213 
> 
> 
> This code will be run if the lock is eliminated and it is in compiled 
> frame.
> So I guess it is valid, but I'm not sure.
> 
> IMHO it should be fixed as another issue if it is incorrect.
> 
> Can I push 8214499 fix?
> 
> 
> Yasumasa
> 
> 
>> Thank you,
>> Jini.
>>
>> On 11/30/2018 9:09 AM, Yasumasa Suenaga wrote:
>>> Hi David, Poonam,
>>>
>>> I filed this issue to JBS and uploaded webrev:
>>>
>>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8214499
>>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8214499/webrev.00/
>>>
>>> This change works fine on test/hotspot/jtreg/serviceability/sa jtreg
>>> test and submit repo
>>> (mach5-one-ysuenaga-JDK-8214499-20181130-0216-12402).
>>>
>>> Could you review it?
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>> 2018年11月30日(金) 10:37 David Holmes <david.holmes at oracle.com>:
>>>>
>>>> Yes please file a bug Yasumasa.
>>>>
>>>> This was an oversight in the fixing of 8150689. I have to remember when
>>>> grepping the code that the SA source is no longer under hotspot :(
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 30/11/2018 5:29 am, Poonam Parhar wrote:
>>>>> Hello Yasumasa,
>>>>>
>>>>> It seems to be a good fix to have in SA. Please file a bug and send in
>>>>> your review request.
>>>>>
>>>>> Thanks,
>>>>> Poonam
>>>>>
>>>>> On 11/29/18 6:29 AM, Yasumasa Suenaga wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> Does someone work for adapting SA to the 8150689?
>>>>>> 8150689 fixed not to show incorrect lock information in thread dump.
>>>>>>
>>>>>> jstack code in SA implements which refer to HotSpot implementation.
>>>>>> So it should be fixed as below:
>>>>>>
>>>>>> ----------------------
>>>>>> diff -r 157c1130b46e
>>>>>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/OopUtilities.java 
>>>>>>
>>>>>>
>>>>>> ---
>>>>>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/OopUtilities.java 
>>>>>>
>>>>>> Thu Nov 29 07:40:45 2018 +0800
>>>>>> +++
>>>>>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/OopUtilities.java 
>>>>>>
>>>>>> Thu Nov 29 22:52:34 2018 +0900
>>>>>> @@ -1,5 +1,5 @@
>>>>>>   /*
>>>>>> - * Copyright (c) 2000, 2017, Oracle and/or its affiliates. All 
>>>>>> rights
>>>>>> reserved.
>>>>>> + * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All 
>>>>>> rights
>>>>>> reserved.
>>>>>>    * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>>>    *
>>>>>>    * This code is free software; you can redistribute it and/or 
>>>>>> modify it
>>>>>> @@ -65,14 +65,14 @@
>>>>>>     // possible values of java_lang_Thread::ThreadStatus
>>>>>>     private static int THREAD_STATUS_NEW;
>>>>>>
>>>>>> -  private static int THREAD_STATUS_RUNNABLE;
>>>>>> -  private static int THREAD_STATUS_SLEEPING;
>>>>>> -  private static int THREAD_STATUS_IN_OBJECT_WAIT;
>>>>>> -  private static int THREAD_STATUS_IN_OBJECT_WAIT_TIMED;
>>>>>> -  private static int THREAD_STATUS_PARKED;
>>>>>> -  private static int THREAD_STATUS_PARKED_TIMED;
>>>>>> -  private static int THREAD_STATUS_BLOCKED_ON_MONITOR_ENTER;
>>>>>> -  private static int THREAD_STATUS_TERMINATED;
>>>>>> +  public static int THREAD_STATUS_RUNNABLE;
>>>>>> +  public static int THREAD_STATUS_SLEEPING;
>>>>>> +  public static int THREAD_STATUS_IN_OBJECT_WAIT;
>>>>>> +  public static int THREAD_STATUS_IN_OBJECT_WAIT_TIMED;
>>>>>> +  public static int THREAD_STATUS_PARKED;
>>>>>> +  public static int THREAD_STATUS_PARKED_TIMED;
>>>>>> +  public static int THREAD_STATUS_BLOCKED_ON_MONITOR_ENTER;
>>>>>> +  public static int THREAD_STATUS_TERMINATED;
>>>>>>
>>>>>>     // java.util.concurrent.locks.AbstractOwnableSynchronizer fields
>>>>>>     private static OopField absOwnSyncOwnerThreadField;
>>>>>> diff -r 157c1130b46e
>>>>>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java 
>>>>>>
>>>>>>
>>>>>> ---
>>>>>> a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java 
>>>>>>
>>>>>> Thu Nov 29 07:40:45 2018 +0800
>>>>>> +++
>>>>>> b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/runtime/JavaVFrame.java 
>>>>>>
>>>>>> Thu Nov 29 22:52:34 2018 +0900
>>>>>> @@ -1,5 +1,5 @@
>>>>>>   /*
>>>>>> - * Copyright (c) 2000, 2017, Oracle and/or its affiliates. All 
>>>>>> rights
>>>>>> reserved.
>>>>>> + * Copyright (c) 2000, 2018, Oracle and/or its affiliates. All 
>>>>>> rights
>>>>>> reserved.
>>>>>>    * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>>>>>>    *
>>>>>>    * This code is free software; you can redistribute it and/or 
>>>>>> modify it
>>>>>> @@ -106,6 +106,9 @@
>>>>>>             StackValue sv = locs.get(0);
>>>>>>             if (sv.getType() == BasicType.getTObject()) {
>>>>>>               OopHandle o = sv.getObject();
>>>>>> +            if
>>>>>> (OopUtilities.threadOopGetThreadStatus(thread.getThreadObj()) ==
>>>>>> OopUtilities.THREAD_STATUS_BLOCKED_ON_MONITOR_ENTER) {
>>>>>> +                waitState = "waiting to re-lock in wait()";
>>>>>> +            }
>>>>>>               printLockedObjectClassName(tty, o, waitState);
>>>>>>             }
>>>>>>           } else {
>>>>>> @@ -146,13 +149,6 @@
>>>>>>               // an inflated monitor that is first on the monitor 
>>>>>> list in
>>>>>>               // the first frame can block us on a monitor enter.
>>>>>>               lockState = identifyLockState(monitor, "waiting to 
>>>>>> lock");
>>>>>> -          } else if (frameCount != 0) {
>>>>>> -            // This is not the first frame so we either own this 
>>>>>> monitor
>>>>>> -            // or we owned the monitor before and called wait(). 
>>>>>> Because
>>>>>> -            // wait() could have been called on any monitor in a 
>>>>>> lower
>>>>>> -            // numbered frame on the stack, we have to check all the
>>>>>> -            // monitors on the list for this frame.
>>>>>> -            lockState = identifyLockState(monitor, "waiting to
>>>>>> re-lock in wait()");
>>>>>>             }
>>>>>>             printLockedObjectClassName(tty, monitor.owner(), 
>>>>>> lockState);
>>>>>>             foundFirstMonitor = true;
>>>>>> ----------------------
>>>>>>
>>>>>>
>>>>>> Please tell me if I should file it to JBS and send review request.
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>

More information about the serviceability-dev mailing list