Skip to content

Re: SPR-8941: Lifecycle processing ignores phases for circularly dependent SmartLifecycle beans #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Re: SPR-8941: Lifecycle processing ignores phases for circularly dependent SmartLifecycle beans #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions ....context/src/main/java/org/springframework/context/support/DefaultLifecycleProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ public class DefaultLifecycleProcessor implements LifecycleProcessor, BeanFactor

private volatile ConfigurableListableBeanFactory beanFactory;

private volatile boolean ignorePhaseOfDependencies = true;

/**
* Specify the maximum time allotted in milliseconds for the shutdown of
Expand All @@ -68,6 +69,21 @@ public void setTimeoutPerShutdownPhase(long timeoutPerShutdownPhase) {
this.timeoutPerShutdownPhase = timeoutPerShutdownPhase;
}

/**
* Specifies whether a lifecycle bean which other lifecycle beans depend on
* (either explicitly or via {@link DependsOn}) } should be started
* (stopped) before (after) its dependents, irrespective of their phase. The
* backwards-compatible default is true, a dependency bean is always started
* (stopped) before (after) the beans that depend on it even if its
* {@link Phased#getPhase()} method indicates that it should be started
* later (earlier). If false, lifecycle beans are never started or stopped
* in an order that disregards their phase.
*/
public void setIgnorePhaseOfDependencies( final boolean ignorePhaseOfDependencies )
{
this.ignorePhaseOfDependencies = ignorePhaseOfDependencies;
}

