Avoid synchronization for shortcut re-resolution

See gh-30883
spring-projects · Jul 16, 2023 · 161a717 · 161a717
1 parent 28e63e9
commit 161a717
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 7 deletions.
diff --git a/...ng-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java b/...ng-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java
@@ -832,14 +832,18 @@ private Object[] resolvePreparedArguments(String beanName, RootBeanDefinition mb
 				}
 				catch (BeansException ex) {
 					// Unexpected target bean mismatch for cached argument -> re-resolve
-					synchronized (descriptor) {
-						if (!descriptor.hasShortcut()) {
-							throw ex;
-						}
+					Set<String> autowiredBeanNames = null;
+					if (descriptor.hasShortcut()) {
+						// Reset shortcut and try to re-resolve it in this thread...
 						descriptor.setShortcut(null);
-						Set<String> autowiredBeanNames = new LinkedHashSet<>(2);
-						argValue = resolveAutowiredArgument(descriptor, paramType, beanName,
-								autowiredBeanNames, converter, true);
+						autowiredBeanNames = new LinkedHashSet<>(2);
+					}
+					logger.debug("Failed to resolve cached argument", ex);
+					argValue = resolveAutowiredArgument(descriptor, paramType, beanName,
+							autowiredBeanNames, converter, true);
+					if (autowiredBeanNames != null && !descriptor.hasShortcut()) {
+						// We encountered as stale shortcut before, and the shortcut has
+						// not been re-resolved by another thread in the meantime...
 						if (argValue != null) {
 							setShortcutIfPossible(descriptor, paramType, autowiredBeanNames);
 						}

diff --git a/...g/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java b/...g/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java
@@ -423,6 +423,22 @@ void optionalResourceInjectionWithSingletonRemoval() {
 		assertThat(bean.nestedTestBeansField[1]).isSameAs(ntb2);
 
 		bf.destroySingleton("testBean");
+		bf.registerSingleton("testBeanX", tb);
+
+		bean = bf.getBean("annotatedBean", OptionalResourceInjectionBean.class);
+		assertThat(bean.getTestBean()).isSameAs(tb);
+		assertThat(bean.getTestBean2()).isSameAs(tb);
+		assertThat(bean.getTestBean3()).isSameAs(tb);
+		assertThat(bean.getTestBean4()).isSameAs(tb);
+		assertThat(bean.getIndexedTestBean()).isSameAs(itb);
+		assertThat(bean.getNestedTestBeans()).hasSize(2);
+		assertThat(bean.getNestedTestBeans()[0]).isSameAs(ntb1);
+		assertThat(bean.getNestedTestBeans()[1]).isSameAs(ntb2);
+		assertThat(bean.nestedTestBeansField).hasSize(2);
+		assertThat(bean.nestedTestBeansField[0]).isSameAs(ntb1);
+		assertThat(bean.nestedTestBeansField[1]).isSameAs(ntb2);
+
+		bf.destroySingleton("testBeanX");
 
 		bean = bf.getBean("annotatedBean", OptionalResourceInjectionBean.class);
 		assertThat(bean.getTestBean()).isNull();
@@ -480,6 +496,22 @@ void optionalResourceInjectionWithBeanDefinitionRemoval() {
 		assertThat(bean.nestedTestBeansField[1]).isSameAs(ntb2);
 
 		bf.removeBeanDefinition("testBean");
+		bf.registerBeanDefinition("testBeanX", new RootBeanDefinition(TestBean.class));
+
+		bean = bf.getBean("annotatedBean", OptionalResourceInjectionBean.class);
+		assertThat(bean.getTestBean()).isSameAs(bf.getBean("testBeanX"));
+		assertThat(bean.getTestBean2()).isSameAs(bf.getBean("testBeanX"));
+		assertThat(bean.getTestBean3()).isSameAs(bf.getBean("testBeanX"));
+		assertThat(bean.getTestBean4()).isSameAs(bf.getBean("testBeanX"));
+		assertThat(bean.getIndexedTestBean()).isSameAs(itb);
+		assertThat(bean.getNestedTestBeans()).hasSize(2);
+		assertThat(bean.getNestedTestBeans()[0]).isSameAs(ntb1);
+		assertThat(bean.getNestedTestBeans()[1]).isSameAs(ntb2);
+		assertThat(bean.nestedTestBeansField).hasSize(2);
+		assertThat(bean.nestedTestBeansField[0]).isSameAs(ntb1);
+		assertThat(bean.nestedTestBeansField[1]).isSameAs(ntb2);
+
+		bf.removeBeanDefinition("testBeanX");
 
 		bean = bf.getBean("annotatedBean", OptionalResourceInjectionBean.class);
 		assertThat(bean.getTestBean()).isNull();
@@ -768,6 +800,17 @@ void constructorResourceInjectionWithSingletonRemoval() {
 		assertThat(bean.getBeanFactory()).isSameAs(bf);
 
 		bf.destroySingleton("nestedTestBean");
+		bf.registerSingleton("nestedTestBeanX", ntb);
+
+		bean = bf.getBean("annotatedBean", ConstructorResourceInjectionBean.class);
+		assertThat(bean.getTestBean()).isSameAs(tb);
+		assertThat(bean.getTestBean2()).isSameAs(tb);
+		assertThat(bean.getTestBean3()).isSameAs(tb);
+		assertThat(bean.getTestBean4()).isSameAs(tb);
+		assertThat(bean.getNestedTestBean()).isSameAs(ntb);
+		assertThat(bean.getBeanFactory()).isSameAs(bf);
+
+		bf.destroySingleton("nestedTestBeanX");
 
 		bean = bf.getBean("annotatedBean", ConstructorResourceInjectionBean.class);
 		assertThat(bean.getTestBean()).isSameAs(tb);
@@ -806,6 +849,17 @@ void constructorResourceInjectionWithBeanDefinitionRemoval() {
 		assertThat(bean.getBeanFactory()).isSameAs(bf);
 
 		bf.removeBeanDefinition("nestedTestBean");
+		bf.registerBeanDefinition("nestedTestBeanX", new RootBeanDefinition(NestedTestBean.class));
+
+		bean = bf.getBean("annotatedBean", ConstructorResourceInjectionBean.class);
+		assertThat(bean.getTestBean()).isSameAs(tb);
+		assertThat(bean.getTestBean2()).isSameAs(tb);
+		assertThat(bean.getTestBean3()).isSameAs(tb);
+		assertThat(bean.getTestBean4()).isSameAs(tb);
+		assertThat(bean.getNestedTestBean()).isSameAs(bf.getBean("nestedTestBeanX"));
+		assertThat(bean.getBeanFactory()).isSameAs(bf);
+
+		bf.removeBeanDefinition("nestedTestBeanX");
 
 		bean = bf.getBean("annotatedBean", ConstructorResourceInjectionBean.class);
 		assertThat(bean.getTestBean()).isSameAs(tb);