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
exceptions, not only BeansExceptions.

Closes gh-28878
  • Loading branch information
snicoll committed Oct 18, 2023
1 parent 46cc75b commit 9af239c
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -619,12 +619,11 @@ public void refresh() throws BeansException, IllegalStateException {
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();

Expand Down Expand Up @@ -974,7 +973,7 @@ protected void finishRefresh() {
* 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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -136,7 +136,7 @@ protected final void refreshBeanFactory() throws BeansException {
}

@Override
protected void cancelRefresh(BeansException ex) {
protected void cancelRefresh(Throwable ex) {
DefaultListableBeanFactory beanFactory = this.beanFactory;
if (beanFactory != null) {
beanFactory.setSerializationId(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ protected final void refreshBeanFactory() throws IllegalStateException {
}

@Override
protected void cancelRefresh(BeansException ex) {
protected void cancelRefresh(Throwable ex) {
this.beanFactory.setSerializationId(null);
super.cancelRefresh(ex);
}
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(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();
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 9af239c

Please sign in to comment.