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

Code generation should not contribute (inferred) destroy method name #28215

Closed
snicoll opened this issue Mar 22, 2022 · 8 comments
Closed

Code generation should not contribute (inferred) destroy method name #28215

snicoll opened this issue Mar 22, 2022 · 8 comments
Assignees
Labels
theme: aot An issue related to Ahead-of-time processing type: bug A general bug
Milestone

Comments

@snicoll
Copy link
Member

snicoll commented Mar 22, 2022

(inferred) is used as a special method name within the core container. When AOT runs, it should handle that and not write that value.

@snicoll snicoll added type: bug A general bug theme: aot An issue related to Ahead-of-time processing labels Mar 22, 2022
@snicoll snicoll added this to the 6.0.0-M4 milestone Mar 22, 2022
@snicoll snicoll self-assigned this Mar 22, 2022
@snicoll snicoll changed the title Code generation should not contribute (inferred) init or destroy method names Code generation should not contribute (inferred) destroy method name Mar 22, 2022
@snicoll
Copy link
Member Author

snicoll commented Mar 22, 2022

Actually I am having some second thoughts on this one. @jhoeller what do you think?

@snicoll
Copy link
Member Author

snicoll commented Mar 23, 2022

We should "infer" the close method at build time rather than letting the container do this at runtime. As we're using reflection to invoke the method, that would miss a hint anyway.

@snicoll
Copy link
Member Author

snicoll commented Apr 4, 2022

We're going to try to lazily infer this when setTargetType is available in AOT use cases.

@jhoeller jhoeller modified the milestones: 6.0.0-M4, 6.0.0-M5 May 10, 2022
@snicoll snicoll removed their assignment Jun 21, 2022
@jhoeller jhoeller modified the milestones: 6.0.0-M5, 6.0.0-M6 Jul 13, 2022
@snicoll snicoll self-assigned this Jul 27, 2022
@snicoll
Copy link
Member Author

snicoll commented Aug 5, 2022

I am still seeing (inferred) in generated code so it looks like the fix above does not handle all cases.

@snicoll snicoll reopened this Aug 5, 2022
@snicoll
Copy link
Member Author

snicoll commented Aug 8, 2022

OK the good news is that it was a local change I was working on that mutates the bean definition late. This has the effect of making the cache stale or something which brings back the default value for some reason. We should identify why that is and keep the resolution around.

snicoll added a commit that referenced this issue Aug 8, 2022
This commit updates InitDestroyAnnotationBeanPostProcessor to mutate
the original bean definition rather than the merged one that can be
recreated without it if the cache gets stale.

See gh-28215
@snicoll
Copy link
Member Author

snicoll commented Aug 8, 2022

2e1538a now mutates the original bean definition.

@snicoll snicoll closed this as completed Aug 8, 2022
@bclozel
Copy link
Member

bclozel commented Sep 2, 2022

Removing the workaround in ApplicationContextAotGeneratorRuntimeHintsTests still fails with the following:

Missing <"ReflectionHints"> for invocation <java.lang.Class#getMethods> on type <org.springframework.context.testfixture.context.generator.annotation.InitDestroyComponent> 
with arguments [].
Stacktrace:
<"org.springframework.beans.BeanUtils#findMethodWithMinimalParameters, Line 350
org.springframework.beans.factory.support.DisposableBeanAdapter#findDestroyMethod, Line 261
org.springframework.beans.factory.support.DisposableBeanAdapter#determineDestroyMethod, Line 250
org.springframework.beans.factory.support.DisposableBeanAdapter#<init>, Line 120
org.springframework.beans.factory.support.AbstractBeanFactory#registerDisposableBeanIfNecessary, Line 1857

Is there another instance of this problem?

@bclozel bclozel reopened this Sep 2, 2022
@snicoll
Copy link
Member Author

snicoll commented Sep 5, 2022

Thanks for sharing that. It's unrelated to this specific issue and affects any use of a destroy method. I've created #29077

@snicoll snicoll closed this as completed Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: aot An issue related to Ahead-of-time processing type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants