From 9af239c8be3ea4a9b4fb30085e9738f4a85b1434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Nicoll?= Date: Fri, 13 Oct 2023 16:58:02 +0200 Subject: [PATCH] Clean resources in case of unexpected exception This commit updates AbstractApplicationContext#refresh to handle any exceptions, not only BeansExceptions. Closes gh-28878 --- .../support/AbstractApplicationContext.java | 5 +- ...AbstractRefreshableApplicationContext.java | 4 +- .../support/GenericApplicationContext.java | 2 +- .../GenericApplicationContextTests.java | 61 +++++++++++++++++++ 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java b/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java index 7ddd510abf2..071ea63d945 100644 --- a/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java +++ b/spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java @@ -619,12 +619,11 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader finishRefresh(); } - catch (BeansException ex) { + catch (RuntimeException | Error ex ) { if (logger.isWarnEnabled()) { logger.warn("Exception encountered during context initialization - " + "cancelling refresh attempt: " + ex); } - // Destroy already created singletons to avoid dangling resources. destroyBeans(); @@ -974,7 +973,7 @@ public abstract class AbstractApplicationContext extends DefaultResourceLoader * after an exception got thrown. * @param ex the exception that led to the cancellation */ - protected void cancelRefresh(BeansException ex) { + protected void cancelRefresh(Throwable ex) { this.active.set(false); // Reset common introspection caches in Spring's core infrastructure. diff --git a/spring-context/src/main/java/org/springframework/context/support/AbstractRefreshableApplicationContext.java b/spring-context/src/main/java/org/springframework/context/support/AbstractRefreshableApplicationContext.java index 9c87844e9d7..fd1bc8e12ff 100644 --- a/spring-context/src/main/java/org/springframework/context/support/AbstractRefreshableApplicationContext.java +++ b/spring-context/src/main/java/org/springframework/context/support/AbstractRefreshableApplicationContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 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. @@ -136,7 +136,7 @@ public abstract class AbstractRefreshableApplicationContext extends AbstractAppl } @Override - protected void cancelRefresh(BeansException ex) { + protected void cancelRefresh(Throwable ex) { DefaultListableBeanFactory beanFactory = this.beanFactory; if (beanFactory != null) { beanFactory.setSerializationId(null); diff --git a/spring-context/src/main/java/org/springframework/context/support/GenericApplicationContext.java b/spring-context/src/main/java/org/springframework/context/support/GenericApplicationContext.java index 7faac66dcf4..55047b4d03b 100644 --- a/spring-context/src/main/java/org/springframework/context/support/GenericApplicationContext.java +++ b/spring-context/src/main/java/org/springframework/context/support/GenericApplicationContext.java @@ -297,7 +297,7 @@ public class GenericApplicationContext extends AbstractApplicationContext implem } @Override - protected void cancelRefresh(BeansException ex) { + protected void cancelRefresh(Throwable ex) { this.beanFactory.setSerializationId(null); super.cancelRefresh(ex); } diff --git a/spring-context/src/test/java/org/springframework/context/support/GenericApplicationContextTests.java b/spring-context/src/test/java/org/springframework/context/support/GenericApplicationContextTests.java index baa0bb1e161..6e28f2b08bd 100644 --- a/spring-context/src/test/java/org/springframework/context/support/GenericApplicationContextTests.java +++ b/spring-context/src/test/java/org/springframework/context/support/GenericApplicationContextTests.java @@ -29,7 +29,11 @@ import org.mockito.ArgumentCaptor; import org.springframework.aot.hint.RuntimeHints; import org.springframework.aot.hint.predicate.RuntimeHintsPredicates; import org.springframework.beans.BeansException; +import org.springframework.beans.factory.BeanCreationException; +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; import org.springframework.beans.factory.NoUniqueBeanDefinitionException; +import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.config.AbstractFactoryBean; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanFactoryPostProcessor; @@ -59,6 +63,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.InstanceOfAssertFactories.type; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; @@ -302,6 +307,39 @@ class GenericApplicationContextTests { .isEqualTo("pong:foo"); } + @Test + void refreshWithRuntimeFailureOnBeanCreationDisposeExistingBeans() { + BeanE one = new BeanE(); + context.registerBean("one", BeanE.class, () -> one); + context.registerBean("two", BeanE.class, () -> new BeanE() { + @Override + public void afterPropertiesSet() { + throw new IllegalStateException("Expected"); + } + }); + assertThatThrownBy(context::refresh).isInstanceOf(BeanCreationException.class) + .hasMessageContaining("two"); + assertThat(one.initialized).isTrue(); + assertThat(one.destroyed).isTrue(); + } + + @Test + void refreshWithRuntimeFailureOnAfterSingletonInstantiatedDisposeExistingBeans() { + BeanE one = new BeanE(); + BeanE two = new BeanE(); + context.registerBean("one", BeanE.class, () -> one); + context.registerBean("two", BeanE.class, () -> two); + context.registerBean("int", SmartInitializingSingleton.class, () -> () -> { + throw new IllegalStateException("expected"); + }); + assertThatThrownBy(context::refresh).isInstanceOf(IllegalStateException.class) + .hasMessageContaining("expected"); + assertThat(one.initialized).isTrue(); + assertThat(two.initialized).isTrue(); + assertThat(one.destroyed).isTrue(); + assertThat(two.destroyed).isTrue(); + } + @Test void refreshForAotSetsContextActive() { GenericApplicationContext context = new GenericApplicationContext(); @@ -646,6 +684,29 @@ class GenericApplicationContextTests { } } + static class BeanE implements InitializingBean, DisposableBean { + + private boolean initialized; + + private boolean destroyed; + + @Override + public void afterPropertiesSet() throws Exception { + if (initialized) { + throw new IllegalStateException("AfterPropertiesSet called twice"); + } + this.initialized = true; + } + + @Override + public void destroy() throws Exception { + if (destroyed) { + throw new IllegalStateException("Destroy called twice"); + } + this.destroyed = true; + } + } + static class TestAotFactoryBean extends AbstractFactoryBean {