RFR: 8308762: Metaspace leak with Instrumentation.retransform

[Coleen Phillimore]

On Thu, 6 Jul 2023 05:18:01 GMT, Jean-Philippe Bempel <jpbempel at openjdk.org> wrote:

> Fix a small leak in constant pool merging during retransformation of a class. If this class has a catch block with `Throwable`, the class `Throwable` is pre-resolved in the constant pool, while all the other classes are in a unresolved state. So the constant pool merging process was considering the entry with pre-resolved class as different compared to the destination and create a new entry. We now try to consider it as equal specially for Methodref/Fieldref.

Hi, I now see why there's a resolved class in the scratch constant pool (ie the constant pool for the new class bytes).  It looks like javac has preresolved it for us, which you said but I didn't know why.  The merged constant pool always has an unresolved class copy of that class entry, because we create the merged constant pool with all unresolved classes.

I now believe that ConstantPool::compare_entry_to should always compare class and resolved class as equivalent.  Since the klass_name_at() function is the same call for both unresolved and resolved class, you could change the tag at the beginning of compare_entry_to() to JVM_CONSTANT_UnresolvedClass if it's resolved, like we do for error classes.  Then remove the is_unresolved_class_mismatch function entirely (the comment at that call might be useful to explain why you're changing the tag in compare_entry_to).  This code is only used for RedefineClasses, but even if it wasn't this comparison is the right thing to do.  For special measure, you could #if INCLUDE_JVMTI around this and the operand functions that call it or file an RFE to do so for further cleanup (compare_entry_to, find_matching_entry, find_matching_operand, compare_operand_to).

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14780#issuecomment-1633051058