RFR 8151546: nsk/jvmti/RedefineClasses/StressRedefine fails in hs nightly

[serguei.spitsyn at oracle.com]

Coleen,

src/share/vm/prims/jvmtiRedefineClasses.cpp

- // Update the version number of the constant pool
+ // Update the version number of the constant pools (may keep scratch_cp)
    merge_cp->increment_and_save_version(old_cp->version());
+ scratch_cp->increment_and_save_version(old_cp->version());


Not sure, I understand the change above.
Could you, please, explain why this change is needed?
I suspect, the scratch_cp->version() is never used.


+ // NOTE: this doesn't work because you can redefine the same class in two
+ // threads, each getting their own constant pool data appended to the
+ // original constant pool. In order for the new methods to work when they
+ // become old methods, they need to keep their updated copy of the 
constant pool.
+

It feels like the statement in this note is too strong, and as such, 
confusing.
Would it be better to tell something like "not always work"?
Otherwise, the question is: why do we need this block of code if it 
doesn't work?


Thanks,
Serguei


On 4/11/16 13:06, Coleen Phillimore wrote:
> Summary: Constant pool merging is not thread safe for source_file_name.
>
> This change includes the change for the following bug because they are 
> tested together.
>
> 8148772: VM crash in nsk/jvmti/RedefineClasses/StressRedefine: assert 
> failed: Corrupted constant pool
> Summary: ConstantPool::resolve_constant_at_impl() isn't thread safe 
> for MethodHandleInError and MethodTypeInError.
>
> The parallel constant pool merges are mostly harmless because the old 
> methods constant pool pointers aren't updated.  The only case I found 
> where it isn't harmless is that we rely on finding the 
> source_file_name_index from the final merged constant pool, which 
> could be any of the parallel merged constant pools.  The code to 
> attempt to dig out the name from redefined classes is removed.
>
> open webrev at http://cr.openjdk.java.net/~coleenp/8151546.01/webrev
> bug link https://bugs.openjdk.java.net/browse/JDK-8151546
>
> Tested with rbt, java/lang/instrument tests, com/sun/jdi tests.  I 
> tried to write a test with all the conditions of the failure but 
> couldn't make it fail (so noreg-hard).
>
> Thanks,
> Coleen