RFR (M) 8155951: VM crash in nsk/jvmti/RedefineClasses/StressRedefine: assert failed: Corrupted constant pool

[Coleen Phillimore]

Thanks, Dan.
Coleen

On 5/24/16 6:46 PM, Daniel D. Daugherty wrote:
> Finally caught up on this thread.
>
> The change looks great. I don't see any locking issues
> with this fix.
>
> Dan
>
>
> On 5/16/16 3:42 PM, Coleen Phillimore wrote:
>>
>> Thank you, David!
>> Coleen
>>
>> On 5/16/16 4:59 PM, David Holmes wrote:
>>> Looks good!
>>>
>>> Thanks,
>>> David
>>>
>>> On 17/05/2016 2:28 AM, Coleen Phillimore wrote:
>>>>
>>>> David, Thank you for looking at this change.
>>>>
>>>> On 5/15/16 9:51 PM, David Holmes wrote:
>>>>> Hi Coleen,
>>>>>
>>>>> On 13/05/2016 4:14 AM, Coleen Phillimore wrote:
>>>>>> 8151066: assert(0 <= i && i < length()) failed: index out of bounds
>>>>>> Summary: lock classes for redefinition because constant pool merging
>>>>>> isn't thread safe, use method constant pool because constant pool
>>>>>> merging doesn't make equivalent cpCaches because of invokedynamic
>>>>>>
>>>>>> I had to make these changes together.   See bugs for more details.
>>>>>>
>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8155951/webrev
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8155951
>>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8151066
>>>>>
>>>>> In general the locking approach seems okay - as long as all mutating
>>>>> actions on the constantpool are being guarded with this locking.
>>>>>
>>>>> But I'm concerned that you are using a bit in _misc_flags for this
>>>>> mutable state. It isn't clear to me when the other bits in 
>>>>> _misc_flags
>>>>> might be set but I'm assuming only during initial class parsing, or
>>>>> some other single-threaded use? Putting a mutable bit into such a 
>>>>> flag
>>>>> set is fragile and can only be done once.
>>>>
>>>> I was uneasy about this too.  Most of the _misc_flags are set during
>>>> initialization but there are a couple that are set while the vm is
>>>> running.   I moved _is_being_redefined to a bool sized gap just about
>>>> the _misc_flags, which makes me more confident of the state of this
>>>> field when doing locking.
>>>>
>>>> Retesting this small change with StressRedefine in a loop.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8155951.02/webrev
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>
>>>>>> Tested with rbt nightly test and 24 hours with StressRedefine test.
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>>>
>>>>>>
>>>>>>
>>>>
>>
>>
>