RFR (XS) 8066679: jvmtiRedefineClasses.cpp assert cache ptrs must match

[serguei.spitsyn at oracle.com]

Great

Thanks, Dan!
Serguei

On 4/7/15 9:44 AM, Daniel D. Daugherty wrote:
> On 3/25/15 7:16 PM, serguei.spitsyn at oracle.com wrote:
>> Please, review the fix for:
>>   https://bugs.openjdk.java.net/browse/JDK-8066679
>>
>>
>> Open hotspot webrev:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8066679-JVMTI-cache.1/ 
>>
>
> src/share/vm/prims/jvmtiRedefineClasses.cpp
>     I concur that the old assertions are bad. They assume that
>     the cached class file bytes on the scratch_class must match
>     the cached class file bytes on the target class. That's a
>     valid assumption in a RedefineClasses() only world, but that's
>     not true when RetransformClasses() is used.
>
> This will be hard to test. I think the race can only happen
> on the first retransform of a class. If you have two parallel
> retransform threads, both see that there are no cached bytes
> so both cache the initial bytes and then both have to update
> the cached bytes pointer at about the same time... Very hard
> to do...
>
> Thumbs up.
>
> Dan
>
>
>>
>>
>> Summary:
>>
>>   There can be multiple concurrent RetransformClasses calls on 
>> different threads.
>>   So that, the assert "cache ptrs must match" is incorrect as it was 
>> based on wrong assumptions.
>>   The fix removes two related asserts and also frees the 
>> scratch_class cached_class_file_bytes
>>   if it is necessary to avoid leaking the memory.
>>
>>
>> Testing:
>>   In progress: nsk redefine classes tests, JTREG java/lang/instrument
>>
>>
>> Thanks,
>> Serguei
>>
>