-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[quarkus-integration-test-jaxb] and others fail with GraalVM master (GraalVM for JDK 22) #37657
Comments
Looking at the stack trace it looks like there is a circular dependency. While initializing
@jerboaa this looks like an OpenJDK issue to me introduced by https://bugs.openjdk.org/browse/JDK-8306055 (openjdk/jdk@93bdc2a) in JDK 22+26. Could you please create an issue in the OpenJDK project to track it? Reproducer without Mandrel, Quarkus etc.:
|
@zakkak Thanks for taking a look. I'll do some more investigation and create an upstream OpenJDK issue once confirmed. |
@zakkak This seems to be an issue with class init order. When I force initialization of
I'm mentioning this, because something like that is hard to reproduce using only supported java xml APIs. So I wonder if graalvm should enforce this init order as well. |
Since this seems a class load ordering issue I'd prefer we fix that on the Quarkus/Mandrel side. |
FWIW, this program also loads the affected
|
Sure, I started looking at how/why this happens in Mandrel/GraalVM, but didn't have the time to complete the analysis. I hope I will be able to get to it tonight. |
Same here. I keep looking as I failed to reliably reproduce yet. Will keep you posted should I find anything. |
FWIW, the case we seem to be running into here is described in this piece of code of
It's the non-determinism of class loading order and inline-before-analysis which seems to make this issue difficult to reproduce reliably. One needs to have the following condition satisfied: InlineBeforeAnalysis encounters bytecode for So far I haven't seen it happening locally for any of the mentioned tests and the same mandrel build as produced from that CI build. |
FWIW I have had the "luck" to reproduce it, not in the debugger though. It looks like the GraalVM team has done some work to deal with JDK-8306055 in the JDK 22+26 version bump ( 🤦 should have seen this earlier), trying to understand whether that's what's causing the issue. Update: When the issue happens the work-around in oracle/graal@2a8653f does not trigger (so it can't be the one causing the issue). Does this mean that there is some other path we need to work-around in a similar manner? Looking... Update 2: Although I have still not managed to reproduce the NPE in the debugger I have managed to get it to freeze in the following state. Thread A (initialization triggered by registration of
Thread B (initialization also triggered by the registration of
Thread A is already in Update 3: I am able to get it to freeze even outside the debugger. The freeze (and probably the NPE as well, I can't reproduce it anymore :/ ) seem to happen due to the registrations in quarkus/extensions/jaxp/deployment/src/main/java/io/quarkus/jaxp/deployment/JaxpProcessor.java Lines 16 to 28 in 7bb333f
and quarkus/extensions/jaxb/deployment/src/main/java/io/quarkus/jaxb/deployment/JaxbProcessor.java Lines 280 to 281 in 4a5006e
which are the ones that trigger the transformation in https://github.com/oracle/graal/blob/a43123982961881e39ebc82321639ba2119f500c/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/xml/Target_jdk_xml_internal_JdkCatalog.java in our case. The freezes seem to happen before the transformation gets in effect hinting that they are independent of it!? |
The best thing I can think of is to eagerly load So something like this should work:
I wonder if there would be a better way to do this in GraalVM or if this could be added to GraalVM itself unconditionally to avoid the |
It appears CI managed to reproduce one deadlock too yesterday trying to build |
That's inline with what I was thinking as well.
I don't know but this looks good enough to me. I also thought about abusing the "typeReachable" condition to only register the classes causing the issue when diff --git a/extensions/jaxb/deployment/src/main/java/io/quarkus/jaxb/deployment/JaxbProcessor.java b/extensions/jaxb/deployment/src/main/java/io/quarkus/jaxb/deployment/JaxbProcessor.java
index 17768f6d8e6..8d74a8c74d3 100644
--- a/extensions/jaxb/deployment/src/main/java/io/quarkus/jaxb/deployment/JaxbProcessor.java
+++ b/extensions/jaxb/deployment/src/main/java/io/quarkus/jaxb/deployment/JaxbProcessor.java
@@ -70,6 +70,7 @@
import io.quarkus.deployment.builditem.nativeimage.NativeImageResourceBundleBuildItem;
import io.quarkus.deployment.builditem.nativeimage.NativeImageSystemPropertyBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
+import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassConditionBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveHierarchyIgnoreWarningBuildItem;
import io.quarkus.deployment.builditem.nativeimage.RuntimeInitializedClassBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ServiceProviderBuildItem;
@@ -275,6 +276,7 @@ void registerClasses(
BuildProducer<NativeImageSystemPropertyBuildItem> nativeImageProps,
BuildProducer<ServiceProviderBuildItem> providerItem,
BuildProducer<ReflectiveClassBuildItem> reflectiveClass,
+ BuildProducer<ReflectiveClassConditionBuildItem> reflectiveClassCondition,
BuildProducer<NativeImageResourceBundleBuildItem> resourceBundle) {
addReflectiveClass(reflectiveClass, true, false, "org.glassfish.jaxb.runtime.v2.ContextFactory");
addReflectiveClass(reflectiveClass, true, false, "com.sun.xml.internal.stream.XMLInputFactoryImpl");
@@ -282,6 +284,13 @@ void registerClasses(
addReflectiveClass(reflectiveClass, true, false, "com.sun.org.apache.xpath.internal.functions.FuncNot");
addReflectiveClass(reflectiveClass, true, false, "com.sun.org.apache.xerces.internal.impl.dv.xs.SchemaDVFactoryImpl");
+ reflectiveClassCondition
+ .produce(new ReflectiveClassConditionBuildItem("com.sun.xml.internal.stream.XMLInputFactoryImpl",
+ "jdk.xml.internal.JdkXmlUtils"));
+ reflectiveClassCondition
+ .produce(new ReflectiveClassConditionBuildItem("com.sun.xml.internal.stream.XMLOutputFactoryImpl",
+ "jdk.xml.internal.JdkXmlUtils"));
+
addResourceBundle(resourceBundle, "jakarta.xml.bind.Messages");
addResourceBundle(resourceBundle, "jakarta.xml.bind.helpers.Messages");
diff --git a/extensions/jaxp/deployment/src/main/java/io/quarkus/jaxp/deployment/JaxpProcessor.java b/extensions/jaxp/deployment/src/main/java/io/quarkus/jaxp/deployment/JaxpProcessor.java
index b7b06fe00cd..f05000ad03d 100644
--- a/extensions/jaxp/deployment/src/main/java/io/quarkus/jaxp/deployment/JaxpProcessor.java
+++ b/extensions/jaxp/deployment/src/main/java/io/quarkus/jaxp/deployment/JaxpProcessor.java
@@ -8,11 +8,13 @@
import io.quarkus.deployment.builditem.nativeimage.NativeImageResourceBuildItem;
import io.quarkus.deployment.builditem.nativeimage.NativeImageResourceBundleBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
+import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassConditionBuildItem;
class JaxpProcessor {
@BuildStep
- void reflectiveClasses(BuildProducer<ReflectiveClassBuildItem> reflectiveClass) {
+ void reflectiveClasses(BuildProducer<ReflectiveClassBuildItem> reflectiveClass,
+ BuildProducer<ReflectiveClassConditionBuildItem> reflectiveClassConditional) {
reflectiveClass
.produce(ReflectiveClassBuildItem.builder("com.sun.org.apache.xerces.internal.jaxp.DocumentBuilderFactoryImpl",
"com.sun.org.apache.xerces.internal.jaxp.datatype.DatatypeFactoryImpl",
@@ -26,6 +28,10 @@ void reflectiveClasses(BuildProducer<ReflectiveClassBuildItem> reflectiveClass)
"com.sun.org.apache.xpath.internal.functions.FuncNot",
"com.sun.org.apache.xerces.internal.impl.dv.xs.SchemaDVFactoryImpl",
"javax.xml.namespace.QName").methods().build());
+
+ reflectiveClassConditional
+ .produce(new ReflectiveClassConditionBuildItem("com.sun.org.apache.xerces.internal.parsers.SAXParser",
+ "jdk.xml.internal.JdkXmlUtils"));
}
@BuildStep
I believe the
It's probably worth an upstream discussion. To my understanding the only reason upstream doesn't face the same issue (if they actually don't) is due to using the |
FWIW, I've got this commit with a tiny patch for upstream discussion. I'll try to create an upstream issue with a proposed PR later today or tomorrow. Two test runs with this (it also passes a very opinionated custom reproducer): |
Upstream issue: oracle/graal#8075 |
Update: The current expectation would be that this should be fixed (as in, not see any more random XML related initializer errors in any CI run) once |
oracle/graal#8193 is now merged. @jerboaa I have cherry picked oracle/graal@944f5d1 (the cherry-pick will be dropped once backported on upstream) in https://github.com/graalvm/mandrel/tree/mandrel/24.0 to work around oracle/graal#8195 and see if oracle/graal#8193 resolves the issue. |
Yes. We'll see if we will see failures again on the JDK 23 based trees going forward.
oracle/graal#8193 needs a backport to the |
Right. I openned graalvm/mandrel#667 to keep track of this. |
We've just had two failures again related to this issue:
So it looks like oracle/graal#8193 didn't fix the issue after all for JDK 23. |
oracle/graal#8256 fixed this for master (24.1), but CI is bust for JDK 23 builds[1] so we don't have good test coverage (yet). |
We have fixes for this in Graal master and the 24.0 tree and CI has run for a while without seeing this issue crop up again. I'm closing this issue as fixed. Master fix: oracle/graal#8256 |
Describe the bug
We see some native integration tests failing in our CI. For example IT
main
,jaxb
orspring-web
. All of them have a NPE in common:See: https://github.com/graalvm/mandrel/actions/runs/7154825726/job/19483074935#step:12:1688
In some cases it looks like this:
See: https://github.com/graalvm/mandrel/actions/runs/7161773572/job/19498392690#step:12:2306
The text was updated successfully, but these errors were encountered: