CDS can archive classpath entries more than once when a JAR manifest has Class-Path attributes

[Steven Schlansker]

Hi hotspot-dev,

Recently, we started experiencing JVM crashes [1] and inexplicable
IncompatibleClassChangeErrors in our testing environment. We use
custom classloaders, NMT, and app-CDS.

#  Internal Error (virtualMemoryTracker.cpp:403), pid=20, tid=128
#  Error: ShouldNotReachHere()
#

# JRE version: OpenJDK Runtime Environment (Red_Hat-21.0.1.0.12-2)
(21.0.1+12) (build 21.0.1+12-LTS)
# Java VM: OpenJDK 64-Bit Server VM (Red_Hat-21.0.1.0.12-2)
(21.0.1+12-LTS, mixed mode, sharing, tiered, compressed oops,
compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x104a06c]
VirtualMemoryTracker::add_reserved_region(unsigned char*, unsigned
long, NativeCallStack const&, MEMFLAGS)+0x6fc

and,

java.lang.IncompatibleClassChangeError:
com.paywholesail.components.util.ByteBuffers and
com.paywholesail.components.util.ByteBuffers$ByteBufferPuttable
disagree on InnerClasses attribute
(I checked with javap, and it looks the same to me...)

At least for the ShouldNotReachHere, it looked like a definite JVM
bug, so I have been trying to create a reproducing test case to make a
good error report. I noticed that the crash only happens when NMT is
combined with Class Data Sharing. At this point, I read the logs
closely, and noticed:

[0.139s][warning][cds       ] shared class paths mismatch
[0.151s][warning][cds,dynamic] Unable to use shared archive. The top
archive failed to load: /.../prebake.jsa

So, I compared the expected and actual class path as printed by the
JVM. In both cases, we run with `-cp lib/*` with a fixed set of
library jars. Imagine my surprise when I find that the only difference
is that the expected (archive-time) classpath includes
lib/stax-ex-1.8.jar *twice*.

By running the generated shared archive file with `strings | grep`, I
am able to verify that the `lib/stax-ex-1.8.jar` entry indeed is
present in the archive twice.

I fixed up my JDK build environment and started sprinkling new logging
and assertions through the archive creation code.

It looks like ClassLoader::add_to_app_classpath_entries can either
check for duplicated classpath entries, or trust that the caller knows
the element is new.
This list of entries is built in part by
ClassLoader::setup_app_search_path, which enumerates the classpath and
adds entries one by one. In this case, duplicate checks are skipped,
presumably because we trust the initial classpath not to have
duplicates.

When an element is added in add_to_app_classpath_entries, for each
jar, it calls process_jar_manifest. Among other things, this reads the
MANIFEST.MF and looks for Class-Path entries, and loads those too.
Indeed, our `jaxb-runtime` has such an entry for `stax-ex`. In this
case, it does guard against duplicate entries.

I think there is a bug here: if a jar is added by a manifest's
Class-Path from a jar *before* we finish processing the initial app
class path, it can get added twice - first with a duplicate check via
the manifest, and then a second time without checking for duplicates
from the app classpath.

I believe this is reproducible on latest 21.0.1+12 with the following
code and steps:

A.java:
class A {
    static {
        System.err.println("A");
    }
}

class B {
    public static void main(String[] args) {
        System.err.println("hi!");
        new A();
    }
}

MANIFEST.MF:
Manifest-Version: 1.0
Class-Path: B.jar


% mkdir lib
% javac A.java
% javac B.java
% jar -m META-INF/MANIFEST.MF -c -f lib/A.jar A.class
% jar cf lib/B.jar B.class

% java -cp lib/B.jar:lib/A.jar -XX:ArchiveClassesAtExit=shared.jsa
-XX:NativeMemoryTracking=summary B
% strings shared.jsa| grep lib/
lib/B.jar
lib/A.jar

% java -cp lib/A.jar:lib/B.jar -XX:ArchiveClassesAtExit=shared2.jsa
-XX:NativeMemoryTracking=summary B
% strings shared2.jsa| grep lib/
lib/A.jar
lib/B.jar
lib/B.jar

When A.jar is loaded first, the Class-Path manifest entry adds B.jar.
Then, B.jar is added *again*, unconditionally.
When B.jar is loaded first, the app classpath entry is created first.
Then, the manifest entry is checked and since it is a duplicate, only
one entry is added.

At this point I felt like I collected enough information to ask for
some expert advice.
Am I on the right track here, that this could be a bug resulting in
duplicate classpath entries in the archive classpath, if a dependent
jar comes in via a manifest class-path entry before the app classpath
finishes processing? Could that possibly be the source of our
assertion failures and IncompatibleClassChangeErrors?

As a related question, this makes me worry that using `-cp lib/*`
might implicitly embed the filesystem enumeration order in the
archive. Maybe the classpath order is not important when verifying,
but at the very least, the wildcard enumeration order influences the
build in a way I did not expect.

If my analysis sounds plausible, I can submit it via the Java bug system.

Thank you for any consideration and advice. Best,
Steven

[1] https://gist.github.com/stevenschlansker/12d1eaeb363ae135c88a965048353b0e