RFR: 8213565: Crash in DependencyContext::remove_dependent_nmethod

[Erik Osterlund]

Hi Vladimir,

Thanks for the review!

/Erik

> On 28 Nov 2018, at 19:03, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
> 
> Looks good to me too.
> 
> Thanks,
> Vladimir
> 
> On 11/28/18 2:48 AM, Robbin Ehn wrote:
>>> Incremental:
>>> http://cr.openjdk.java.net/~eosterlund/8213565/webrev.01_02/
>> Still good!
>>> 
>>> Full:
>>> http://cr.openjdk.java.net/~eosterlund/8213565/webrev.02/
>>> 
>>> Still need one more reviewer for this change.
>> Also bump :)
>> /Robbin
>>> 
>>> /Erik
>>> 
>>>> On 2018-11-23 16:24, Erik Österlund wrote:
>>>> Hi Robbin,
>>>> 
>>>> Thanks for the review.
>>>> 
>>>> /Erik
>>>> 
>>>>> On 2018-11-23 16:26, Robbin Ehn wrote:
>>>>> Looks good, thanks!
>>>>> 
>>>>> /Robbin
>>>>> 
>>>>>> On 2018-11-23 16:16, Erik Österlund wrote:
>>>>>> Hi,
>>>>>> 
>>>>>> Here is an updated version with some minor adjustments based on feedback from Robbin Ehn.
>>>>>> 
>>>>>> Incremental:
>>>>>> http://cr.openjdk.java.net/~eosterlund/8213565/webrev.00_01/
>>>>>> 
>>>>>> Full:
>>>>>> http://cr.openjdk.java.net/~eosterlund/8213565/webrev.01/
>>>>>> 
>>>>>> Thanks,
>>>>>> /Erik
>>>>>> 
>>>>>>> On 2018-11-20 16:10, Erik Österlund wrote:
>>>>>>> Ping.
>>>>>>> 
>>>>>>> /Erik
>>>>>>> 
>>>>>>>> On 2018-11-12 23:02, Erik Österlund wrote:
>>>>>>>> Hi,
>>>>>>>> 
>>>>>>>> In my change 8209189, I moved a lot of code around relating to code cache unloading. Unfortunately, I failed to see that when making nmethods unloaded, and their dependents are flushed... they are not immediately removed. Instead, they are tagged to have stale entries, that are later cleaned during weak metadata cleaning of InstanceKlass. As for method handles, they leak instead, with some hacks to reduce the leaking by expunging entries while adding and removing.
>>>>>>>> 
>>>>>>>> Obviously, with InstanceKlass cleanup and code cache unloading now
>>>>>>>> possibly running in parallel after 8209189, things can go wrong. These two phases used
>>>>>>>> to be separated by a "wait barrier" for the worker threads preventing that parallelism.
>>>>>>>> 
>>>>>>>> The reason dependency contexts were not cleaned when they were found
>>>>>>>> during code cache cleaning, is because it was not thread safe when code
>>>>>>>> cache unloading was made parallel instead of serial. But now that we are
>>>>>>>> implementing concurrent class unloading, we are going to need concurrent
>>>>>>>> cleanup of dependency contexts anyway. Therefore I will propose a bug fix that fixes the problem in a way that works for both serial, parallel and concurrent class unloading.
>>>>>>>> 
>>>>>>>> The solution is to during code cache unloading claim cleaning of
>>>>>>>> encountered stale dependency contexts, and clean them straight away.
>>>>>>>> Entries are unlinked in a lock-free fashion, and placed on a purge list that is walked and deleted during ClassLoaderDataGraph::purge(). This follows the thinking of first unlinking, then syncing all threads, and then purging.
>>>>>>>> 
>>>>>>>> New accessors for the head and next links, hide is_unloading entries and helps unlinking them and putting them into the purge list.
>>>>>>>> 
>>>>>>>> Webrev:
>>>>>>>> http://cr.openjdk.java.net/~eosterlund/8213565/webrev.00/
>>>>>>>> 
>>>>>>>> Bug:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8213565
>>>>>>>> 
>>>>>>>> Thanks,
>>>>>>>> /Erik
>>>>>>> 
>>>>>> 
>>>>