RFR: 8214499: SA should follow 8150689
Yasumasa Suenaga
yasuenag at gmail.com
Fri Nov 30 07:06:24 UTC 2018
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