RFR: 8265605: Cannot call BootLoader::loadClassOrNull before initPhase2

[Alan Bateman]

On Wed, 12 May 2021 05:51:14 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> This bug was discovered during the development of [JDK-6824466](https://bugs.openjdk.java.net/browse/JDK-6824466): when CDS is enabled, if `BootLoader::loadClassOrNull` is called before `initPhase2`, it would trigger the initialization of the archived module graph in the wrong order. Because of unanticipated nesting of `<clinit>` methods, `BootLoader::SERVICES_CATALOG` becomes empty, causing future `ServiceLoader` operations to fail.
> 
> The fix has 2 parts:
> 
> - `BootLoader::loadClassOrNull` no longer calls `ClassLoaders::bootLoader()`. This avoids triggering the archived module graph initialization. Instead, it makes a direct call to `Classloader::findBootstrapClassOrNull()`. We don't actually need a `ClassLoader` instance for this call, so I changed `Classloader::findBootstrapClassOrNull()` to be a static method.
> - The object returned by `BootLoader::getServicesCatalog()` is now maintained inside `jdk.internal.loader.ClassLoaders`.  Although not strictly required for the fix, this simplifies the initialization of the archived module graph. It also makes the logic consistent for the 3 built-in loaders (boot/platform/app).
> 
> Testing: tiers1-4 in progress. I also verified that the bug reported by Mandy is no longer reproducible after I applied this patch on her branch.

Changes for CDS usually make the code harder to maintain but I think this patch is okay and improves a few areas. Just a few cleanups to keep the code consistent.

src/java.base/share/classes/jdk/internal/loader/BootLoader.java line 75:

> 73:         = NativeLibraries.jniNativeLibraries(null);
> 74: 
> 75:     private static final JavaLangAccess JLA = SharedSecrets.getJavaLangAccess();

It be nicer if you could move this to the top to be consistent with the other files.

src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java line 58:

> 56:     private static final AppClassLoader APP_LOADER;
> 57: 
> 58:     private static void setArchivedServicesCatalog(ArchivedClassLoaders archivedClassLoaders, ClassLoader loader) {

I'd prefer if we could keep the code consistent if possible. In this case, the method declaration can be split over two lines to avoid one really one line in the file. Also can you add a one line // comment to be consistent with the other private methods.

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

Marked as reviewed by alanb (Reviewer).

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