On Thu, 6 Jul 2023 05:18:01 GMT, Jean-Philippe Bempel <jpbem...@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

Reply via email to