RFR: 8268139: CDS ArchiveBuilder may reference unloaded classes

[Coleen Phillimore]

On Wed, 2 Jun 2021 21:31:55 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

Except a minor comment, this looks good.

src/hotspot/share/classfile/systemDictionaryShared.cpp line 1514:

> 1512:   return k->class_loader_data()->is_alive();
> 1513: }
> 1514: 

There's a k->is_loader_alive() call, so you don't need this.

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

Marked as reviewed by coleenp (Reviewer).

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