RFR: 8308387: CLD created and unloading list sharing _next node pointer leads to concurrent YC missing CLD roots [v4]

[Coleen Phillimore]

On Wed, 31 May 2023 12:46:18 GMT, Axel Boldt-Christmas <aboldtch at openjdk.org> wrote:

>> [JDK-8307106](https://bugs.openjdk.org/browse/JDK-8307106) introduced the ability to walk the created CLDs list in the CLDG without a lock. This was primarily introduced to allow lockless concurrent CLD roots scanning for young collections in generational ZGC. However because the CLD _next node pointer is shared between the two list this can lead to a concurrent iteration of the created CLDs missing list entries. 
>> 
>> This change introduces a second _unloading_next node pointer which is used for the unloading CLDs list. The set_next is now maintains the invariant that it only ever unlinks is_unloading() CLDs and maintains a consistent view of the tail list for anyone reading the list concurrently.  
>> 
>> Testing: tier1-3 and tier1-7 with Generational ZGC
>
> Axel Boldt-Christmas has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Spelling
>  - Tidy up comment
>  - Include unlink_next() in comment and remove whitespace

Looks good.  Is there a way to add a test?

src/hotspot/share/classfile/classLoaderDataGraph.cpp line 503:

> 501: 
> 502:   for (ClassLoaderData* data = _head; data != nullptr; data = data->next()) {
> 503:     if (data->is_alive()) {

There used to be more code for the case where data->is_alive() so this code had this continue, but you could simplify this by having an 'else' clause and remove the continue. That would clean this up a little.

-------------

Marked as reviewed by coleenp (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/14241#pullrequestreview-1454123113
PR Review Comment: https://git.openjdk.org/jdk/pull/14241#discussion_r1212285905