Skip to content
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

Entity-less native compilation failures #5343

Merged
merged 3 commits into from
Nov 11, 2019

Conversation

Sanne
Copy link
Member

@Sanne Sanne commented Nov 8, 2019

Fixes #2482 and Fixes #5262

The culprit was the fact that when there's no entities, we'd bail out early.

In fact we'd abort a little too early, as it would by-pass setting a couple of key static fields which I had intentionally crafted to guide the code analysis of the native-image compiler.

Example symptoms include issues which seem related to entirely different components, such as:

Trace:
	at parsing java.lang.System$2.getConstantPool(System.java:1227)
Call path from entry point to java.lang.System$2.getConstantPool(Class):
	at java.lang.System$2.getConstantPool(System.java:1227)
	at sun.reflect.annotation.TypeAnnotationParser.parseAllTypeAnnotations(TypeAnnotationParser.java:323)
	at sun.reflect.annotation.TypeAnnotationParser.fetchBounds(TypeAnnotationParser.java:299)
	at sun.reflect.annotation.TypeAnnotationParser.parseAnnotatedBounds(TypeAnnotationParser.java:244)
	at sun.reflect.annotation.TypeAnnotationParser.parseAnnotatedBounds(TypeAnnotationParser.java:237)
	at sun.reflect.generics.reflectiveObjects.TypeVariableImpl.getAnnotatedBounds(TypeVariableImpl.java:241)
	at com.oracle.svm.reflect.TypeVariable_getAnnotatedBounds_88ea462da2b17ea45028db307519aaeeec9a4036_776.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.jboss.resteasy.core.ContextParameterInjector$GenericDelegatingProxy.invoke(ContextParameterInjector.java:118)
	at com.sun.proxy.$Proxy225.hashCode(Unknown Source)
	at java.util.HashMap.hash(HashMap.java:339)
	at java.util.HashMap.get(HashMap.java:557)

I've also seen this manifest as failures around:

  • JTA logging system, which defines an illegal invocationhandler during initialization
  • Various Byte Buddy helpers doing illegal stuff like reading annotation parameters in toString(), equals() and hashcode implementations
  • the RESTEasy default invocation handler attempting to define a class
  • (unexplained) attempts to load javax/security/jacc/PolicyContextException
  • failure as it would reach io.undertow.protocols.alpn.OpenSSLAlpnProvider.setProtocols
  • .. !

The most puzzling failure I got was this one:

        at java.lang.reflect.Method.invoke(Method.java:498)
        at javax.enterprise.util.AnnotationLiteral.invoke(AnnotationLiteral.java:288)
        at javax.enterprise.util.AnnotationLiteral.getMemberValue(AnnotationLiteral.java:276)
        at javax.enterprise.util.AnnotationLiteral.toString(AnnotationLiteral.java:135)
        at java.lang.String.valueOf(String.java:2994)
        at java.lang.StringBuilder.append(StringBuilder.java:131)
        at com.oracle.svm.core.amd64.AMD64CPUFeatureAccess.verifyHostSupportsArchitecture

at which I almost gave up 🔢 😕

@Sanne Sanne added the backport? label Nov 8, 2019
@Sanne Sanne added this to the 1.1.0 milestone Nov 8, 2019
@jaikiran
Copy link
Member

jaikiran commented Nov 9, 2019

Agreed. I gave up on this one a while back for the same reasons as you state - every time I thought I had narrowed this down and did some change to "fix" it, it would show up with some other odd and hard to understand error :) Glad you narrowed it down to this fix.

@geoand
Copy link
Contributor

geoand commented Nov 9, 2019

Looks good to me, but I don't know enough about the hibernate integration to provide any valuable comments or feedback.
Who should review this one @Sanne?
Seems like we definitely want it in sooner rather than later.

@rsvoboda
Copy link
Member

rsvoboda commented Nov 9, 2019

@Sanne, issue I commented about in #2482 (comment) is no longer visible to me with this fix.

@Sanne
Copy link
Member Author

Sanne commented Nov 11, 2019

@rsvoboda thanks for checking! I forgot about that one. Is there another issue I should mark resolved by this one or are we good as it is?

@Sanne
Copy link
Member Author

Sanne commented Nov 11, 2019

@geoand anyone can review it. The Hibernate integration is stuff I did myself, so as long as it fixes stuff and nobody sees any risk of regressions, I just need an approval to merge ;)

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems absolutely reasonable to me. I'll let @rsvoboda verify that everything checks out with the other issue as well

@Sanne
Copy link
Member Author

Sanne commented Nov 11, 2019

A suggestion to any reviewer: revert the patch, keep the integration test. Look how horribly it fails :-)

@Sanne
Copy link
Member Author

Sanne commented Nov 11, 2019

thanks for the approval @geoand ! I'm fairly confident about this being necessary, I'll merge.

If @rsvoboda could check if there's more issues to be marked as resolved, we can do that when he sees this.

