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

[Vladimir Ivanov]

>> 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);
>
> i want assert to catch this issue. if static stubs are cleared, assert
> wouldn't fail.
I see. Then I suggest to rename the method to 
WhiteBox.cleanupInlineCaches() and iterate over the whole code cache 
(don't specify Method*).

void CodeCache::cleanup_inline_caches() {
   assert_locked_or_safepoint(CodeCache_lock);
   NMethodIterator iter;
   while(iter.next_alive()) {
     iter.method()->cleanup_inline_caches(true);
   }
}

Best regards,
Vladimir Ivanov


> -Jamsheed
>> }
>>
>> 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
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>