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.

Sign up for GitHub

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

Jump to bottom

Enable --exact-reachability-metadata with Mandrel >= 23.1 (GraalVM for JDK 21) #41995

Open
zakkak opened this issue Jul 19, 2024 · 1 comment
Open

Enable --exact-reachability-metadata with Mandrel >= 23.1 (GraalVM for JDK 21) #41995

zakkak opened this issue Jul 19, 2024 · 1 comment
Labels
area/native-image kind/enhancement New feature or request

Comments

@zakkak
Copy link
Contributor

zakkak commented Jul 19, 2024 •
edited
Loading

Description

Background

Starting with Mandrel 23.0 (GraalVM for JDK 17) an option -H:+ThrowMissingRegistrationErrors was introduced to throw an exception

when it is impossible to tell whether a reflective query should fail because the queried element has not been registered at build-time, or because it doesn't exist.

See oracle/graal#6139

Starting with Mandrel 23.1 (GraalVM for JDK 21) the option was extended to also cover other elements, e.g., classes, resources, etc.

See https://github.com/oracle/graal/blob/release/graal-vm/23.1/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/doc-files/MissingRegistrationHelp.txt and oracle/graal#6448

Starting with Mandrel 24.1 (GraalVM for JDK 23) the option will be available as a public flag (meaning it's no longer experimental) with the intention to ultimately become the default.

See https://github.com/oracle/graal/blob/release/graal-vm/24.1/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/doc-files/ExactReachabilityMetadataHelp.txt, oracle/graal#5173 (comment) and oracle/graal#9036

Pros

Enabling the flag by default seems like a good idea since it will notify the users about elements that the code tries to access reflectively but have not been registered, which right now is leading in unexpected behavior without the users even knowing in some cases (for an example see #37626).

Cons/Difficulties

The so far identified cons and difficulties of having this option enabled by default are:

  1. The exception is only thrown at runtime, so one needs to run the native executable to see if it will throw the exception. This requires several run-fix-build-run cycles which is tedious... The process can be sped up by using the native-image-agent to generate the reachability-metadata configuration, but it still depends highly on how good coverage the run will have.
  2. Some libraries "blindly" try to access resources, e.g. SmallRye tries to find various configuration files in a few places leading to an exception being thrown even if in Quarkus we know we don't need these resources. Registering the said resources resolves the issue, but comes with the drawback that if any of these resources happens to be found at build-time it will actually end up in the native-executable and get loaded at run-time, which right now is not and the effects of it are unknown (see Enhance SmallRyeConfig handling  #41994)
  3. We need to make sure that registering things to avoid the exception doesn't change the intended behavior. E.g. if we don't want a configuration to be accessed at run-time we need to somehow make sure it's also not present at build-time so that it doesn't end up getting embedded in the native executable.

Implementation ideas

WIP PR #36378
@zakkak zakkak added the kind/enhancement New feature or request label Jul 19, 2024
@quarkus-bot
Copy link

quarkus-bot bot commented Jul 19, 2024

/cc @Karm (mandrel), @galderz (mandrel)

@zakkak zakkak changed the title Enable ThrowMissingRegistrationErrors with Mandrel >= 23.1 (GraalVM for JDK 21) Enable --exact-reachability-metadata with Mandrel >= 23.1 (GraalVM for JDK 21) Jul 22, 2024
@zakkak zakkak mentioned this issue Jul 22, 2024
Closed
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 23, 2024
@zakkak
Improve the way we register providers for reflection
4486fa2 
Instead of registering all constructors we only need to register the
nullary constructor.

Furthermore, we need to register the `provider()` method for reflective
lookup to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

The said constructor and method are accessed in
ServiceLoader#loadProvider and ServiceLoader#findStaticProviderMethod.

Relates to quarkusio#41995
@zakkak zakkak mentioned this issue Jul 23, 2024
Merged
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 23, 2024
@zakkak
Improve reflection registrations in picocli extension
8857958 
Picocli invokes getDeclaredFields on the declaring classes of annotated
fields so we need to register all fields instead of just the annotated
ones to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

Relates to quarkusio#41995
This was referenced Jul 23, 2024
Merged
Open
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jul 23, 2024
@zakkak @gsmet
Improve reflection registrations in picocli extension
a778fc9 
Picocli invokes getDeclaredFields on the declaring classes of annotated
fields so we need to register all fields instead of just the annotated
ones to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

Relates to quarkusio#41995

(cherry picked from commit 8857958)
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 23, 2024
@zakkak
Improve the way we register providers for reflection
862bebb 
Instead of registering all constructors we only need to register the
nullary constructor.

Furthermore, we need to register the `provider()` method for reflective
lookup to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

The said constructor and method are accessed in
ServiceLoader#loadProvider and ServiceLoader#findStaticProviderMethod.

Relates to quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 24, 2024
@zakkak
Register sun.security.provider.NativePRNG#<init> for reflection
a22461c 
When instantiating a SecureRandom the constructor reflectively looks for
the NativePRNG constructor and invokes it.

Although the lookup succeeds without the explicit registration, it's
better to explicitly request it. This also prevents getting a
`MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Relates to quarkusio#41995
@zakkak zakkak mentioned this issue Jul 24, 2024
Merged
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 24, 2024
@zakkak
Register sun.security.provider.NativePRNG#<init> for reflection
0e8f9cd 
When instantiating a SecureRandom the constructor reflectively looks for
the NativePRNG constructor and invokes it.

Although the lookup succeeds without the explicit registration, it's
better to explicitly request it. This also prevents getting a
`MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Relates to quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 24, 2024
@zakkak
Disable external configuration sources parsing in native-mode
acc264f 
`SmallryeConfigBuilder` tries to access properties files from various
sources, despite them not being available at run-time nor supported in
native-mode, see quarkusio#41994

This patch also avoids getting a `MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Related to quarkusio#41994 and
quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 24, 2024
@zakkak
Disable external configuration sources parsing in native-mode
f3c6c72 
`SmallryeConfigBuilder` tries to access properties files from various
sources, despite them not being available at run-time nor supported in
native-mode, see quarkusio#41994

This patch also avoids getting a `MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Related to quarkusio#41994 and
quarkusio#41995
@zakkak zakkak mentioned this issue Jul 24, 2024
Closed
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 25, 2024
@zakkak
Limit external configuration sources parsing in native-mode
fc678ed 
`SmallryeConfigBuilder` tries to access properties files from various
sources, despite them not being available at run-time nor supported in
native-mode, see quarkusio#41994

This patch also avoids getting a `MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Related to quarkusio#41994 and
quarkusio#41995
barreiro pushed a commit to barreiro/quarkus that referenced this issue Jul 25, 2024
@zakkak @barreiro
Improve reflection registrations in picocli extension
c6e4572 
Picocli invokes getDeclaredFields on the declaring classes of annotated
fields so we need to register all fields instead of just the annotated
ones to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

Relates to quarkusio#41995
@zakkak zakkak mentioned this issue Jul 25, 2024
Draft
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 26, 2024
@zakkak
Limit external configuration sources parsing in native-mode
628ee63 
`SmallryeConfigBuilder` tries to access properties files from various
sources, despite them not being available at run-time nor supported in
native-mode, see quarkusio#41994

This patch also avoids getting a `MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Related to quarkusio#41994 and
quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 30, 2024
@zakkak
Extend reflection configuration for VertX
c9edbe6 
Doesn't seem to fix any functionality issues but prevents
`MissingRegistrationErrors` from being thrown when
`ThrowMissingRegistrationErrors` is enabled.

Related to quarkusio#41995
@zakkak zakkak mentioned this issue Jul 30, 2024
Closed
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 31, 2024
@zakkak
Extend reflection configuration for VertX
ee25435 
Doesn't seem to fix any functionality issues but prevents
`MissingRegistrationErrors` from being thrown when
`ThrowMissingRegistrationErrors` is enabled.

Related to quarkusio#41995
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Jul 31, 2024
@zakkak @holly-cummins
Improve reflection registrations in picocli extension
2793909 
Picocli invokes getDeclaredFields on the declaring classes of annotated
fields so we need to register all fields instead of just the annotated
ones to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

Relates to quarkusio#41995
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Jul 31, 2024
@zakkak @holly-cummins
Improve the way we register providers for reflection
7900f41 
Instead of registering all constructors we only need to register the
nullary constructor.

Furthermore, we need to register the `provider()` method for reflective
lookup to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

The said constructor and method are accessed in
ServiceLoader#loadProvider and ServiceLoader#findStaticProviderMethod.

Relates to quarkusio#41995
holly-cummins pushed a commit to holly-cummins/quarkus that referenced this issue Jul 31, 2024
@zakkak @holly-cummins
Register sun.security.provider.NativePRNG#<init> for reflection
49ecb24 
When instantiating a SecureRandom the constructor reflectively looks for
the NativePRNG constructor and invokes it.

Although the lookup succeeds without the explicit registration, it's
better to explicitly request it. This also prevents getting a
`MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Relates to quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Jul 31, 2024
@zakkak
Extend reflection configuration for VertX
10e2b31 
Doesn't seem to fix any functionality issues but prevents
`MissingRegistrationErrors` from being thrown when
`ThrowMissingRegistrationErrors` is enabled.

Related to quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Aug 1, 2024
@zakkak
Extend reflection configuration for VertX
25421b5 
Doesn't seem to fix any functionality issues but prevents
`MissingRegistrationErrors` from being thrown when
`ThrowMissingRegistrationErrors` is enabled.

Related to quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Aug 1, 2024
@zakkak
Extend reflection configuration for VertX
39090e8 
Doesn't seem to fix any functionality issues but prevents
`MissingRegistrationErrors` from being thrown when
`ThrowMissingRegistrationErrors` is enabled.

Related to quarkusio#41995
gsmet pushed a commit to gsmet/quarkus that referenced this issue Aug 1, 2024
@zakkak @gsmet
Register sun.security.provider.NativePRNG#<init> for reflection
937991c 
When instantiating a SecureRandom the constructor reflectively looks for
the NativePRNG constructor and invokes it.

Although the lookup succeeds without the explicit registration, it's
better to explicitly request it. This also prevents getting a
`MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Relates to quarkusio#41995

(cherry picked from commit 0e8f9cd)
zakkak added a commit to zakkak/quarkus that referenced this issue Aug 2, 2024
@zakkak
Extend reflection configuration for VertX
3182f74 
Doesn't seem to fix any functionality issues but prevents
`MissingRegistrationErrors` from being thrown when
`ThrowMissingRegistrationErrors` is enabled.

Related to quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Aug 2, 2024
@zakkak
Extend reflection configuration for VertX
b0a9a9e 
Doesn't seem to fix any functionality issues but prevents
`MissingRegistrationErrors` from being thrown when
`ThrowMissingRegistrationErrors` is enabled.

Related to quarkusio#41995
zakkak added a commit to zakkak/quarkus that referenced this issue Sep 4, 2024
@zakkak
Extend reflection configuration for SmallRye FT
9812460 
Related to quarkusio#41995
danielsoro pushed a commit to danielsoro/quarkus that referenced this issue Sep 20, 2024
@zakkak @danielsoro
Improve reflection registrations in picocli extension
1cbabbd 
Picocli invokes getDeclaredFields on the declaring classes of annotated
fields so we need to register all fields instead of just the annotated
ones to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

Relates to quarkusio#41995
danielsoro pushed a commit to danielsoro/quarkus that referenced this issue Sep 20, 2024
@zakkak @danielsoro
Improve the way we register providers for reflection
62e9730 
Instead of registering all constructors we only need to register the
nullary constructor.

Furthermore, we need to register the `provider()` method for reflective
lookup to avoid `MissingReflectionRegistrationError`s at run-time when
using `-H:+ThrowMissingRegistrationErrors` or
`--exact-reachability-metadata`.

The said constructor and method are accessed in
ServiceLoader#loadProvider and ServiceLoader#findStaticProviderMethod.

Relates to quarkusio#41995
danielsoro pushed a commit to danielsoro/quarkus that referenced this issue Sep 20, 2024
@zakkak @danielsoro
Register sun.security.provider.NativePRNG#<init> for reflection
553e550 
When instantiating a SecureRandom the constructor reflectively looks for
the NativePRNG constructor and invokes it.

Although the lookup succeeds without the explicit registration, it's
better to explicitly request it. This also prevents getting a
`MissingRegistrationError` when using
`-H:+ThrowMissingRegistrationErrors` or `--exact-reachability-metadata`.

Relates to quarkusio#41995
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/native-image kind/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@zakkak