From 427e5f58996ac15d95c14c99d56789c8c62de390 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 19 Aug 2022 17:30:59 +0200 Subject: [PATCH] Final merged bean definition clearing in freezeConfiguration Instead of individual last-minute clearing in markBeanAsCreated, the factory clears all merged bean definitions in freezeConfiguration, retaining the changes of merged bean definition post-processing after that point (in particular in refreshForAotProcessing). Closes gh-28948 --- .../ConfigurableListableBeanFactory.java | 8 +++++-- .../AbstractAutowireCapableBeanFactory.java | 2 +- .../factory/support/AbstractBeanFactory.java | 4 ++-- .../support/DefaultListableBeanFactory.java | 1 + .../factory/support/RootBeanDefinition.java | 11 +++++++++ .../DefaultListableBeanFactoryTests.java | 23 +++++++++++++++++++ .../PostProcessorRegistrationDelegate.java | 4 ++++ 7 files changed, 48 insertions(+), 5 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/config/ConfigurableListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/config/ConfigurableListableBeanFactory.java index 3b4c8f595e..249d6bc31d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/config/ConfigurableListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/config/ConfigurableListableBeanFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -137,7 +137,10 @@ public interface ConfigurableListableBeanFactory /** * Freeze all bean definitions, signalling that the registered bean definitions * will not be modified or post-processed any further. - *

This allows the factory to aggressively cache bean definition metadata. + *

This allows the factory to aggressively cache bean definition metadata + * going forward, after clearing the initial temporary metadata cache. + * @see #clearMetadataCache() + * @see #isConfigurationFrozen() */ void freezeConfiguration(); @@ -145,6 +148,7 @@ public interface ConfigurableListableBeanFactory * Return whether this factory's bean definitions are frozen, * i.e. are not supposed to be modified or post-processed any further. * @return {@code true} if the factory's configuration is considered frozen + * @see #freezeConfiguration() */ boolean isConfigurationFrozen(); diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index 331b633e6f..f4ff0f593d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -576,7 +576,7 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac throw new BeanCreationException(mbd.getResourceDescription(), beanName, "Post-processing of merged bean definition failed", ex); } - mbd.postProcessed = true; + mbd.markAsPostProcessed(); } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java index dddae5dcca..47e339331d 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java @@ -1700,12 +1700,12 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp protected void markBeanAsCreated(String beanName) { if (!this.alreadyCreated.contains(beanName)) { synchronized (this.mergedBeanDefinitions) { - if (!this.alreadyCreated.contains(beanName)) { + if (!isBeanEligibleForMetadataCaching(beanName)) { // Let the bean definition get re-merged now that we're actually creating // the bean... just in case some of its metadata changed in the meantime. clearMergedBeanDefinition(beanName); - this.alreadyCreated.add(beanName); } + this.alreadyCreated.add(beanName); } } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java index 9f5c3b4ec3..8bd9b30a0c 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java @@ -887,6 +887,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto @Override public void freezeConfiguration() { + clearMetadataCache(); this.configurationFrozen = true; this.frozenBeanDefinitionNames = StringUtils.toStringArray(this.beanDefinitionNames); } 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 cd1223e8a9..238f3c0e78 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 @@ -439,6 +439,17 @@ public class RootBeanDefinition extends AbstractBeanDefinition { } } + /** + * Mark this bean definition as post-processed, + * i.e. processed by {@link MergedBeanDefinitionPostProcessor}. + * @since 6.0 + */ + public void markAsPostProcessed() { + synchronized (this.postProcessingLock) { + this.postProcessed = true; + } + } + /** * Register an externally managed configuration method or field. */ diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java index d492f28d94..17f183ed0b 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java @@ -772,6 +772,29 @@ class DefaultListableBeanFactoryTests { assertThat(factory.getType("child")).isEqualTo(DerivedTestBean.class); } + @Test + void mergedBeanDefinitionChangesRetainedAfterFreezeConfiguration() { + RootBeanDefinition parentDefinition = new RootBeanDefinition(Object.class); + ChildBeanDefinition childDefinition = new ChildBeanDefinition("parent"); + + DefaultListableBeanFactory factory = new DefaultListableBeanFactory(); + factory.registerBeanDefinition("parent", parentDefinition); + factory.registerBeanDefinition("child", childDefinition); + + assertThat(factory.getType("parent")).isEqualTo(Object.class); + assertThat(factory.getType("child")).isEqualTo(Object.class); + ((RootBeanDefinition) factory.getBeanDefinition("parent")).setBeanClass(TestBean.class); + + factory.freezeConfiguration(); + + assertThat(factory.getType("parent")).isEqualTo(TestBean.class); + assertThat(factory.getType("child")).isEqualTo(TestBean.class); + ((RootBeanDefinition) factory.getMergedBeanDefinition("child")).setBeanClass(DerivedTestBean.class); + + assertThat(factory.getBean("parent")).isInstanceOf(TestBean.class); + assertThat(factory.getBean("child")).isInstanceOf(DerivedTestBean.class); + } + @Test void nameAlreadyBound() { Properties p = new Properties(); diff --git a/spring-context/src/main/java/org/springframework/context/support/PostProcessorRegistrationDelegate.java b/spring-context/src/main/java/org/springframework/context/support/PostProcessorRegistrationDelegate.java index 23e4be2f4f..4f5f0aabbe 100644 --- a/spring-context/src/main/java/org/springframework/context/support/PostProcessorRegistrationDelegate.java +++ b/spring-context/src/main/java/org/springframework/context/support/PostProcessorRegistrationDelegate.java @@ -423,6 +423,7 @@ final class PostProcessorRegistrationDelegate { } } + private static final class MergedBeanDefinitionPostProcessorInvoker { private final DefaultListableBeanFactory beanFactory; @@ -438,12 +439,14 @@ final class PostProcessorRegistrationDelegate { RootBeanDefinition bd = (RootBeanDefinition) this.beanFactory.getMergedBeanDefinition(beanName); Class beanType = resolveBeanType(bd); postProcessRootBeanDefinition(postProcessors, beanName, beanType, bd); + bd.markAsPostProcessed(); } registerBeanPostProcessors(this.beanFactory, postProcessors); } private void postProcessRootBeanDefinition(List postProcessors, String beanName, Class beanType, RootBeanDefinition bd) { + BeanDefinitionValueResolver valueResolver = new BeanDefinitionValueResolver(this.beanFactory, beanName, bd); postProcessors.forEach(postProcessor -> postProcessor.postProcessMergedBeanDefinition(bd, beanType, beanName)); for (PropertyValue propertyValue : bd.getPropertyValues().getPropertyValueList()) { @@ -466,6 +469,7 @@ final class PostProcessorRegistrationDelegate { private void resolveInnerBeanDefinition(BeanDefinitionValueResolver valueResolver, BeanDefinition innerBeanDefinition, BiConsumer resolver) { + valueResolver.resolveInnerBean(null, innerBeanDefinition, (name, rbd) -> { resolver.accept(name, rbd); return Void.class;