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

Enable ThrowMissingRegistrationErrors with GraalVM >= 23.1 #36378

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

zakkak
Copy link
Contributor

@zakkak zakkak commented Oct 10, 2023

"This feature will become available as non-experimental in the next release (GraalVM for Java 22), and will be promoted to the default behavior as the community adopts it." -- @vjovanov

See oracle/graal#5173 (comment)

@quarkus-bot

This comment has been minimized.

@jerboaa
Copy link
Contributor

jerboaa commented Oct 10, 2023

@zakkak Is there a test run of quarkus native tests with a graalvm for JDK 22 build with this somewhere (in order to gauge the impact of this change)?

@zakkak
Copy link
Contributor Author

zakkak commented Oct 16, 2023

nativeImageArgs.add("--strict-image-heap");
// This feature will become available as non-experimental in the next release (GraalVM for Java 22,
// GraalVM 24.0.0), and will be promoted to the default behavior as the community adopts it.
addExperimentalVMOption(nativeImageArgs, "-H:ThrowMissingRegistrationErrors=");

Choose a reason for hiding this comment

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

I am afraid this change could break downstream code. I would put a package filter that handles only Quarkus classes and Quarkus-managed libraries in code that goes to users. Then, we can ask the users in the next release to add --throw-missing-registration-errors= to their build.

We can use the unfiltered version for internal tests (ideally used with our dev builds). In tests, it is ideally used in combination with -H:MissingRegistrationReportingMode=Exit to make sure metadata is never missed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the input @vjovanov.

FYI when running without -H:MissingRegistrationReportingMode=Exit the Quarkus binary exits with error code 1 without printing anything.

When adding -H:MissingRegistrationReportingMode=Exit I get to see the exception:

org.graalvm.nativeimage.MissingReflectionRegistrationError: The program tried to reflectively access method sun.security.provider.NativePRNG#<init>(java.security.SecureRandomParameters) without it being registered for runtime reflection. Add sun.security.provider.NativePRNG#<init>(java.security.SecureRandomParameters) to the reflection metadata to solve this problem. See https://www.graalvm.org/latest/reference-manual/native-image/metadata/#reflection for help.

My understanding is that Quarkus catches the exception at some point which results in the error message not being shown, yet it still fails.

Copy link

@vjovanov vjovanov Oct 18, 2023

Choose a reason for hiding this comment

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

My understanding is that Quarkus catches the exception at some point which results in the error message not being shown, yet it still fails.

I would say the same. That is why we introduced -H:MissingRegistrationReportingMode=Exit.
To speed up the migration you can run the programs with -H:MissingRegistrationReportingMode=Warn and it will only print the missing elements.

@zakkak zakkak marked this pull request as draft October 17, 2023 07:12
@zakkak
Copy link
Contributor Author

zakkak commented Oct 17, 2023

Converting to draft due to #36378 (comment) which requires further investigation.

@zakkak zakkak force-pushed the 2023-10-10-enable-ThrowMissingRegistrationErrors branch from bda05ba to ff90474 Compare October 17, 2023 07:14
@zakkak zakkak force-pushed the 2023-10-10-enable-ThrowMissingRegistrationErrors branch from ff90474 to 5919813 Compare January 25, 2024 07:31
@zakkak zakkak force-pushed the 2023-10-10-enable-ThrowMissingRegistrationErrors branch from 5919813 to b35a9cc Compare May 28, 2024 13:32
@quarkus-bot quarkus-bot bot added area/netty area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure labels May 28, 2024
@quarkus-bot

This comment has been minimized.

@zakkak zakkak force-pushed the 2023-10-10-enable-ThrowMissingRegistrationErrors branch from 31c1600 to 98e0541 Compare May 29, 2024 08:04
@quarkus-bot

This comment has been minimized.

This feature will become available as non-experimental in the next
release (GraalVM for Java 22), and will be promoted to the default
behavior as the community adopts it.
@zakkak zakkak force-pushed the 2023-10-10-enable-ThrowMissingRegistrationErrors branch from 98e0541 to 84e463f Compare July 18, 2024 09:25
@quarkus-bot

This comment has been minimized.

@zakkak zakkak force-pushed the 2023-10-10-enable-ThrowMissingRegistrationErrors branch from 26603b3 to 35501e1 Compare July 18, 2024 09:49
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 18, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 35501e1.

Note

🚫 This build has been cancelled.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/native-image area/netty
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants