RFR: 8214499: SA should follow 8150689

[Jini George]

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).

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) ?

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
>>>