Skip to content

Commit

Permalink
Clean resources in case of unexpected exception
Browse files Browse the repository at this point in the history
This commit updates AbstractApplicationContext#refresh to handle any
runtime exceptions, not only BeansExceptions. If the exception is not
a BeansException, it is wrapped in an ApplicationContextException.

Closes spring-projectsgh-28878
  • Loading branch information
snicoll committed Oct 13, 2023
1 parent 607c84f commit caebff1
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.springframework.beans.support.ResourceEditorRegistrar;
import org.springframework.context.ApplicationContext;
import org.springframework.context.ApplicationContextAware;
import org.springframework.context.ApplicationContextException;
import org.springframework.context.ApplicationEvent;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.context.ApplicationEventPublisherAware;
Expand Down Expand Up @@ -619,20 +620,22 @@ public void refresh() throws BeansException, IllegalStateException {
finishRefresh();
}

catch (BeansException ex) {
catch (RuntimeException ex) {
if (logger.isWarnEnabled()) {
logger.warn("Exception encountered during context initialization - " +
"cancelling refresh attempt: " + ex);
}
BeansException beansException = (ex instanceof BeansException bex) ? bex
: new ApplicationContextException(ex.getMessage(), ex);

// Destroy already created singletons to avoid dangling resources.
destroyBeans();

// Reset 'active' flag.
cancelRefresh(ex);
cancelRefresh(beansException);

// Propagate exception to caller.
throw ex;
throw beansException;
}

finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,11 @@
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;
Expand Down Expand Up @@ -59,6 +63,7 @@
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;
Expand Down Expand Up @@ -302,6 +307,39 @@ private void assertGetResourceSemantics(ResourceLoader resourceLoader, Class<? e
.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(BeansException.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();
Expand Down Expand Up @@ -646,6 +684,29 @@ public void setCounter(Integer counter) {
}
}

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<T> extends AbstractFactoryBean<T> {

Expand Down

0 comments on commit caebff1

Please sign in to comment.