RFR: 8308762: Metaspace leak with Instrumentation.retransform

[David Holmes]

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.

Sorry I'm having a lot of trouble trying to understand the fix in the context of the problem description as outlined in the bug report. The issue pertained only to the treatment of Throwable due to it being pre-resolved by the verifier, but your fix is looking at Field and MethodRefs ??

There are also remaining comments about resolved and unresolved class entries deliberately not being considered the same.

src/hotspot/share/oops/constantPool.cpp line 1281:

> 1279: // Returns true if the current mismatch is due to a resolved/unresolved
> 1280: // class pair. Otherwise, returns false.
> 1281: bool ConstantPool::is_unresolved_class_mismatch(int index1,

Has this been moved verbatim from jvmtiRedefineClasses.cpp?

There are a couple of style nits with that existing code that don't fit this file:
- parameters should line up ie. const under int
- no comment on the closing brace of the method body.

src/hotspot/share/oops/constantPool.cpp line 1298:

> 1296:   }
> 1297: 
> 1298:   char *s1 = klass_name_at(index1)->as_C_string();

`as_C_string` needs an active `ResourceMark` - not sure where it is coming from even in the existing code. There should at least be a comment that an active ResourceMark is needed.

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

PR Review: https://git.openjdk.org/jdk/pull/14780#pullrequestreview-1515832397
PR Review Comment: https://git.openjdk.org/jdk/pull/14780#discussion_r1253992199
PR Review Comment: https://git.openjdk.org/jdk/pull/14780#discussion_r1253988705