ghost
commented
3 years ago /cc @geoand
Closed jerboaa closed 2 years ago
ghost
commented
3 years ago /cc @geoand
geoand
commented
3 years ago Are there other similar issues that have reported?
zakkak
commented
3 years ago Are there other similar issues that have reported?
We are aware of:
quarkus-integration-test-hibernate-orm-panache-999-SNAPSHOT-runner
quarkus-integration-test-spring-web-999-SNAPSHOT-runner
quarkus-integration-test-liquibase-999-SNAPSHOT-runner(see https://github.com/quarkusio/quarkus/pull/14700#issuecomment-769891115)
gsmet
commented
3 years ago @geoand I'm interested in talking about this maybe on Friday when things are more quiet for me. Would it work for you?
geoand
commented
3 years ago Sure yeah
jerboaa
commented
3 years ago @geoand @gsmet
ImageIO support (in whatever library) really should be explicit opt-in. The reason for this is that basic ImageIO support works with GraalVM 21.0+, but it requires explicit help by running through the code in JVM mode and the Graal VM diagnostic agent attached so as to produce the right config for the native-image generator. Without that extra config which seems rather dynamic and application specific, the generated image then just fails at runtime.
One example of a runtime failure is here: https://github.com/quarkusio/quarkus/issues/12972#issuecomment-775981230
So, before https://github.com/quarkusio/quarkus/issues/13567 is implemented properly it makes little sense to let ImageIO classes into a generated native image.
geoand
commented
3 years ago @gsmet let's talk about this one on Monday, okay?
geoand
commented
3 years ago The spring-web integration test handles some XML stuff and thus it brings in JAX-B which as you mention brings in ImageIO
jerboaa
commented
3 years ago Right. Note that many other tests (or apps) use JAX-B in some form or another and it will affect them too. This spring-web integration test is just one example how this could look like.
gsmet
commented
3 years ago We discussed this with @geoand today.
If the issue boils down to the issue mentioned here: https://github.com/quarkusio/quarkus/pull/14700#issuecomment-769915587 then I'm not sure what we could do.
I have nothing against having a configuration property to enable ImageIO support (disabled by default) and a build item for extensions to potentially enable it (and that we can do!) but... we need to find a way to disable it properly in JAXB and for that, I think we will need help from the Mandrel team as it doesn't look obvious to us.
zakkak
commented
3 years ago @gsmet @geoand wouldn't a conditional @Delete do the trick?
If ImageIO support is not enabled we @Delete ImageIO.
geoand
commented
3 years ago I don't get what we would delete in that case
zakkak
commented
3 years ago I was thinking about "deleting" the whole ImageIO class like we do in DeleteIIOImageProviderHelper.java and if that's too aggressive we could possible substitute only specific methods from ImageIO like we do in DeleteIIOImageProvider.java
geoand
commented
3 years ago In the examples you mentioned, we are deleting RESTEasy classes.
But in this issue, it's a JAX-B class (com.sun.xml.bind.v2.model.impl.RuntimeBuiltinLeafInfoImpl) that loads javax.imageio.ImageIO in <clinit> (https://github.com/eclipse-ee4j/jaxb-ri/blob/2.3.3-b01-RI-RELEASE/jaxb-ri/runtime/impl/src/main/java/com/sun/xml/bind/v2/model/impl/RuntimeBuiltinLeafInfoImpl.java#L355-L443), so I don't understand what you are proposing we delete.
geoand
commented
3 years ago we need to find a way to disable it properly in JAXB and for that, I think we will need help from the Mandrel team as it doesn't look obvious to us.
What Guillaume is referring to here is that we would like the Mandrel team to write the (gigantic) necessary substitution that would be necessary for RuntimeBuiltinLeafInfoImpl to make it work as proposed
zakkak
commented
3 years ago ... to write the (gigantic) necessary substitution ...
I would try to avoid this kind of fix, since it essentially entails replicating code from the corresponding library. Ideally I would like a more generic approach that just "removes"/substitutes the classes that are causing the issue.
I toyed a bit with @Delete and unfortunately it turns out it doesn't do the trick. Even if we @Delete the classes GraalVM sees them as reachable and still brings in the dependencies.
Note: Even if it did work we would need to combine it with --report-unsupported-elements-at-runtime which is not desirable.
\me thinking...
geoand
commented
3 years ago As far as we know, the aforementioned class is the one causing the issue when XML is needed.
I personally don't see any other solution
gsmet
commented
3 years ago You can't use @Delete but you could substitute the methods of the called ImageIO methods to throw exceptions.
I suppose you wouldn't need the library if it's just shell methods throwing exceptions.
Now, how practical it is and how it will scale if we have a ton of different methods called...
zakkak
commented
3 years ago You can't use
@Deletebut you could substitute the methods of the called ImageIO methods to throw exceptions.
That's what I thought. Unfortunately substituting just the methods is not enough, we need to substitute the class initializers as well.
For example, compiling the following code does remove the liblcms dependency (due to the ImageIO.read substitution) but still brings in libawt and libawt_headless (due to the ImageIO class initialization).
import javax.imageio.ImageIO;
import java.io.File;
import java.io.IOException;
import java.awt.image.BufferedImage;
import com.oracle.svm.core.annotate.*;
public class AWT {
public static void main(String[] args) {
try {
ImageIO.read(new File("myfile.jpeg"));
} catch (IOException e) {
//
}
}
}
@TargetClass(className = "javax.imageio.ImageIO")
final class Target_javax_imageio_ImageIO {
@Substitute
public static BufferedImage read(File input) throws IOException {
throw new UnsupportedOperationException("Not implemented yet for GraalVM native images");
}
}I suppose you wouldn't need the library if it's just shell methods throwing exceptions.
Correct, but GraalVM can't figure this out.
Now, how practical it is and how it will scale if we have a ton of different methods called...
True.
As far as we know, the aforementioned class is the one causing the issue when XML is needed.
I personally don't see any other solution
I am also looking into how we could achieve this as well (i.e. substitute the class initializer). Unfortunately the following doesn't work:
@Substitute //
@TargetElement(name = "<clinit>")
public static void classInitializer() {
}I am also looking into how we could achieve this as well (i.e. substitute the class initializer). Unfortunately the following doesn't work:
@Substitute @TargetElement(name = "<clinit>") public static void classInitializer() { }
Yeah, that would have been cool :)
zakkak
commented
3 years ago That's what I thought. Unfortunately substituting just the methods is not enough, we need to substitute the class initializers as well.
For example, compiling the following code does remove the
liblcmsdependency (due to theImageIO.readsubstitution) but still brings inlibawtandlibawt_headless(due to theImageIOclass initialization).
FTR one can get this to work (i.e. remove all dependencies to libawt, libawt_headless, and libcms) by adding @Substitute to the class as well (see here for more details), e.g.:
import javax.imageio.ImageIO;
import java.io.File;
import java.io.IOException;
import java.awt.image.BufferedImage;
import com.oracle.svm.core.annotate.*;
public class AWT {
public static void main(String[] args) {
try {
ImageIO.read(new File("myfile.jpeg"));
} catch (IOException e) {
//
}
}
}
@Substitute // <-- Added
@TargetClass(className = "javax.imageio.ImageIO")
final class Target_javax_imageio_ImageIO {
@Substitute
public static BufferedImage read(File input) throws IOException {
throw new UnsupportedOperationException("Not implemented yet for GraalVM native images");
}
}Unfortunately this is not enough in our case since the graph libraries are still being pulled in and we need to perform similar substitutions for:
java.awt.Componentjava.awt.Graphicsjava.awt.Imagejava.awt.MediaTrackerjava.awt.image.BufferedImage\me exploring/working on :point_up:
I have also opened https://github.com/oracle/graal/issues/3225 to discuss how we could make this easier through GraalVM support.
zakkak
commented
3 years ago So far the best solution I could come up with is the following.
Substitute the inner class used in the RuntimeBuiltinLeafInfoImpl class initializer using the following:
package io.quarkus.runtime.graal;
import com.oracle.svm.core.annotate.*;
import java.awt.*;
import java.awt.image.BufferedImage;
@TargetClass(className = "com.sun.xml.bind.v2.model.impl.RuntimeBuiltinLeafInfoImpl$10")
final class Target_com_sun_xml_bind_v2_model_impl_RuntimeBuiltinLeafInfoImpl$10 {
@Substitute
private Image parse(CharSequence text) {
throw new UnsupportedOperationException("Not implemented yet for GraalVM native images");
}
@Substitute
private BufferedImage convertToBufferedImage(Image image) {
throw new UnsupportedOperationException("Not implemented yet for GraalVM native images");
}
@Substitute
private Target_Base64Data print(Image v) {
throw new UnsupportedOperationException("Not implemented yet for GraalVM native images");
}
}
@TargetClass(className = "com.sun.xml.bind.v2.runtime.unmarshaller.Base64Data")
final class Target_Base64Data {
}Unfortunately this requires the removal of some guarantees from GraalVM:
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/AnnotationSubstitutionProcessor.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/AnnotationSubstitutionProcessor.java
index 6bf8ba9fd12..fe04a936b6b 100644
--- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/AnnotationSubstitutionProcessor.java
+++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/substitute/AnnotationSubstitutionProcessor.java
@@ -277,8 +277,8 @@ public class AnnotationSubstitutionProcessor extends SubstitutionProcessor {
}
private void handleClass(Class<?> annotatedClass) {
- guarantee(Modifier.isFinal(annotatedClass.getModifiers()) || annotatedClass.isInterface(), "Annotated class must be final: %s", annotatedClass);
- guarantee(annotatedClass.getSuperclass() == Object.class || annotatedClass.isInterface(), "Annotated class must inherit directly from Object: %s", annotatedClass);
+// guarantee(Modifier.isFinal(annotatedClass.getModifiers()) || annotatedClass.isInterface(), "Annotated class must be final: %s", annotatedClass);
+// guarantee(annotatedClass.getSuperclass() == Object.class || annotatedClass.isInterface(), "Annotated class must inherit directly from Object: %s", annotatedClass);
if (!NativeImageGenerator.includedIn(ImageSingletons.lookup(Platform.class), lookupAnnotation(annotatedClass, Platforms.class))) {
return;
@@ -570,9 +570,9 @@ public class AnnotationSubstitutionProcessor extends SubstitutionProcessor {
}
private void handleSubstitutionClass(Class<?> annotatedClass, Class<?> originalClass) {
- // Not sure what happens if the target class is in a hierarchy - so prohibit that for now.
- guarantee(annotatedClass.isInterface() == originalClass.isInterface(), "if original is interface, target must also be interface: %s", annotatedClass);
- guarantee(originalClass.getSuperclass() == Object.class || originalClass.isInterface(), "target class must inherit directly from Object: %s", originalClass);
+// // Not sure what happens if the target class is in a hierarchy - so prohibit that for now.
+// guarantee(annotatedClass.isInterface() == originalClass.isInterface(), "if original is interface, target must also be interface: %s", annotatedClass);
+// guarantee(originalClass.getSuperclass() == Object.class || originalClass.isInterface(), "target class must inherit directly from Object: %s", originalClass);
ResolvedJavaType original = metaAccess.lookupJavaType(originalClass);
ResolvedJavaType annotated = metaAccess.lookupJavaType(annotatedClass);I have sent an email to the graalvm-dev list to see if those guarantees are indeed needed or if we could get rid of them.
jerboaa
commented
3 years ago @TargetClass(className = "com.sun.xml.bind.v2.model.impl.RuntimeBuiltinLeafInfoImpl$10")
@Delete
final class Target_com_sun_xml_bind_v2_model_impl_RuntimeBuiltinLeafInfoImpl_10 {
}Together with --report-unsupported-elements-at-runtime as a native image option seems to work, but is an ugly hack.
zakkak
commented
3 years ago Together with
--report-unsupported-elements-at-runtimeas a native image option seems to work, but is an ugly hack.
I would add risky and dev-hostile, on top of ugly :)
jerboaa
commented
3 years ago Together with
--report-unsupported-elements-at-runtimeas a native image option seems to work, but is an ugly hack.I would add risky and dev-hostile, on top of ugly :)
Yes. That being said, it could be viable for opt-in users who have the JAXB app (including imageio) well tested and now just want to get rid of the AWT cruft in the native image.
geoand
commented
3 years ago We really don't want to have to introduce global settings to get around extension specific issues
jerboaa
commented
3 years ago We really don't want to have to introduce global settings to get around extension specific issues
Makes sense.
zakkak
commented
3 years ago I have sent an email to the graalvm-dev list to see if those guarantees are indeed needed or if we could get rid of them.
Unfortunately, according to @christianwimmer (see here) those guarantees are still needed:
Hi,
First my usual disclaimer: You should not use substitutions at all. They are a maintenance nightmare.
Nothing has changed in the substitution system for a long time, so the comment is still accurate. The problem are virtual method calls in target classes. The image generator delegates all virtual method resolution to the HotSpot VM via JVMCI. But the HotSpot VM does not know anything about the substitutions and target classes. So if you have a virtual dispatch in a target class, very strange things could happen. Forcing target classes to be final and inheriting directly from Object means that there are only invokespecial and no dynamic dispatch.
Virtual method calls would also be the only reason for inheriting from target classes. All other use cases can be handled by having separate target classes, e.g. when you have classes A extends B, you can have separate Target_A and Target_B. And you can cast freely between A, B, Target_A, and Target_B - at run time only classes A and B exist.
-Christian
So in lack of a better alternative I think the way forward is https://github.com/oracle/graal/issues/3225
Karm
commented
2 years ago @geoand We can close this one.
Not exactly "fixed by" #20850, but we advice users to install AWT extension if they attempt to deserialize an image. Tested here: https://github.com/quarkusio/quarkus/blob/main/integration-tests/jaxb/src/test/java/io/quarkus/it/jaxb/JaxbAWTIT.java#L62
Describe the bug Building a native-image for the spring-web integration test with Graal VM 21.0 CE results in AWT classes getting pulled into the native image. This didn't happen with Graal VM 20.3 CE.
Expected behavior No AWT classes, and, thus, native OpenJDK library dependencies should get pulled into the native image.
Actual behavior AWT classes and dependencies get pulled in for a (seemingly) headless example app.
To Reproduce
Note
libawt_headless.aandliblcms.aetc. being added to the link command traced with-H:+TraceNativeToolUsage.Environment (please complete the following information):
uname -aorver: Linux x86_64java -version: 11.0.10+9Additional context See my comment about this in the resteasy ImageIO leak fix. Re-posting it here for posterity:
In an asynchronous discussion it was discovered that ImageIO usages in jaxb are legitimate. However, there should be some way to tell quarkus "this-is-a-headless-app-using-jaxb" and this, in turn, would remove/substitute ImageIO usages in jaxb and therefore not pull in AWT classes and native libs.