Skip to content

Commit

Permalink
Do not change availability on close unless context is active
Browse files Browse the repository at this point in the history
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
  • Loading branch information
wilkinsona committed May 27, 2020
1 parent b5673db commit f17f125
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,9 @@ protected HttpHandler getHttpHandler() {

@Override
protected void doClose() {
AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC);
if (this.isActive()) {
AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC);
}
super.doClose();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ protected void onRefresh() {

@Override
protected void doClose() {
AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC);
if (this.isActive()) {
AvailabilityChangeEvent.publish(this, ReadinessState.REFUSING_TRAFFIC);
}
super.doClose();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
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;
Expand Down Expand Up @@ -141,6 +142,18 @@ void whenContextIsClosedThenApplicationAvailabilityChangesToRefusingTraffic() {
.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();
Expand Down Expand Up @@ -186,4 +199,12 @@ List<ApplicationEvent> receivedEvents() {

}

static class RefreshFailure {

RefreshFailure() {
throw new RuntimeException("Fail refresh");
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,17 @@ void applicationIsUnreadyDuringShutdown() {
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();
Expand Down Expand Up @@ -531,4 +542,12 @@ ServletRequest getRequest() {

}

static class RefreshFailure {

RefreshFailure() {
throw new RuntimeException("Fail refresh");
}

}

}

0 comments on commit f17f125

Please sign in to comment.