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

Tue Dec 12 19:28:38 UTC 2023

Thank you very much Calvin and Johan.

We have applied a workaround that seems to work for us: construct a
-cp by sorting all jar entries with a manifest class-path to the end.
While this bug is obscure, the symptoms are painful and difficult to
diagnose. I think end-users would appreciate a jdk21 backport if it is
feasible.

Looking forward to future work in 22, 23, and beyond. It's a great
time to be in the Java ecosystem :)

On Tue, Dec 12, 2023 at 2:59 AM Johan Sjolen <johan.sjolen at oracle.com> wrote:
>
> Hi Steven,
>
> The issue you saw in NMT is an indication that something is calling
> NMT's API in an unexpected manner. This is probably a symptom of the CDS
> bug, so when that bug is fixed NMT will no longer crash the JVM. There
> are plans to make NMT more resilient against these sorts of misuses, but
> those changes are planned for JDK-23.
>
> Thank you for the in-depth research on this issue.
>
> All the best,
> Johan
>
> On 2023-12-08 18:56, Steven Schlansker wrote:
> > 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