RFR(S) 8156137: SIGSEGV in ReceiverTypeData::clean_weak_klass_links

[dean.long at oracle.com]

Thanks Serguei!

dl


On 8/30/16 5:54 PM, serguei.spitsyn at oracle.com wrote:
> Hi Dean,
>
> It looks good.
> Nice catch!
>
> Thanks,
> Serguei
>
>
> On 8/30/16 14:49, Coleen Phillimore wrote:
>>
>>
>> On 8/30/16 5:40 PM, dean.long at oracle.com wrote:
>>> Thanks for the review.  Do I need another review before pushing?
>>
>> Maybe Dan or Serguei could look at it too.
>>
>> Coleen
>>
>>>
>>> dl
>>>
>>>
>>> On 8/30/16 1:57 PM, Coleen Phillimore wrote:
>>>>
>>>> This change looks good.  Thank you for doing the analysis and 
>>>> figuring this out.
>>>> Coleen
>>>>
>>>> On 8/30/16 3:24 PM, dean.long at oracle.com wrote:
>>>>> http://cr.openjdk.java.net/~dlong/8156137/webrev/
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8156137
>>>>>
>>>>> The problem: JVMTI RedefineClasses creates scratch classes to hold 
>>>>> the old methods until they can be freed (they are no longer active 
>>>>> in a thread stack).  G1ConcurrentMark sees these scratch classes, 
>>>>> but G1MarkSweep does not.
>>>>>
>>>>> The details:
>>>>>
>>>>> G1ConcurrentMark uses ClassLoaderDataGraphKlassIteratorAtomic to 
>>>>> iterate over *all* classes and calls 
>>>>> clean_weak_instanceklass_links to clean.
>>>>>
>>>>> G1MarkSweep (and other GCs) use Klass::clean_weak_klass_links() to 
>>>>> iterate over *live, non-scratch* classes, and calls 
>>>>> clean_weak_instanceklass_links to clean.
>>>>>
>>>>> Now the problem scenario is:
>>>>>
>>>>> 0: scratch class S has MethodData that references a class U that 
>>>>> is going to be unloaded.
>>>>>
>>>>> 1: G1MarkSweep skips class S in Klass::clean_weak_klass_links, 
>>>>> because scratch classes are not added to the class hierarchy tree. 
>>>>> The full GC then frees the metadata for class U. Now the 
>>>>> MethodData for S contains stale metadata.
>>>>>
>>>>> 2. When a later G1ConcurrentMark calls 
>>>>> clean_weak_instanceklass_links on S, it will crash on the stale 
>>>>> metadata.
>>>>>
>>>>> Solution: have Klass::clean_weak_klass_links() process these 
>>>>> scratch classes that can be found on the "previous versions" list.
>>>>>
>>>>> Tested with bigapps/Kitchensink.
>>>>>
>>>>> dl
>>>>>
>>>>
>>>
>>
>