8076110: VM crash when class is redefined with Instrumentation.redefineClasses

[Coleen Phillimore]

On 6/11/15 7:20 AM, Andreas Eriksson wrote:
> Hi,
>
> Deleting the resolution errors from InstanceKlass::deallocate contents 
> works, but I'm unsure if the deletion code should be placed inside the 
> is_shared() if-block or not.
> I don't think that CDS should affect our decision to delete the cached 
> resolution errors, but I might be missing something.
> Coleen, do you know if it affects anything?

The resolution errors aren't shared in the CDS archive so it's fine to 
delete them outside the is_shared block.  The other things are in the 
archive so have to be excluded.

I'm still trying to figure out why deleting them inside 
ConstantPool::deallocate_contents() deleted them outside a safepoint.  I 
didn't find that code path.   You wouldn't happen to still have an 
hs_err file from that experiment?

Thanks,
Coleen
>
> New webrev:
> http://cr.openjdk.java.net/~aeriksso/8076110/webrev.01/
>
> Regards,
> Andreas
>
> On 2015-06-09 01:49, serguei.spitsyn at oracle.com wrote:
>> Hi Andreas,
>>
>> I agree, it is a good idea to delete the resolution errors from 
>> ConstantPool::deallocate_contents.
>> So, I'm waiting for next webrev.
>>
>> Thanks,
>> Serguei
>>
>> On 6/8/15 9:53 AM, Andreas Eriksson wrote:
>>> Hi Coleen, thanks for looking at this.
>>>
>>> That might work, I'll make a build to test it.
>>>
>>> Just FYI:
>>> My first attempt to fix this was actually to delete the resolution 
>>> errors from ConstantPool::deallocate_contents, but that was 
>>> sometimes called when not at safepoint, which made me do this change 
>>> instead.
>>>
>>> Regards,
>>> Andreas
>>>
>>>
>>> On 2015-06-06 00:07, Coleen Phillimore wrote:
>>>>
>>>> Thank you so much for fixing this, Andreas!
>>>>
>>>> I was wondering if you could delete the resolution errors in 
>>>> InstanceKlass::deallocate_contents instead at the point where you 
>>>> free the constant pool?   This code is at a safepoint at this time, 
>>>> even with the CMS collector.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 6/4/15 5:23 PM, serguei.spitsyn at oracle.com wrote:
>>>>> Added the SVC list as it impacts the Serviceability as well.
>>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>> On 6/4/15 4:56 AM, Andreas Eriksson wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Please review this fix of a crash because of a class redefinition 
>>>>>> race.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8076110
>>>>>> Webrev: http://cr.openjdk.java.net/~aeriksso/8076110/webrev.00/
>>>>>>
>>>>>> The crash happens when the redefine occurs while the redefined 
>>>>>> method is still running.
>>>>>> This is because VM_RedefineClasses::redefine_single_class deletes 
>>>>>> the resolution errors at redefine time, but the running method 
>>>>>> might still need them.
>>>>>>
>>>>>> This is fixed by moving the deletion of the cached resolution 
>>>>>> errors to when the {add,purge}_previous_version functions have 
>>>>>> come to the conclusion that no running methods need the old class 
>>>>>> any more (and therefore no one needs the constant pool and its 
>>>>>> associated resolution errors either).
>>>>>>
>>>>>> A jtreg regression test that reproduces the problem using 
>>>>>> redefineclasses and objectweb.asm is included in the change.
>>>>>>
>>>>>> Thanks,
>>>>>> Andreas
>>>>>
>>>>
>>>
>>
>