From b2aad1c3b1091ed0e4e1d1f337ffdb8714463762 Mon Sep 17 00:00:00 2001
From: Sam Brannen <sbrannen@pivotal.io>
Date: Tue, 17 Sep 2019 12:59:37 +0200
Subject: [PATCH] Ensure bean definitions can be removed concurrently

Prior to this commit, concurrent invocations of
DefaultListableBeanFactory.removeBeanDefinition() could result in a
NullPointerException.

This commit fixes this by adding an appropriate not-null check in
resetBeanDefinition().

Closes gh-23542
---
 .../support/DefaultListableBeanFactory.java   |  4 ++-
 .../DefaultListableBeanFactoryTests.java      | 27 +++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)

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 f970f8447ef..57bea2020d5 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
@@ -1003,7 +1003,9 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
 		for (String bdName : this.beanDefinitionNames) {
 			if (!beanName.equals(bdName)) {
 				BeanDefinition bd = this.beanDefinitionMap.get(bdName);
-				if (beanName.equals(bd.getParentName())) {
+				// Ensure bd is non-null due to potential concurrent modification
+				// of the beanDefinitionMap.
+				if (bd != null && beanName.equals(bd.getParentName())) {
 					resetBeanDefinition(bdName);
 				}
 			}
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 3883b76c5e9..9f27a5f7899 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
@@ -38,6 +38,7 @@ import java.util.Properties;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import javax.annotation.Priority;
 import javax.security.auth.Subject;
@@ -817,6 +818,32 @@ public class DefaultListableBeanFactoryTests {
 		assertTrue(lbf.getBean("test2") instanceof NestedTestBean);
 	}
 
+	@Test // gh-23542
+	public void concurrentBeanDefinitionRemoval() {
+		final int MAX = 200;
+		lbf.setAllowBeanDefinitionOverriding(false);
+
+		// Register the bean definitions before invoking preInstantiateSingletons()
+		// to simulate realistic usage of an ApplicationContext; otherwise, the bean
+		// factory thinks it's an "empty" factory which causes this test to fail in
+		// an unrealistic manner.
+		IntStream.range(0, MAX).forEach(this::registerTestBean);
+		lbf.preInstantiateSingletons();
+
+		// This test is considered successful if the following does not result in an exception.
+		IntStream.range(0, MAX).parallel().forEach(this::removeTestBean);
+	}
+
+	private void registerTestBean(int i) {
+		String name = "test" + i;
+		lbf.registerBeanDefinition(name, new RootBeanDefinition(TestBean.class));
+	}
+
+	private void removeTestBean(int i) {
+		String name = "test" + i;
+		lbf.removeBeanDefinition(name);
+	}
+
 	@Test
 	public void testBeanDefinitionOverridingNotAllowed() {
 		lbf.setAllowBeanDefinitionOverriding(false);