From f17f1255a46b46a36f9349b1a05505a95659a486 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 27 May 2020 14:43:12 +0100 Subject: [PATCH] Do not change availability on close unless context is active Previously, an AvailabilityChangeEvent was published when the servlet and reactive web server application contexts were closed, irrespective of whether or not the context was active. This caused problems when the context was not active due to a refresh failure as the event publication could then trigger bean creation and post-processing that relied upon beans that had been destroyed when cleaning up after the refresh failure. The most commonly seen symptom was a missing importRegistry bean that is required by ImportAwareBeanPostProcessor. This commit updates the two web server application contexts to only publish the availability change event if the context is active. Fixes gh-21588 --- .../ReactiveWebServerApplicationContext.java | 4 +++- .../ServletWebServerApplicationContext.java | 4 +++- ...ctiveWebServerApplicationContextTests.java | 21 +++++++++++++++++++ ...rvletWebServerApplicationContextTests.java | 19 +++++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java index 12bba932fe1..07f7f41c1df 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContext.java @@ -136,7 +136,9 @@ public class ReactiveWebServerApplicationContext extends GenericReactiveWebAppli @Override protected void doClose() { - AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC); + if (this.isActive()) { + AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC); + } super.doClose(); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java index b852fa05758..ed24c599934 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContext.java @@ -164,7 +164,9 @@ public class ServletWebServerApplicationContext extends GenericWebApplicationCon @Override protected void doClose() { - AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC); + if (this.isActive()) { + AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC); + } super.doClose(); } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContextTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContextTests.java index a217123c4fc..c16930732be 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContextTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/context/ReactiveWebServerApplicationContextTests.java @@ -24,6 +24,7 @@ import java.util.List; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.boot.availability.AvailabilityChangeEvent; import org.springframework.boot.availability.ReadinessState; @@ -141,6 +142,18 @@ class ReactiveWebServerApplicationContextTests { .isEqualTo(ReadinessState.REFUSING_TRAFFIC); } + @Test + void whenContextIsNotActiveThenCloseDoesNotChangeTheApplicationAvailability() { + addWebServerFactoryBean(); + addHttpHandlerBean(); + TestApplicationListener listener = new TestApplicationListener(); + this.context.addApplicationListener(listener); + this.context.registerBeanDefinition("refreshFailure", new RootBeanDefinition(RefreshFailure.class)); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(this.context::refresh); + this.context.close(); + assertThat(listener.receivedEvents()).isEmpty(); + } + @Test void whenTheContextIsRefreshedThenASubsequentRefreshAttemptWillFail() { addWebServerFactoryBean(); @@ -186,4 +199,12 @@ class ReactiveWebServerApplicationContextTests { } + static class RefreshFailure { + + RefreshFailure() { + throw new RuntimeException("Fail refresh"); + } + + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java index 26f30ad3588..82289d2cf29 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/context/ServletWebServerApplicationContextTests.java @@ -182,6 +182,17 @@ class ServletWebServerApplicationContextTests { ContextClosedEvent.class); } + @Test + void whenContextIsNotActiveThenCloseDoesNotChangeTheApplicationAvailability() { + addWebServerFactoryBean(); + TestApplicationListener listener = new TestApplicationListener(); + this.context.addApplicationListener(listener); + this.context.registerBeanDefinition("refreshFailure", new RootBeanDefinition(RefreshFailure.class)); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(this.context::refresh); + this.context.close(); + assertThat(listener.receivedEvents()).isEmpty(); + } + @Test void cannotSecondRefresh() { addWebServerFactoryBean(); @@ -531,4 +542,12 @@ class ServletWebServerApplicationContextTests { } + static class RefreshFailure { + + RefreshFailure() { + throw new RuntimeException("Fail refresh"); + } + + } + }