@Sanne Sanne added kind/bug Something isn't working area/user-experience Will make us lose users labels Nov 11, 2019
@Sanne Sanne merged commit 987e0db into quarkusio:master Nov 11, 2019
@Sanne Sanne deleted the EntityLessNativeFailure branch November 11, 2019 08:44
@geoand
Copy link
Contributor

geoand commented Nov 11, 2019

@Sanne makes sense!

@jaikiran
Copy link
Member

@Sanne, I'm not 100% sure, but I think native tests that run against the automated CI are selectively enabled (in https://github.com/quarkusio/quarkus/blob/master/ci-templates/stages.yml?). So I think this new integration-test will have to be included in there for it to catch any regressions in native mode.

@geoand
Copy link
Contributor

geoand commented Nov 11, 2019

@jaikiran your understanding is correct!

However in this case I don't think we need to run the integration test in native mode, since it's more about addressing a build time issue.
Up to @Sanne of course to decide :)

@jaikiran
Copy link
Member

jaikiran commented Nov 11, 2019

Hello @geoand,

However in this case I don't think we need to run the integration test in native mode, since it's more about addressing a build time issue.

Just so I understand how things are run - does this mean that a native image will be created for this integration test, even if it's not explicitly listed in that CI template? So, that CI template just controls which native tests are run against these (always) generated native images?

@geoand
Copy link
Contributor

geoand commented Nov 11, 2019

Hey @jaikiran

No, no native image will be created in this case, just a regular jvm build and test.

@rsvoboda
Copy link
Member

@Sanne didn't find any additional unresolved issue related to this PR / topic

@Sanne
Copy link
Member Author

Sanne commented Nov 11, 2019

@geoand this failure would happen during a native-image build though, so I think @jaikiran is correct that CI wouldn't detect regressions here.

That might need a follow up, I'll open an issue to track as I can't do it myself today.

@geoand
Copy link
Contributor

geoand commented Nov 11, 2019

Oh OK. I thought it showed up in a normal build as well, my bad :)

@machi1990
Copy link
Member

@geoand this failure would happen during a native-image build though, so I think @jaikiran is correct that CI wouldn't detect regressions here.

That might need a follow up, I'll open an issue to track as I can't do it myself today.

Hi @Sanne @geoand, I just landed on this space. To be sure I followed the missing piece for CI is it about adding jpa-without-entity module in native ci? If so I can come up with a PR.

@geoand
Copy link
Contributor

geoand commented Nov 11, 2019

@machi1990 yes, that is correct

@machi1990
Copy link
Member

Cool, let me do it.

@Sanne
Copy link
Member Author

Sanne commented Nov 11, 2019

Many thanks @machi1990 ! Opened #5369

machi1990 added a commit to machi1990/quarkus that referenced this pull request Nov 11, 2019
@gsmet gsmet removed the backport? label Nov 14, 2019
@gsmet gsmet modified the milestones: 1.1.0, 1.0.0.Final Nov 14, 2019
gsmet pushed a commit that referenced this pull request Nov 14, 2019
ia3andy pushed a commit to dmlloyd/quarkus that referenced this pull request Nov 19, 2019
@Sanne
Copy link
Member Author

Sanne commented Nov 25, 2019

@galderz have a look at the errors this solved - both the "stacktrace" (it's not a stacktrace) I pasted in this PR and the linked issues it resolved.

mmusgrov pushed a commit to mmusgrov/quarkus that referenced this pull request Dec 13, 2019
@hakdogan
Copy link

hakdogan commented Dec 6, 2020

Hi,

I want to report a problem I think is related to this issue.

I have an entity-less application that has hibernate-orm dependency and uses QuarkusApplication. One of the app's dependency includes the entity and it has beans.xml.

First I got NoClassDefFoundError exception for javax/security/jacc/PolicyContextException in native complation time.

After I added the javax.security.jacc-api dependency to the pom, I got the following error

Error: No instances of java.net.SocksSocketImpl are allowed in the image heap as this class should be initialized at image runtime. To see how this object got instantiated use -H:+TraceClassInitialization.

Quarkus version 1.9.2.Final
GraalVM version 20.2.0

@Sanne
Copy link
Member Author

Sanne commented Dec 7, 2020

Hi, if you have an PolicyContextException exception in native, adding jacc-api is not the right solution; one would need to understand why this class was being attempted to be loaded -especially important as it seems from the second error that this is bringing you into a rabbit hole :)

Why do you have a dependency on hibernate-orm if you're not using it?

Probably best to open a new issue.

@hakdogan
Copy link

hakdogan commented Dec 7, 2020

Hi,

Why do you have a dependency on hibernate-orm if you're not using it?

I use it, only my entity object is in a dependency. As you said, it will be better to open a new issue and discuss the issue there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user-experience Will make us lose users kind/bug Something isn't working
Projects
None yet
7 participants