[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Request for review (XS): SA should cope with partially loaded ConstantPool
From:       rednaxelafx () gmail ! com (Krystal Mok)
Date:       2011-12-30 12:36:30
Message-ID: CA+cQ+tRSf-iDrmgK7QDTz4aKrksK-_Yu0ypFEyKkYwO9V0-+bQ () mail ! gmail ! com
[Download RAW message or body]

Hi all,

I was using CLHSDB to dump the contents of PermGen the other day, and ran
into a ClassCastException, as shown in [1].

It turns out that there was a partially loaded constantPoolOopDesc instance
in the PermGen, which is actually dead already, but not collected yet
(because no GC has happened yet). The way it's marked to be "partially
loaded" is setting a pointer to this constantPoolOopDesc object itself to
its _pool_holder field, which caused the exception in the Serviceability
Agent.

There's no problem with VM, but I think SA should cope with this behavior.
So here's a patch to fix SA:

diff -r fe2c87649981
agent/src/share/classes/sun/jvm/hotspot/oops/ConstantPool.java
--- a/agent/src/share/classes/sun/jvm/hotspot/oops/ConstantPool.java    Thu
Dec 29 15:14:33 2011 -0800
+++ b/agent/src/share/classes/sun/jvm/hotspot/oops/ConstantPool.java    Fri
Dec 30 20:15:10 2011 +0800
@@ -648,7 +648,12 @@
   }

   public void printValueOn(PrintStream tty) {
-    tty.print("ConstantPool for " + getPoolHolder().getName().asString());
+    Oop holder = poolHolder.getValue(this);
+    if (holder instanceof Klass) {
+      tty.print("ConstantPool for " + ((Klass)
holder).getName().asString());
+    } else {
+      tty.print("ConstantPool for partially loaded class");
+    }
   }

   public long getObjectSize() {


By the way, there's another bug in current tip version of SA.
In 6990754 [2], Symbols were moved into native memory, and SA was updated
accordingly. But it missed a case in ConstantPool.iterateFields(OopVisitor
visitor, boolean doVMFields).

A quick-n-dirty fix would be:

diff -r fe2c87649981
agent/src/share/classes/sun/jvm/hotspot/oops/ConstantPool.java
--- a/agent/src/share/classes/sun/jvm/hotspot/oops/ConstantPool.java    Thu
Dec 29 15:14:33 2011 -0800
+++ b/agent/src/share/classes/sun/jvm/hotspot/oops/ConstantPool.java    Fri
Dec 30 20:15:10 2011 +0800
@@ -454,7 +454,7 @@
         case JVM_CONSTANT_Class:
         case JVM_CONSTANT_UnresolvedString:
         case JVM_CONSTANT_Utf8:
-          visitor.doOop(new OopField(new
NamedFieldIdentifier(nameForTag(ctag)), indexOffset(index), true), true);
+          visitor.doInt(new IntField(new
NamedFieldIdentifier(nameForTag(ctag)), indexOffset(index), true), true);
           break;

         case JVM_CONSTANT_Fieldref:

But fixing it like this would make it hard to see the connection between a
ConstantPool and the Symbols it's referencing.
I'm not so sure about what the best fix would look like. Tried adding a
"SymbolField" type, but it felt too heavy. Any suggestions?

Regards,
Kris Mok

[1]: https://gist.github.com/1526668#file_clhsdb_session
[2]: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6990754
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20111230/56e45c09/attachment.html \



[prev in list] [next in list] [prev in thread] [next in thread]