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

Please initialize SLF4J for Java 22 GraalVM native images #2600

Closed
joshlong opened this issue Mar 25, 2024 · 5 comments
Closed

Please initialize SLF4J for Java 22 GraalVM native images #2600

joshlong opened this issue Mar 25, 2024 · 5 comments
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@joshlong
Copy link

The native image.properties needs to be updated for Java 22 requirements

see this comment in a bug report for an example of the bug and the fix

@fniephaus
Copy link

Hey, I work on the GraalVM team and have looked into this a bit. We've made some improvements and changes with regard to build-time initialization and I believe the current native-image.properties file no longer works with --strict-image-heap in GraalVM for JDK 21+. It sets --initialize-at-build-time for org.slf4j.LoggerFactory, which in turn allocates objects from the org.slf4j.helpers package, so the policy is incomplete.

Long story short, I think it's a good idea to revise the native-image.properties file, especially --initialize-at-build-time needs to be used very carefully and I'm not sure the current entries are in the right place or even needed:

  • org.conscrypt: This package probably provides a security provider that needs to be installed at build time. However, it'd be better if --initialize-at-build-time=org.conscrypt is done within org.conscrypt itself and not indirectly by the gax package.
  • org.slf4j.LoggerFactory: I'm not sure why this is needed. Maybe because conscrypt uses slf4j? If it's really needed, then the org.slf4j.helpers package (or individual types from that package) must also be marked for build time initialization for completeness.
  • org.junit.platform.engine.TestTag: I'm not sure this should ever be part of production code, even though it shouldn't have an effect when JUnit is not on the class/module path. I also don't see why only this type is needed and nothing else from JUnit.
  • --initialize-at-run-time=com.google.api.client.googleapis.services.AbstractGoogleClientRequest$ApiClientVersion: Another item I'm not sure is really needed. This only makes sense if types initialized at build time create instances of ApiClientVersion, but I don't see how that'd be the case.

If someone maintain the gax package could take a look, that'd be great. Also, feel free to reach out to me if you have any questions.

@blakeli0 blakeli0 added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p2 Moderately-important priority. Fix may not be included in next release. labels Mar 25, 2024
@blakeli0
Copy link
Collaborator

blakeli0 commented Mar 25, 2024

Thanks @joshlong for reporting this issue and thanks @fniephaus for the comprehensive explanation! We'll take a look and see what we can do. + @mpeddada1 @burkedavison

@mpeddada1
Copy link
Contributor

Thank you so much for the very detailed context! Taking a look at these suggestions.

@mpeddada1
Copy link
Contributor

Thanks again for bringing this up @joshlong! And for your input @fniephaus! We have re-evaluated the use of these configurations in native-image.properties and have removed two configurations (build time initialization of org.slf4j.LoggerFactory and run time initialization of AbstractGoogleClientRequest$ApiClientVersion) that we've determined to be obsolete. This change has been included as part of libraries-bom:26.38.0 . Keeping org.junit.platform.engine.TestTag which is used by tests that use the Category tag (reproducer) and org.conscrypt to avoid any unintentional breaking changes for now.

Closing this issue but please feel free to open a new one if you're still running into any issues!

@fniephaus
Copy link

Thanks for the update, @mpeddada1!

It seems that org.junit.platform.engine.TestTag is needed to make mvn test -P native work, which triggers the native testing infrastructure provided by our Native Build Tools (NBT). To me, this looks like a bug in our NBTs that we should fix, e.g. by extending this list.

Initializing org.conscrypt at build time is probably required to make the security provider work (that's currently a requirement). Ideally, however, the option is set by org.conscrypt itself, not this sdk-platform-java project. If you could help upstreaming the option to the conscrypt project, that'd be great.

Anyway, the main issue, build-time init of slf4j, has been resolved, so that you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

4 participants