-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add opt-in link-at-build-time option to fail fast on NoClassDefFound #6725
Conversation
0b1eb3b
to
d225ddf
Compare
d225ddf
to
b0fc67f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This patch looks good to me. It gives users who rely on linkAtBuildTime the option to bail out with an informative error while still preserving the default behaviour: i.e. continuing the analysis with an UnknownType
but foregoing the opportunity to observe what was the cause of the linkage error. This change means that users who run into an unexplained linkage issue with the default setting will not be left in the dark. They can rerun with this flag set in order to identify what went wrong.
Throwable cause = e.getCause(); | ||
if (cause instanceof NoClassDefFoundError && linkAtBuildTime && LinkAtBuildTimeSupport.singleton().failFast()) { | ||
reportUnresolvedElement("type", method.format("%H.%n(%P)"), e); | ||
} else if (cause instanceof LinkageError) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this dropping the IllegalAccessError
check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it is a subclass of LinkageError
Works around oracle#6253 till a proper JVCMI fix is available. Closes: oracle#6253
b0fc67f
to
d1555ac
Compare
As mentioned in #6253 (comment) this appears to no longer be needed. |
Hi @christianwimmer and @cstancu
this is a respin of #6507
I understand that a proper solution requires JVMCI changes and I am willing to work on it given some guidance. However, this is expected to take significant time to reach a release.
In the meantime, #6253 (originally reported in #4661) is a long standing issue for Quarkus users and we would really like a fix for it (even a work around like the one I propose). Note that by default Quarkus initializes and links all classes at build time, so this is not a corner case for Quarkus.
This PR works around #6253, by providing users the option to fail fast if a class is missing and they are linking at build time. The option is intentionally made an opt-in in order to not alter the current behavior till a proper JVCMI fix is available and it should be completely removed once we get the JVMCI fix in.
Ideally we would like this backported to 23.0 as well.
To try this out you may run:
Passing
-H:+LinkAtBuildTimeFailFast
results in the following output:while without it we get: