[9] RFR(XS): 8059735: make_not_entrant_or_zombie sees zombies

[Albert Noll]

Hi Tobias,

this looks good to me as well.

Best,
Albert

On 10/07/2014 04:52 PM, Tobias Hartmann wrote:
> Hi Albert,
>
> On 07.10.2014 15:21, Albert Noll wrote:
>> Hi Tobias,
>>
>> On 10/07/2014 03:08 PM, Tobias Hartmann wrote:
>>> Hi Albert,
>>>
>>> thanks for the review.
>>>
>>> On 07.10.2014 13:16, Albert Noll wrote:
>>>> Hi Tobias,
>>>>
>>>> would it also work to check the state of the nmethod before calling
>>>> make_zombie?
>>>>
>>>> if (nm->is_not_entrant()) {
>>>>    nm->make_zombie()
>>>> }
>>>
>>> Yes, regarding Mikael's comment, that is a better solution.
>>>
>>>> In any case, it would be nice the have a comment that explains why we
>>>> should not enter a
>>>> safpoint / have to do the extra check as described above.
>>>
>>> I changed the fix accordingly and added an additional comment. I also
>>> moved the statistics/debugging code such that it is only executed if
>>> the sweeper sets the nmethod to zombie.
>>>
>>> New webrev: http://cr.openjdk.java.net/~thartmann/8059735/webrev.01/
>> I have one additional question. Do we need to hold the "CompiledIC_lock"
>> when
>> we call make_zombie()?
>
> No, we don't need to hold the lock for 'make_zombie()'. I narrowed the 
> scope of the lock:
>
> http://cr.openjdk.java.net/~thartmann/8059735/webrev.02/
>
> Thanks,
> Tobias
>
>>
>> Best,
>> Albert
>>
>>> Thanks,
>>> Tobias
>>>
>>>>
>>>> Best,
>>>> Albert
>>>>
>>>>
>>>> On 10/07/2014 01:03 PM, Tobias Hartmann wrote:
>>>>> Hi,
>>>>>
>>>>> please review the following patch.
>>>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8059735
>>>>> Webrev: http://cr.openjdk.java.net/~thartmann/8059735/webrev.00/
>>>>>
>>>>> Problem:
>>>>> In 'NMethodSweeper::process_nmethod' a non-entrant nmethod is
>>>>> processed and can be converted to zombie (line 549 of 
>>>>> sweeper.cpp). To
>>>>> clean the IC stubs of that nmethod [1] we acquire the CompiledIC_lock
>>>>> and check for a safepoint. In this case, there is a safepoint request
>>>>> and we block. During the safepoint a VM_Deoptimize operation is
>>>>> executed (due to classloading) and the non-entrant nmethod is set to
>>>>> zombie by 'CodeCache::make_marked_nmethods_zombies'. After the
>>>>> safepoint the sweeper continues and tries to set the nmethod to 
>>>>> zombie
>>>>> again.
>>>>>
>>>>> Solution:
>>>>> Use a MutexLockerEx with the '_no_safepoint_check_flag' to acquire 
>>>>> the
>>>>> CompiledIC_lock.
>>>>>
>>>>> Testing:
>>>>> - Failing tests
>>>>> - JPRT
>>>>>
>>>>> Thanks,
>>>>> Tobias
>>>>>
>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8058737
>>>>
>>