Skip to content

Commit 9af239c

Browse files
committed
Clean resources in case of unexpected exception
This commit updates AbstractApplicationContext#refresh to handle any exceptions, not only BeansExceptions. Closes gh-28878
1 parent 46cc75b commit 9af239c

File tree

4 files changed

+66
-6
lines changed

4 files changed

+66
-6
lines changed

spring-context/src/main/java/org/springframework/context/support/AbstractApplicationContext.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -619,12 +619,11 @@ public void refresh() throws BeansException, IllegalStateException {
619619
finishRefresh();
620620
}
621621

622-
catch (BeansException ex) {
622+
catch (RuntimeException | Error ex ) {
623623
if (logger.isWarnEnabled()) {
624624
logger.warn("Exception encountered during context initialization - " +
625625
"cancelling refresh attempt: " + ex);
626626
}
627-
628627
// Destroy already created singletons to avoid dangling resources.
629628
destroyBeans();
630629

@@ -974,7 +973,7 @@ protected void finishRefresh() {
974973
* after an exception got thrown.
975974
* @param ex the exception that led to the cancellation
976975
*/
977-
protected void cancelRefresh(BeansException ex) {
976+
protected void cancelRefresh(Throwable ex) {
978977
this.active.set(false);
979978

980979
// Reset common introspection caches in Spring's core infrastructure.

spring-context/src/main/java/org/springframework/context/support/AbstractRefreshableApplicationContext.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2020 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -136,7 +136,7 @@ protected final void refreshBeanFactory() throws BeansException {
136136
}
137137

138138
@Override
139-
protected void cancelRefresh(BeansException ex) {
139+
protected void cancelRefresh(Throwable ex) {
140140
DefaultListableBeanFactory beanFactory = this.beanFactory;
141141
if (beanFactory != null) {
142142
beanFactory.setSerializationId(null);

spring-context/src/main/java/org/springframework/context/support/GenericApplicationContext.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ protected final void refreshBeanFactory() throws IllegalStateException {
297297
}
298298

299299
@Override
300-
protected void cancelRefresh(BeansException ex) {
300+
protected void cancelRefresh(Throwable ex) {
301301
this.beanFactory.setSerializationId(null);
302302
super.cancelRefresh(ex);
303303
}

spring-context/src/test/java/org/springframework/context/support/GenericApplicationContextTests.java

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@
2929
import org.springframework.aot.hint.RuntimeHints;
3030
import org.springframework.aot.hint.predicate.RuntimeHintsPredicates;
3131
import org.springframework.beans.BeansException;
32+
import org.springframework.beans.factory.BeanCreationException;
33+
import org.springframework.beans.factory.DisposableBean;
34+
import org.springframework.beans.factory.InitializingBean;
3235
import org.springframework.beans.factory.NoUniqueBeanDefinitionException;
36+
import org.springframework.beans.factory.SmartInitializingSingleton;
3337
import org.springframework.beans.factory.config.AbstractFactoryBean;
3438
import org.springframework.beans.factory.config.BeanDefinition;
3539
import org.springframework.beans.factory.config.BeanFactoryPostProcessor;
@@ -59,6 +63,7 @@
5963
import static org.assertj.core.api.Assertions.assertThat;
6064
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
6165
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
66+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
6267
import static org.assertj.core.api.InstanceOfAssertFactories.type;
6368
import static org.mockito.ArgumentMatchers.any;
6469
import static org.mockito.ArgumentMatchers.eq;
@@ -302,6 +307,39 @@ private void assertGetResourceSemantics(ResourceLoader resourceLoader, Class<? e
302307
.isEqualTo("pong:foo");
303308
}
304309

310+
@Test
311+
void refreshWithRuntimeFailureOnBeanCreationDisposeExistingBeans() {
312+
BeanE one = new BeanE();
313+
context.registerBean("one", BeanE.class, () -> one);
314+
context.registerBean("two", BeanE.class, () -> new BeanE() {
315+
@Override
316+
public void afterPropertiesSet() {
317+
throw new IllegalStateException("Expected");
318+
}
319+
});
320+
assertThatThrownBy(context::refresh).isInstanceOf(BeanCreationException.class)
321+
.hasMessageContaining("two");
322+
assertThat(one.initialized).isTrue();
323+
assertThat(one.destroyed).isTrue();
324+
}
325+
326+
@Test
327+
void refreshWithRuntimeFailureOnAfterSingletonInstantiatedDisposeExistingBeans() {
328+
BeanE one = new BeanE();
329+
BeanE two = new BeanE();
330+
context.registerBean("one", BeanE.class, () -> one);
331+
context.registerBean("two", BeanE.class, () -> two);
332+
context.registerBean("int", SmartInitializingSingleton.class, () -> () -> {
333+
throw new IllegalStateException("expected");
334+
});
335+
assertThatThrownBy(context::refresh).isInstanceOf(IllegalStateException.class)
336+
.hasMessageContaining("expected");
337+
assertThat(one.initialized).isTrue();
338+
assertThat(two.initialized).isTrue();
339+
assertThat(one.destroyed).isTrue();
340+
assertThat(two.destroyed).isTrue();
341+
}
342+
305343
@Test
306344
void refreshForAotSetsContextActive() {
307345
GenericApplicationContext context = new GenericApplicationContext();
@@ -646,6 +684,29 @@ public void setCounter(Integer counter) {
646684
}
647685
}
648686

687+
static class BeanE implements InitializingBean, DisposableBean {
688+
689+
private boolean initialized;
690+
691+
private boolean destroyed;
692+
693+
@Override
694+
public void afterPropertiesSet() throws Exception {
695+
if (initialized) {
696+
throw new IllegalStateException("AfterPropertiesSet called twice");
697+
}
698+
this.initialized = true;
699+
}
700+
701+
@Override
702+
public void destroy() throws Exception {
703+
if (destroyed) {
704+
throw new IllegalStateException("Destroy called twice");
705+
}
706+
this.destroyed = true;
707+
}
708+
}
709+
649710

650711
static class TestAotFactoryBean<T> extends AbstractFactoryBean<T> {
651712

0 commit comments

Comments
 (0)