RFR: 8268139: CDS ArchiveBuilder may reference unloaded classes [v2]

[Calvin Cheung]

On Thu, 3 Jun 2021 00:07:54 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> During CDS dump, we enter a safepoint (e.g., `VM_PopulateDynamicDumpSharedSpace`) and run `ArchiveBuilder::gather_source_objs()` to collect all classes to be dumped. However, some of these classes may belong to custom class loaders that are no longer alive. While we are still inside the safepoint, concurrent GC operations may free these class loaders (see https://github.com/openjdk/jdk/pull/4286). Thus `ArchiveBuilder` may end up referencing unloaded classes.
>> 
>> The fix:
>> 
>> (1) Add a class `k` to ArchiveBuilder only if `k->class_loader_data()->is_alive()`
>> 
>> (2) All the classes added to `ArchiveBuilder` are taken from `DumpTimeSharedClassTable`. When a class is loaded, it's added to this table. When a class is unloaded, it's removed from this table (via `SystemDictionaryShared::remove_dumptime_info`). This table is protected by `DumpTimeTable_lock`. To make sure that this table is not changed during the CDS safepoint, we grab this lock before calling `ArchiveBuilder::gather_source_objs()`, and release this lock only when the CDS dump is finished. As a result, in step (1) above, when we call `k->class_loader_data()->is_alive()`, we are sure that `k` itself has not been deallocated and is still a valid pointer.
>> 
>> Testing -- Mach5 tiers 1-4 in progress
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   @coleenp comments

Looks good. In systemDictionaryShared.cpp

 `432   assert(DumpTimeTable_lock->owned_by_self(), "sanity");`

Can the above be changed to `assert_lock_strong(DumpTimeTable_lock);`?

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

Marked as reviewed by ccheung (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4322