Discussion:
SA side fix for 8150689
(too old to reply)
Yasumasa Suenaga
2018-11-29 14:29:26 UTC
Permalink
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
Poonam Parhar
2018-11-29 19:29:28 UTC
Permalink
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
Post by Yasumasa Suenaga
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.
----------------------
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
David Holmes
2018-11-30 01:37:43 UTC
Permalink
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
Post by Poonam Parhar
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
Post by Yasumasa Suenaga
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.
----------------------
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
Yasumasa Suenaga
2018-11-30 03:39:22 UTC
Permalink
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
Post by David Holmes
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
Post by Poonam Parhar
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
Post by Yasumasa Suenaga
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.
----------------------
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
David Holmes
2018-11-30 04:09:23 UTC
Permalink
Hi Yasumasa,
Post by Yasumasa Suenaga
Hi David, Poonam,
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?
As per the main bug you should delete this part of the comment block:

102 // At this level we can't distinguish the two cases to report
103 // "waited on" rather than "waiting on" for the second case.

Otherwise looks fine. No need to see an updated webrev.

Thanks,
David
Post by Yasumasa Suenaga
Thanks,
Yasumasa
Post by David Holmes
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
Post by Poonam Parhar
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
Post by Yasumasa Suenaga
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.
----------------------
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
Yasumasa Suenaga
2018-11-30 04:42:40 UTC
Permalink
Thanks David!

I will remove them.


Yasumasa
Post by David Holmes
Hi Yasumasa,
Post by Yasumasa Suenaga
Hi David, Poonam,
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?
102 // At this level we can't distinguish the two cases to report
103 // "waited on" rather than "waiting on" for the second case.
Otherwise looks fine. No need to see an updated webrev.
Thanks,
David
Post by Yasumasa Suenaga
Thanks,
Yasumasa
Post by David Holmes
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
Post by Poonam Parhar
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
Post by Yasumasa Suenaga
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.
----------------------
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
Jini George
2018-11-30 06:15:22 UTC
Permalink
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.
Post by Yasumasa Suenaga
Hi David, Poonam,
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
Post by David Holmes
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
Post by Poonam Parhar
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
Post by Yasumasa Suenaga
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.
----------------------
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
Yasumasa Suenaga
2018-11-30 07:06:24 UTC
Permalink
Hi Jini,
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.
Post by Yasumasa Suenaga
Hi David, Poonam,
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
Post by David Holmes
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
Post by Poonam Parhar
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
Post by Yasumasa Suenaga
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.
----------------------
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
Jini George
2018-11-30 07:50:14 UTC
Permalink
Yes, Yasumasa. You can push the fix. Thanks!

- Jini.
Post by Yasumasa Suenaga
Hi Jini,
Post by 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).
Thanks!
I will fix it.
Post by Jini George
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) ?
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
Post by Jini George
Thank you,
Jini.
Post by Yasumasa Suenaga
Hi David, Poonam,
   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
Post by David Holmes
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
Post by Poonam Parhar
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
Post by Yasumasa Suenaga
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.
----------------------
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
Loading...