From 2e1538a20b585132dcc671ac0a2491e4d3b7ab5a Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 8 Aug 2022 14:33:00 +0200 Subject: [PATCH] Make sure inferred destroy method is set on the original bean definition 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 --- ...nitDestroyAnnotationBeanPostProcessor.java | 32 +++++++++++++------ .../support/AbstractBeanDefinition.java | 8 +++++ .../support/DisposableBeanAdapter.java | 9 ++++-- .../factory/support/RootBeanDefinition.java | 9 ------ ...stroyAnnotationBeanPostProcessorTests.java | 13 ++++++++ 5 files changed, 49 insertions(+), 22 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java index 6b7da1d5b2..85a75c8456 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessor.java @@ -41,7 +41,9 @@ import org.springframework.beans.BeansException; import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.aot.BeanRegistrationAotContribution; import org.springframework.beans.factory.aot.BeanRegistrationAotProcessor; +import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.DestructionAwareBeanPostProcessor; +import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.support.MergedBeanDefinitionPostProcessor; import org.springframework.beans.factory.support.RegisteredBean; import org.springframework.beans.factory.support.RootBeanDefinition; @@ -158,20 +160,30 @@ public class InitDestroyAnnotationBeanPostProcessor implements DestructionAwareB @Override public BeanRegistrationAotContribution processAheadOfTime(RegisteredBean registeredBean) { - RootBeanDefinition beanDefinition = registeredBean.getMergedBeanDefinition(); - beanDefinition.resolveDestroyMethodIfNecessary(); - LifecycleMetadata metadata = findInjectionMetadata(beanDefinition, registeredBean.getBeanClass()); - if (!CollectionUtils.isEmpty(metadata.initMethods)) { - String[] initMethodNames = safeMerge(beanDefinition.getInitMethodNames(), metadata.initMethods); - beanDefinition.setInitMethodNames(initMethodNames); - } - if (!CollectionUtils.isEmpty(metadata.destroyMethods)) { - String[] destroyMethodNames = safeMerge(beanDefinition.getDestroyMethodNames(), metadata.destroyMethods); - beanDefinition.setDestroyMethodNames(destroyMethodNames); + AbstractBeanDefinition beanDefinition = getOriginalBeanDefinition(registeredBean); + if (beanDefinition != null) { + RootBeanDefinition mergedBeanDefinition = registeredBean.getMergedBeanDefinition(); + beanDefinition.resolveDestroyMethodIfNecessary(); + LifecycleMetadata metadata = findInjectionMetadata(mergedBeanDefinition, registeredBean.getBeanClass()); + if (!CollectionUtils.isEmpty(metadata.initMethods)) { + String[] initMethodNames = safeMerge(beanDefinition.getInitMethodNames(), metadata.initMethods); + beanDefinition.setInitMethodNames(initMethodNames); + } + if (!CollectionUtils.isEmpty(metadata.destroyMethods)) { + String[] destroyMethodNames = safeMerge(beanDefinition.getDestroyMethodNames(), metadata.destroyMethods); + beanDefinition.setDestroyMethodNames(destroyMethodNames); + } + registeredBean.getBeanFactory().clearMetadataCache(); } return null; } + @Nullable + private AbstractBeanDefinition getOriginalBeanDefinition(RegisteredBean registeredBean) { + BeanDefinition beanDefinition = registeredBean.getBeanFactory().getBeanDefinition(registeredBean.getBeanName()); + return (beanDefinition instanceof AbstractBeanDefinition abd ? abd : null); + } + private LifecycleMetadata findInjectionMetadata(RootBeanDefinition beanDefinition, Class beanType) { LifecycleMetadata metadata = findLifecycleMetadata(beanType); metadata.checkConfigMembers(beanDefinition); diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java index 201e3d2718..ef223e569e 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java @@ -1186,6 +1186,14 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess } } + /** + * Resolve the inferred destroy method if necessary. + * @since 6.0 + */ + public void resolveDestroyMethodIfNecessary() { + setDestroyMethodNames(DisposableBeanAdapter + .inferDestroyMethodsIfNecessary(getResolvableType().toClass(), this)); + } /** * Public declaration of Object's {@code clone()} method. diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java index 374e65aa0d..91fa30031b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DisposableBeanAdapter.java @@ -344,13 +344,14 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable { * interfaces, reflectively calling the "close" method on implementing beans as well. */ @Nullable - static String[] inferDestroyMethodsIfNecessary(Class target, RootBeanDefinition beanDefinition) { + static String[] inferDestroyMethodsIfNecessary(Class target, AbstractBeanDefinition beanDefinition) { String[] destroyMethodNames = beanDefinition.getDestroyMethodNames(); if (destroyMethodNames != null && destroyMethodNames.length > 1) { return destroyMethodNames; } - String destroyMethodName = beanDefinition.resolvedDestroyMethodName; + String destroyMethodName = (beanDefinition instanceof RootBeanDefinition rbd + ? rbd.resolvedDestroyMethodName : null); if (destroyMethodName == null) { destroyMethodName = beanDefinition.getDestroyMethodName(); boolean autoCloseable = (AutoCloseable.class.isAssignableFrom(target)); @@ -378,7 +379,9 @@ class DisposableBeanAdapter implements DisposableBean, Runnable, Serializable { } } } - beanDefinition.resolvedDestroyMethodName = (destroyMethodName != null ? destroyMethodName : ""); + if (beanDefinition instanceof RootBeanDefinition rbd) { + rbd.resolvedDestroyMethodName = (destroyMethodName != null ? destroyMethodName : ""); + } } return (StringUtils.hasLength(destroyMethodName) ? new String[] {destroyMethodName} : null); } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java index 7a50e6f015..72fd154fe3 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java @@ -550,15 +550,6 @@ public class RootBeanDefinition extends AbstractBeanDefinition { } } - /** - * Resolve the inferred destroy method if necessary. - * @since 6.0 - */ - public void resolveDestroyMethodIfNecessary() { - setDestroyMethodNames(DisposableBeanAdapter - .inferDestroyMethodsIfNecessary(getResolvableType().toClass(), this)); - } - /** * Register an externally managed configuration destruction method — * for example, a method annotated with JSR-250's diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessorTests.java index 9856e9d1cf..09f40c10f0 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/InitDestroyAnnotationBeanPostProcessorTests.java @@ -90,6 +90,19 @@ class InitDestroyAnnotationBeanPostProcessorTests { assertThat(mergedBeanDefinition.getDestroyMethodNames()).containsExactly("close"); } + @Test + void processAheadOfTimeWhenHasInferredDestroyMethodIsRetainedIfMergedBeanDefinitionIsStale() { + RootBeanDefinition beanDefinition = new RootBeanDefinition(InferredDestroyBean.class); + beanDefinition.setDestroyMethodNames(AbstractBeanDefinition.INFER_METHOD); + processAheadOfTime(beanDefinition); + RootBeanDefinition mergedBeanDefinition = getMergedBeanDefinition(); + assertThat(mergedBeanDefinition.getInitMethodNames()).isNull(); + assertThat(mergedBeanDefinition.getDestroyMethodNames()).containsExactly("close"); + RootBeanDefinition originalBeanDefinition = (RootBeanDefinition) this.beanFactory.getBeanDefinition("test"); + assertThat(originalBeanDefinition.getInitMethodNames()).isNull(); + assertThat(originalBeanDefinition.getDestroyMethodNames()).containsExactly("close"); + } + @Test void processAheadOfTimeWhenHasInferredDestroyMethodAndNoCandidateDoesNotMutateRootBeanDefinition() { RootBeanDefinition beanDefinition = new RootBeanDefinition(NoInitDestroyBean.class);