RFR: 8067247: Crash: assert(method_holder->data() == 0 ...) failed: a) MT-unsafe modification of inline cache

[Jamsheed C m]

On 3/28/2016 4:44 PM, Vladimir Ivanov wrote:
>
>> clear_inline_caches clears static stubs. i wanted this api to look more
>> like cleanup_inline_caches functionality(with clean all ics). So used 
>> this.
>
> Are you sure? WhiteBox.clearInlineCaches() should clear all ICs in 
> alive nmethods. Why doesn't it work for you?

in addition it clears this.
void static_stub_Relocation::clear_inline_cache() {
   // Call stub is only used when calling the interpreted code.
   // It does not really need to be cleared, except that we want to 
clean out the methodoop.
   CompiledStaticCall::set_stub_to_clean(this);
}

Best Regards,
Jamsheed
>
> WB_ENTRY(void, WB_ClearInlineCaches(JNIEnv* env, jobject wb))
>   VM_ClearICs clear_ics;
>   VMThread::execute(&clear_ics);
> WB_END
>
> class VM_ClearICs: public VM_Operation {
> ...
>   void doit()         { CodeCache::clear_inline_caches(); }
> ...
> };
>
> void CodeCache::clear_inline_caches() {
>   assert_locked_or_safepoint(CodeCache_lock);
>   NMethodIterator iter;
>   while(iter.next_alive()) {
>     iter.method()->clear_inline_caches();
>   }
> }
>
> void nmethod::clear_inline_caches() {
>   assert(SafepointSynchronize::is_at_safepoint(), "cleaning of IC's 
> only allowed at safepoint");
>   if (is_zombie()) {
>     return;
>   }
>
>   RelocIterator iter(this);
>   while (iter.next()) {
>     iter.reloc()->clear_inline_cache();
>   }
> }
>
> void static_call_Relocation::clear_inline_cache() {
>   // Safe call site info
>   CompiledStaticCall* handler = compiledStaticCall_at(this);
>   handler->set_to_clean();
> }
>
> void opt_virtual_call_Relocation::clear_inline_cache() {
>   // No stubs for ICs
>   // Clean IC
>   ResourceMark rm;
>   CompiledIC* icache = CompiledIC_at(this);
>   icache->set_to_clean();
> }
>
> void virtual_call_Relocation::clear_inline_cache() {
>   // No stubs for ICs
>   // Clean IC
>   ResourceMark rm;
>   CompiledIC* icache = CompiledIC_at(this);
>   icache->set_to_clean();
> }
>
> Best regards,
> Vladimir Ivanov
>
>>
>> Best Regards,
>> Jamsheed
>>
>> On 3/26/2016 1:50 PM, Dean Long wrote:
>>> Instead of changing cleanup_inline_caches() to take a new flag, can
>>> you use the existing
>>> clear_inline_caches()?
>>>
>>> dl
>>>
>>> On 3/25/2016 9:54 AM, Jamsheed C m wrote:
>>>> Thank you Chris.
>>>> I have updated the code.
>>>>
>>>> + if (method == NULL) {
>>>> + return;
>>>> + }
>>>> + nmethod* nm = method->code();
>>>> + if (nm == NULL || nm->is_unloaded()) {
>>>> + return;
>>>> + }
>>>> + nm->cleanup_inline_caches(true);
>>>> Best Regards,
>>>> Jamsheed
>>>>
>>>> On 3/25/2016 6:58 AM, Christian Thalinger wrote:
>>>>>
>>>>>> On Mar 24, 2016, at 11:05 AM, Jamsheed C m
>>>>>> <jamsheed.c.m at oracle.com> wrote:
>>>>>>
>>>>>> Hi All,
>>>>>>
>>>>>> Request for review,
>>>>>>
>>>>>> bug url: https://bugs.openjdk.java.net/browse/JDK-8067247
>>>>>>
>>>>>> webrevs:
>>>>>> fix:
>>>>>>   jdk part: http://cr.openjdk.java.net/~jcm/8067247/webrev.jdk.00/
>>>>>> <http://cr.openjdk.java.net/%7Ejcm/8067247/webrev.jdk.00/>
>>>>>>
>>>>>> newly added test case
>>>>>> hotspot part:
>>>>>> http://cr.openjdk.java.net/~jcm/8067247/webrev.hs.00/
>>>>>> <http://cr.openjdk.java.net/%7Ejcm/8067247/webrev.hs.00/>
>>>>>> under hs-comp/test
>>>>>> http://cr.openjdk.java.net/~jcm/8067247/webrev.hs_comp.00/
>>>>>> <http://cr.openjdk.java.net/%7Ejcm/8067247/webrev.hs_comp.00/>
>>>>>>
>>>>>> Unit Test: test/compiler/jsr292/misc/gc/MHInvokeTest.java
>>>>>> Testing: JPRT with new test case, with fix, without fix
>>>>>>
>>>>>> Problem Summary:  MH.invoke linksite take assistance of java code
>>>>>> to get an adapter method. Here a new method holder class and a
>>>>>> adapter method are created for a MT and lform instance is cached.
>>>>>> Normally this cached lform get returned for a linksite request of
>>>>>> same MT.  When these cached lform get collected(due to memory
>>>>>> pressure),  a new class and method gets created for same MT(even
>>>>>> though old method holder class and  adapter method are live).
>>>>>> Fix Summary: Kept a strong reference to lform instance in adapter
>>>>>> method holder class of  MT.
>>>>>
>>>>> Wow!  You found the cause for his long-standing issue? Nice.
>>>>> + if (method == NULL) { return; }
>>>>> + nmethod* nm = method->code();
>>>>> + if (nm == NULL) { return; }
>>>>> + if (nm->is_unloaded()) { return; }
>>>>> Please put the return and } on separate lines.
>>>>>
>>>>>>
>>>>>> Best Regards,
>>>>>> Jamsheed
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>