RFR: 8308762: Metaspace leak with Instrumentation.retransform

[David Holmes]

On Thu, 6 Jul 2023 11:02:48 GMT, Jean-Philippe Bempel <jpbempel at openjdk.org> wrote:

> Sorry I don't understand what you mean, can you elaborate please?

In `ConstantPool::compare_entry_to` it states:

  if (t1 != t2) {
    // Not the same entry type so there is nothing else to check. Note
    // that this style of checking will consider resolved/unresolved
    // class pairs as different.
    // From the ConstantPool* API point of view, this is correct
    // behavior. See VM_RedefineClasses::merge_constant_pools() to see how this
    // plays out in the context of ConstantPool* merging.
    return false;
  }

and in ` VM_RedefineClasses::merge_constant_pools()` it says

 // Pass 0:
    // The old_cp is copied to *merge_cp_p; this means that any code
    // using old_cp does not have to change. This work looks like a
    // perfect fit for ConstantPool*::copy_cp_to(), but we need to
    // handle one special case:
    // - revert JVM_CONSTANT_Class to JVM_CONSTANT_UnresolvedClass
// This will make verification happy.

these comments both pertain to the problem at hand:  the merge reverts the resolved class entry to be unresolved; unresolved entries are not considered matches for resolved ones, hence we grow the constant pool.

Your fix is addressing the second part of the issue by forcing a match, but as per my other comment, it is not clear to me how the placement of that fix addresses the issue originally reported with the catch block, nor is it obvious to me that we should always consider resolved and unresolved class entries to be equal.

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

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