public void setBeanFactory(BeanFactory beanFactory) {
Assert.isInstanceOf(ConfigurableListableBeanFactory.class, beanFactory);
this.beanFactory = (ConfigurableListableBeanFactory) beanFactory;
Expand Down Expand Up @@ -154,9 +170,13 @@ private void startBeans(boolean autoStartupOnly) {
private void doStart(Map<String, ? extends Lifecycle> lifecycleBeans, String beanName, boolean autoStartupOnly) {
Lifecycle bean = lifecycleBeans.remove(beanName);
if (bean != null && !this.equals(bean)) {
String[] dependenciesForBean = this.beanFactory.getDependenciesForBean(beanName);
for (String dependency : dependenciesForBean) {
doStart(lifecycleBeans, dependency, autoStartupOnly);
final String[] dependencies = this.beanFactory.getDependenciesForBean(beanName);
final int currentPhase = getPhase( bean );
for (final String dependency : dependencies) {
final Lifecycle dependencyBean = lifecycleBeans.get( dependency );
if( ignorePhaseOfDependencies || dependencyBean == null || getPhase( dependencyBean ) <= currentPhase ) {
doStart(lifecycleBeans, dependency, autoStartupOnly);
}
}
if (!bean.isRunning() &&
(!autoStartupOnly || !(bean instanceof SmartLifecycle) || ((SmartLifecycle) bean).isAutoStartup())) {
Expand Down Expand Up @@ -209,9 +229,13 @@ private void doStop(Map<String, ? extends Lifecycle> lifecycleBeans, final Strin

Lifecycle bean = lifecycleBeans.remove(beanName);
if (bean != null) {
String[] dependentBeans = this.beanFactory.getDependentBeans(beanName);
for (String dependentBean : dependentBeans) {
doStop(lifecycleBeans, dependentBean, latch, countDownBeanNames);
final String[] dependents = this.beanFactory.getDependentBeans(beanName);
final int currentPhase = getPhase( bean );
for (final String dependent : dependents) {
final Lifecycle dependentBean = lifecycleBeans.get( dependent );
if( ignorePhaseOfDependencies || dependentBean == null || getPhase( dependentBean ) >= currentPhase ) {
doStop(lifecycleBeans, dependent, latch, countDownBeanNames);
}
}
try {
if (bean.isRunning()) {
Expand Down
57 changes: 47 additions & 10 deletions ...ext/src/test/java/org/springframework/context/support/DefaultLifecycleProcessorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,15 @@ public void mixedShutdown() throws Exception {

@Test
public void dependencyStartedFirstEvenIfItsPhaseIsHigher() throws Exception {
impactOfDependencyAndPhaseOnStartupOrder(true);
}

@Test
public void dependencyStartedLastIfItsPhaseIsHigher() throws Exception {
impactOfDependencyAndPhaseOnStartupOrder(false);
}

private void impactOfDependencyAndPhaseOnStartupOrder(final boolean ignorePhaseOfDependencies) {
CopyOnWriteArrayList<Lifecycle> startedBeans = new CopyOnWriteArrayList<Lifecycle>();
TestSmartLifecycleBean beanMin = TestSmartLifecycleBean.forStartupTests(Integer.MIN_VALUE, startedBeans);
TestSmartLifecycleBean bean2 = TestSmartLifecycleBean.forStartupTests(2, startedBeans);
Expand All @@ -369,23 +378,37 @@ public void dependencyStartedFirstEvenIfItsPhaseIsHigher() throws Exception {
context.getBeanFactory().registerSingleton("bean99", bean99);
context.getBeanFactory().registerSingleton("beanMax", beanMax);
context.getBeanFactory().registerDependentBean("bean99", "bean2");

final BeanDefinition beanDefinition = new RootBeanDefinition(DefaultLifecycleProcessor.class);
beanDefinition.getPropertyValues().addPropertyValue("ignorePhaseOfDependencies", ignorePhaseOfDependencies);
context.registerBeanDefinition("lifecycleProcessor", beanDefinition);

context.refresh();
assertTrue(beanMin.isRunning());
assertTrue(bean2.isRunning());
assertTrue(bean99.isRunning());
assertTrue(beanMax.isRunning());
assertEquals(4, startedBeans.size());
assertEquals(Integer.MIN_VALUE, getPhase(startedBeans.get(0)));
assertEquals(99, getPhase(startedBeans.get(1)));
assertEquals(bean99, startedBeans.get(1));
assertEquals(2, getPhase(startedBeans.get(2)));
assertEquals(bean2, startedBeans.get(2));
assertEquals(ignorePhaseOfDependencies ? 99 : 2, getPhase(startedBeans.get(1)));
assertEquals(ignorePhaseOfDependencies ? bean99 : bean2, startedBeans.get(1));
assertEquals(ignorePhaseOfDependencies ? 2 : 99, getPhase(startedBeans.get(2)));
assertEquals(ignorePhaseOfDependencies ? bean2 : bean99, startedBeans.get(2));
assertEquals(Integer.MAX_VALUE, getPhase(startedBeans.get(3)));
context.stop();
}

@Test
public void dependentShutdownFirstEvenIfItsPhaseIsLower() throws Exception {
impactOfDependencyAndPhaseOnShutdownOrder(true);
}

@Test
public void dependentShutdownLastIfItsPhaseIsLower() throws Exception {
impactOfDependencyAndPhaseOnShutdownOrder(false);
}

private void impactOfDependencyAndPhaseOnShutdownOrder(final boolean ignorePhaseOfDependencies) {
CopyOnWriteArrayList<Lifecycle> stoppedBeans = new CopyOnWriteArrayList<Lifecycle>();
TestSmartLifecycleBean beanMin = TestSmartLifecycleBean.forShutdownTests(Integer.MIN_VALUE, 100, stoppedBeans);
TestSmartLifecycleBean bean1 = TestSmartLifecycleBean.forShutdownTests(1, 200, stoppedBeans);
Expand All @@ -401,7 +424,12 @@ public void dependentShutdownFirstEvenIfItsPhaseIsLower() throws Exception {
context.getBeanFactory().registerSingleton("bean99", bean99);
context.getBeanFactory().registerSingleton("beanMax", beanMax);
context.getBeanFactory().registerDependentBean("bean99", "bean2");

final BeanDefinition beanDefinition = new RootBeanDefinition(DefaultLifecycleProcessor.class);
beanDefinition.getPropertyValues().addPropertyValue("ignorePhaseOfDependencies", ignorePhaseOfDependencies);
context.registerBeanDefinition("lifecycleProcessor", beanDefinition);
context.refresh();

assertTrue(beanMin.isRunning());
assertTrue(bean1.isRunning());
assertTrue(bean2.isRunning());
Expand All @@ -417,12 +445,21 @@ public void dependentShutdownFirstEvenIfItsPhaseIsLower() throws Exception {
assertFalse(beanMax.isRunning());
assertEquals(6, stoppedBeans.size());
assertEquals(Integer.MAX_VALUE, getPhase(stoppedBeans.get(0)));
assertEquals(2, getPhase(stoppedBeans.get(1)));
assertEquals(bean2, stoppedBeans.get(1));
assertEquals(99, getPhase(stoppedBeans.get(2)));
assertEquals(bean99, stoppedBeans.get(2));
assertEquals(7, getPhase(stoppedBeans.get(3)));
assertEquals(1, getPhase(stoppedBeans.get(4)));
if (ignorePhaseOfDependencies) {
assertEquals(2, getPhase(stoppedBeans.get(1)));
assertEquals(bean2, stoppedBeans.get(1));
assertEquals(99, getPhase(stoppedBeans.get(2)));
assertEquals(bean99, stoppedBeans.get(2));
assertEquals(7, getPhase(stoppedBeans.get(3)));
assertEquals(1, getPhase(stoppedBeans.get(4)));
} else {
assertEquals(99, getPhase(stoppedBeans.get(1)));
assertEquals(bean99, stoppedBeans.get(1));
assertEquals(7, getPhase(stoppedBeans.get(2)));
assertEquals(bean7, stoppedBeans.get(2));
assertEquals(2, getPhase(stoppedBeans.get(3)));
assertEquals(1, getPhase(stoppedBeans.get(4)));
}
assertEquals(Integer.MIN_VALUE, getPhase(stoppedBeans.get(5)));
}

Expand Down