Call Lifecycle.stop() for already started beans on failed refresh

Closes gh-20028

Author: jhoeller

Diff for: spring-context/src/main/java/org/springframework/context/support/DefaultLifecycleProcessor.java

@@ -159,6 +159,8 @@ private ConfigurableListableBeanFactory getBeanFactory() {
 	public void start() {
 		this.stoppedBeans = null;
 		startBeans(false);
+		// If any bean failed to explicitly start, the exception propagates here.
+		// The caller may choose to subsequently call stop() if appropriate.
 		this.running = true;
 	}
 
@@ -183,7 +185,15 @@ public void onRefresh() {
 		}
 
 		this.stoppedBeans = null;
-		startBeans(true);
+		try {
+			startBeans(true);
+		}
+		catch (ApplicationContextException ex) {
+			// Some bean failed to auto-start within context refresh:
+			// stop already started beans on context refresh failure.
+			stopBeans();
+			throw ex;
+		}
 		this.running = true;
 	}
 

Diff for: spring-context/src/test/java/org/springframework/context/support/DefaultLifecycleProcessorTests.java

@@ -24,12 +24,14 @@
 import org.springframework.beans.factory.FactoryBean;
 import org.springframework.beans.factory.config.BeanDefinition;
 import org.springframework.beans.factory.support.RootBeanDefinition;
+import org.springframework.context.ApplicationContextException;
 import org.springframework.context.Lifecycle;
 import org.springframework.context.LifecycleProcessor;
 import org.springframework.context.SmartLifecycle;
 import org.springframework.core.testfixture.EnabledForTestGroups;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.springframework.core.testfixture.TestGroup.LONG_RUNNING;
 
 /**
@@ -107,6 +109,21 @@ void singleSmartLifecycleAutoStartupWithLazyInitFactoryBean() {
 		context.close();
 	}
 
+	@Test
+	void singleSmartLifecycleAutoStartupWithFailingLifecycleBean() {
+		CopyOnWriteArrayList<Lifecycle> startedBeans = new CopyOnWriteArrayList<>();
+		TestSmartLifecycleBean bean = TestSmartLifecycleBean.forStartupTests(1, startedBeans);
+		bean.setAutoStartup(true);
+		StaticApplicationContext context = new StaticApplicationContext();
+		context.getBeanFactory().registerSingleton("bean", bean);
+		context.registerSingleton("failingBean", FailingLifecycleBean.class);
+		assertThat(bean.isRunning()).isFalse();
+		assertThatExceptionOfType(ApplicationContextException.class)
+				.isThrownBy(context::refresh).withCauseInstanceOf(IllegalStateException.class);
+		assertThat(bean.isRunning()).isFalse();
+		assertThat(startedBeans).hasSize(1);
+	}
+
 	@Test
 	void singleSmartLifecycleWithoutAutoStartup() {
 		CopyOnWriteArrayList<Lifecycle> startedBeans = new CopyOnWriteArrayList<>();
@@ -832,4 +849,23 @@ public int getPhase() {
 		}
 	}
 
+
+	public static class FailingLifecycleBean implements SmartLifecycle {
+
+		@Override
+		public void start() {
+			throw new IllegalStateException();
+		}
+
+		@Override
+		public void stop() {
+			throw new IllegalStateException();
+		}
+
+		@Override
+		public boolean isRunning() {
+			return false;
+		}
+	}
+
 }