Fix JAXB substitutions for native-image

[zakkak]

• Unconditionally substitute jaxb methods to detect changes in libraries through failures, as done for other libraries
• Remove substitutions for com.sun.xml.internal.* Classes, they don't exist in the org.glassfish.jaxb:jaxb-runtime jaxb implementation

The above two were not done previously in order to support Java 8 which Quarkus no longer supports.

[zakkak]

Hi @Sanne and @yrodiere, I see from the gitlogs that you have done some work on HibernateOrmProcessor regarding registering Proxy Classes for native compilation. Could you please have a look at this Draft PR and provide some guidance on how to properly tackle this?

The issue is that with 22.1 Quarkus now returns runtime errors like the following:

com.oracle.svm.core.jdk.UnsupportedFeatureError: Proxy class defined by interfaces [interface javax.json.bind.annotation.JsonbTransient, interface com.sun.xml.bind.v2.model.annotation.Locatable] not found. Generating proxy classes at runtime is not supported. Proxy classes need to be defined at image build time by specifying the list of interfaces that they implement. To define proxy classes use -H:DynamicProxyConfigurationFiles=<comma-separated-config-files> and -H:DynamicProxyConfigurationResources=<comma-separated-config-resources> options.

I started by making sure the reported Proxy Classes mentioned in the errors are properly registered, but stopped once I saw that new Proxy classes (not referenced till now) are needed to be registered. Do you have any recommendation on how to find all the Proxy classes that are needed to be registered and how to decide what would be the best place to register them? (i.e. how to decide if I should register them in the hibernate-orm extension or jaxb or elsewhere?)

[yrodiere]

Hi @zakkak,

So... do I understand well that some annotations used not to be included in the runtime image, and therefore no proxy was ever being created for those annotations?

And now that annotations are present in the runtime image, the annotation proxies do get instantiated?

I'm not sure trying to add annotation proxies is a good idea... Mainly because Quarkus used to work before, without ever needing these annotations or instantiating these proxies, so evidently they were not necessary.

In the case of Hibernate ORM, we add annotation proxies because we know they are necessary at runtime. So, it's different.

I can think of two other approaches, feel free to tell me if they are not viable:

1. Add substitutions to avoid ever instantiating these proxies at runtime. I see we already have something about Locatable in the jaxb extension: io.quarkus.jaxb.runtime.graal.Target_com_sun_xml_internal_bind_v2_model_annotation_LocatableAnnotation.Locatable
2. Prevent GraalVM from adding these annotations... somehow? Maybe substitutions allow something like that (removing a class entirely)?

As to where the fixes should be... I'd say that depends on what instantiates the annotation proxies. If it's jaxb, the fix should be in the jaxb extension for example. Do you have stack traces?

[Sanne]

I'm not sure the problems you are seeing are actually being caused by Hibernate. From the look of the errors, it seems reminiscent of those situations in which GraalVM gets confused regarding the callpath and reports a call path which in fact isn't reachable in practice.
I've seen this happening most often when combined with the flag which allows a broken classpath to be compiled - is that flag set, or perhaps the other change making that the default was merged already?

[zakkak]

So... do I understand well that some annotations used not to be included in the runtime image, and therefore no proxy was ever being created for those annotations?

And now that annotations are present in the runtime image, the annotation proxies do get instantiated?

@yrodiere Yes, that's my understanding as well!

I'm not sure trying to add annotation proxies is a good idea... Mainly because Quarkus used to work before, without ever needing these annotations or instantiating these proxies, so evidently they were not necessary.

I agree with you. The argument behind this decision is:

"""
The previous criteria for inclusion could lead to configuration-related failures when an annotation got newly referenced anywhere in a project, whereas the current behaviour is completely predictable.
"""

Please feel free to argue against that in oracle/graal#4414

In the case of Hibernate ORM, we add annotation proxies because we know they are necessary at runtime. So, it's different.

I can think of two other approaches, feel free to tell me if they are not viable:

1. Add substitutions to avoid ever instantiating these proxies at runtime. I see we already have something about `Locatable` in the jaxb extension: `io.quarkus.jaxb.runtime.graal.Target_com_sun_xml_internal_bind_v2_model_annotation_LocatableAnnotation.Locatable`

I am not sure whether this will work. I will need to check. Thanks for the suggestion!

2. Prevent GraalVM from adding these annotations... somehow? Maybe substitutions allow something like that (removing a class entirely)?

Well I can't think of any other way than using substitutions so it looks the same to option 1 :)
I'll if it works.

As to where the fixes should be... I'd say that depends on what instantiates the annotation proxies. If it's jaxb, the fix should be in the jaxb extension for example. Do you have stack traces?

Yes I do, I'll come back if I need more help on that front. Thanks again

I'm not sure the problems you are seeing are actually being caused by Hibernate. From the look of the errors, it seems reminiscent of those situations in which GraalVM gets confused regarding the callpath and reports a call path which in fact isn't reachable in practice. I've seen this happening most often when combined with the flag which allows a broken classpath to be compiled - is that flag set, or perhaps the other change making that the default was merged already?

@Sanne I agree that these call paths are not reachable, the thing is that GraalVM >= 22.1 now includes all the annotations so that even if something becomes reachable it will be able to support it without requiring reconfiguration, at least according to oracle/graal#4414 (comment). Regarding the allow-incomplete-classpath this is indeed the new default, but Quarkus main is already flipping that default for GraalVM >= 22.1 (see #24213).

[yrodiere]

I'm not sure trying to add annotation proxies is a good idea... Mainly because Quarkus used to work before, without ever needing these annotations or instantiating these proxies, so evidently they were not necessary.

I agree with you. The argument behind this decision is:

""" The previous criteria for inclusion could lead to configuration-related failures when an annotation got newly referenced anywhere in a project, whereas the current behaviour is completely predictable. """

Please feel free to argue against that in oracle/graal#4414

Well I don't necessarily disagree with that change.

In a nutshell:

• Whether GraalVM adding annotation classes to the runtime image is a good idea... I don't know.
• Whether us adding annotation proxies to the runtime image is a good idea: I don't think so, and would rather use substitutions if we can.

[Sanne]

@zakkak but my objection isn't about reachability, but about impossible callstacks. For example, looking into the stacktraces on oracle/graal#4414 there's none actually involving org.hibernate code.

So that's why I wonder if this actually relates to Hibernate; yet is seems a common problem of integration tests which also happen to have Hibernate included in the application. So it's possible the reported callstacks are being confused - I've seen that happening before.

Do you have other stacktraces or callpaths actually pointing to specific Hibernate code?

[zakkak]

Following @yrodiere 's suggestion I traced the issue to be in JAXBSubstitutions.java which no longer substitutes some of the methods due to changes between the JDK 8 implementation of jaxb (which Quarkus no longer supports) and the one provided by org.glassfish.jaxb:jaxb-runtime which is being used now. Fixing the substitutions resolves the issue with the proxy classes as well.

[Sanne]

That had an awful lot of failures... I've rebased the branch to see if a fresh build helps