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

Runtime hint registration for property binding should not fail when parameter information is unavailable #40051

Closed
sdeleuze opened this issue Mar 21, 2024 · 8 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@sdeleuze
Copy link
Contributor

sdeleuze commented Mar 21, 2024

On Spring Petclinic during a native build or a JVM one with the AOT plugin enabled, if I change the pom.xml to use tomcat-jdbc as following:

    <dependency>
      <groupId>org.springframework.boot</groupId>
      <artifactId>spring-boot-starter-data-jpa</artifactId>
      <exclusions>
        <exclusion>
          <groupId>com.zaxxer</groupId>
          <artifactId>HikariCP</artifactId>
        </exclusion>
      </exclusions>
    </dependency>
    <dependency>
      <groupId>org.apache.tomcat</groupId>
      <artifactId>tomcat-jdbc</artifactId>
    </dependency>

I get the following exception while building the application:

Exception in thread "main" org.springframework.boot.context.properties.bind.MissingParametersCompilerArgumentException: Constructor binding in a native image requires compilation with -parameters but the following classes were compiled without it:
        org.apache.tomcat.jdbc.pool.PoolProperties$InterceptorProperty

        at org.springframework.boot.context.properties.bind.BindableRuntimeHintsRegistrar.registerHints(BindableRuntimeHintsRegistrar.java:100)
        at org.springframework.boot.context.properties.ConfigurationPropertiesBeanFactoryInitializationAotProcessor$ConfigurationPropertiesReflectionHintsContribution.applyTo(ConfigurationPropertiesBeanFactoryInitializationAotProcessor.java:74)
        at org.springframework.context.aot.BeanFactoryInitializationAotContributions.applyTo(BeanFactoryInitializationAotContributions.java:78)

Would be nice if the registrar would ignore failures due to a lack of parameter information.
Notice the application works fine without AOT involved.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Mar 21, 2024
@sdeleuze sdeleuze changed the title Make BindableRuntimeHintsRegistrar more lenient when parameters lack of parameter information Make BindableRuntimeHintsRegistrar more lenient when there is a lack of parameter information Mar 21, 2024
@wilkinsona wilkinsona changed the title Make BindableRuntimeHintsRegistrar more lenient when there is a lack of parameter information Runtime hint registration for property binding should not fail when parameter information is unavailable Mar 21, 2024
@wilkinsona wilkinsona added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Mar 21, 2024
@wilkinsona wilkinsona added this to the 3.1.x milestone Mar 21, 2024
@wilkinsona
Copy link
Member

Test that reproduces the problem:

@Test
void registerHintsDoesNotThrowWhenParameterInformationForConstructorBindingIsNotAvailable()
		throws NoSuchMethodException, SecurityException {
	Constructor<?> constructor = PoolProperties.InterceptorProperty.class.getConstructor(String.class, String.class);
	String[] parameterNames = new StandardReflectionParameterNameDiscoverer().getParameterNames(constructor);
	assertThat(parameterNames).isNull();
	assertThatNoException().isThrownBy(() -> registerHints(PoolProperties.class));
}

@rlawngus0910
Copy link

I checked the commit that added MissingParametersCompilerArgumentException.
I think you should attach '-parameters' to build.gradle unconditionally when building with native image.
421f2fa

Also, if you check the issue below, it seems that another error will occur if you do not throw MissingParametersComputerArgumentException.
#33182

@mhalbritter
Copy link
Contributor

The line which throws the exception has been added in #33182.

If we remove the exception, the build works, but the application still doesn't work on native, because without -parameters the LocalVariableTableParameterNameDiscoverer fallback is used (which doesn't work in a native image).

@wilkinsona
Copy link
Member

@mhalbritter Can you share the failure? I'm not sure why we're trying to bind org.apache.tomcat.jdbc.pool.PoolProperties$InterceptorProperty at runtime. Hopefully the stack trace might shed some light on it.

@mhalbritter
Copy link
Contributor

mhalbritter commented Apr 22, 2024

Oh sorry, that was not clearly phrased. I haven't tested it, just read through the history of #33182.

Now that I've tested it, indeed the native image application works fine, even when using the spring.datasource.tomcat properties.

So this looks like a false positive of the early failure detection introduced in #33182.

@wilkinsona
Copy link
Member

Great, thank you. That's what I expected. I think this issue shows that the early failure detection is a bit too much as it may generate false positives. I think we should remove it.

@mhalbritter
Copy link
Contributor

mhalbritter commented Apr 22, 2024

The chain it follows is: PoolProperties -> InterceptorDefinition[] getJdbcInterceptorsAsArray() -> InterceptorDefinition -> Map<String, InterceptorProperty> getProperties() -> InterceptorProperty -> constructor InterceptorProperty(String name, String value). Because it has decided for InterceptorProperty to use constructor binding, it checks if it has been compiled with -parameters, which it hasn't, and then it fails.

@mhalbritter
Copy link
Contributor

I have removed the exception.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants