8216541: CompiledICHolders of VM locked unloaded nmethods are released too late

[Erik Österlund]

Hi Kozlov,

Thanks for the review.

/Erik

On 2019-01-29 23:31, Vladimir Kozlov wrote:
> Looks good to me too.
> 
> Thanks,
> Vladimir
> 
> On 1/29/19 2:40 AM, Tobias Hartmann wrote:
>> Hi Erik,
>>
>> okay, got it. Thanks for the details, your fix looks good to me!
>>
>> Best regards,
>> Tobias
>>
>> On 29.01.19 11:38, Erik Österlund wrote:
>>> Hi Tobias,
>>>
>>> Thanks for having a look at this.
>>>
>>> On 2019-01-29 09:16, Tobias Hartmann wrote:
>>>> Hi Erik,
>>>>
>>>> very nice analysis, thanks a lot for investigating!
>>>>
>>>> On 28.01.19 14:56, Erik Österlund wrote:
>>>>> http://cr.openjdk.java.net/~eosterlund/8216541/webrev.00/
>>>>
>>>> Why did you remove the call to 
>>>> thread->set_scanned_compiled_method(NULL) in sweeper.cpp?
>>>
>>> Because the CompiledMethodMarker destructor already nulls this out, 
>>> and redundantly nulling it out
>>> again offers no extra protection.
>>>
>>> The idea of nulling it out before calling flush seems to have been to 
>>> prevent the GC scanning from
>>> seeing this flushed nmethod in a safepoint, accidentally resurrecting 
>>> it from the dead. But that is
>>> already impossible, because flush() is called with a never safepoint 
>>> checking lock (which guarantees
>>> we don't have any and can't add any safepoint checks while holding 
>>> that lock or we will deadlock
>>> badly). Therefore such safepoints will happen strictly after the 
>>> processing of the compiled method
>>> is finished, and it is already cleared the normal way.
>>>
>>> By removing that pointless clearing, I could get rid of the 
>>> release_compiled_method() function and
>>> just call flush directly instead. I get confused by there being two 
>>> "destroy" functions, one in the
>>> sweeper and one in the nmethod, so I wanted it gone.
>>>
>>>>
>>>>> The proposed change has survived 200 rounds of kitchensink, 
>>>>> hs-tier1-3 and hs-precheckin-comp.
>>>>
>>>> In the meanwhile, could you please run some more 100x iterations of 
>>>> kitchensink?
>>>
>>> Sure, running some more as we speak.
>>>
>>> Thanks,
>>> /Erik
>>>
>>>>
>>>> Thanks,
>>>> Tobias
>>>>