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

Access checking fails due to wrong `getCallerClass()` results #1479

Closed
dmlloyd opened this issue Jul 8, 2019 · 15 comments

Comments

@dmlloyd
Copy link
Contributor

commented Jul 8, 2019

I don't know the exact issue but I think Reflection.getCallerClass() is (maybe only sometimes) wrong now. If you look at the stack trace of quarkusio/quarkus#3074, you can see that the result of Reflection.getCallerClass() within Constructor was com.sun.xml.bind.v2.runtime.ClassBeanInfoImpl, which is one frame lower than the class which should have been returned. I'm thinking it might be a result of a14d2f1, specifically the factoring out of StackTraceUtils. The problem occurs in 19.1.0 but not 19.0.x.

Reference: quarkusio/quarkus#3074

/cc @christianwimmer

@christianwimmer

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

Fixed in 506f2f1

@gsmet

This comment has been minimized.

Copy link

commented Jul 9, 2019

@christianwimmer cool thanks. Any idea when the next micro release is due? We can't upgrade Quarkus to use 19.1.0 until this one is fixed.

Thanks!

@christianwimmer

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

@gsmet There should be a 19.1.1 soon for the Oracle's July critical patch update. I'll work to get it in there.

In general, we would like to test Quarkus before we make a release (ideally in our gate before every merge). Can you provide @cstancu with some input on what and how to test automatically?

@dmlloyd

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

It's quite easy. You do need to have Apache Maven on your PATH though.

  • Check out the project
  • Set JAVA_HOME either to the GraalVM distro or to a JVMCI-enabled JDK (only Java 8 is supported but 11 should come soon)
  • Set GRAALVM_HOME to the GraalVM distro
  • mvn clean install -Dnative

It takes quite a while to build all the integration tests with native unfortunately.

@christianwimmer

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

Is there a possibility to run the same tests without needing the Quarkus repository, i.e., just with a Quarkus release on Maven?

@dmlloyd

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2019

The integration tests are not runnable without checking out the source at present.

@cstancu

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

@dmlloyd I tried the quarkus integration tests with the GraalVM latest master and Quarkus - Integration Tests - Vert.x fails with Error: Already registered: sun.nio.ch.DatagramChannelImpl.disconnect0(FileDescriptor, boolean) com.oracle.svm.core.util.UserError$UserException: Already registered: sun.nio.ch.DatagramChannelImpl.disconnect0(FileDescriptor, boolean). This is likely because you still have the substitution in your repo and in the meantime, we integrated the PR for the same substitution in our repo.

@cstancu

This comment has been minimized.

Copy link
Member

commented Jul 9, 2019

@dmlloyd Would it be possible to make a Quarkus release variant that also includes the integration tests in the future? That would make it easier for us to integrate them in our internal CI.

@bobmcwhirter

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

@christianwimmer

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

How many workarounds are there in Quarkus? Can you push them all out before we make the next release?

In the meantime and to run with multiple different GraalVM release, you can make substitutions conditional on the GraalVM version number by using the onlyWith attribute of substitutions.

Currently we have a bit of a testing issue: until we reach a point where you can test the Quarkus master with the most recent GraalVM release, and we can test the most recent Quarkus release with GraalVM master, there will always be a manual integration step that discovers some problems.

For us it is also difficult (for various non-technical reasons) to test based on the Quarkus source code, so having a way to run the Quarkus test only using Maven artifacts would be very helpful.

@dmlloyd

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

How many workarounds are there in Quarkus?

Usually not many. In fact I'm surprised about the Vert.x problem because nothing like that came up in my last round of tests, just a few days ago.

Using onlyWith is a good idea; I'll pass that on.

@dmlloyd

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

What's the best API to query to get the current GraalVM version?

@christianwimmer

This comment has been minimized.

Copy link
Member

commented Jul 10, 2019

The system property org.graalvm.version contains the GraalVM version string.

@olpaw @gilles-duboscq we should probably add a GraalVM API (either in ImageInfo or even better somewhere that is native image independent) to do version comparisons by string, i.e., to ask queries like "am I running on GraalVM version 19.1 or higher".

@dmlloyd

This comment has been minimized.

Copy link
Contributor Author

commented Jul 11, 2019

quarkusio/quarkus#3188 should hopefully fix the problem.

@cstancu

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

@dmlloyd I can confirm that the latest Quarkus master now successfully builds with the latest GraalVM master using mvn clean install -Dnative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.