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

Reply via email to