diff --git a/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/GenericScope.java b/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/GenericScope.java index 2b02f11e..0e6617a1 100644 --- a/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/GenericScope.java +++ b/spring-cloud-context/src/main/java/org/springframework/cloud/context/scope/GenericScope.java @@ -154,8 +154,8 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, Disposable if (this.lifecycle == null) { this.lifecycle = new StandardBeanLifecycleDecorator(this.proxyTargetClass); } - BeanLifecycleWrapper value = this.cache.put(name, new BeanLifecycleWrapper(name, - objectFactory, this.lifecycle)); + BeanLifecycleWrapper value = this.cache.put(name, + new BeanLifecycleWrapper(name, objectFactory, this.lifecycle)); return value.getBean(); } @@ -198,7 +198,8 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, Disposable return parser.parseExpression(input); } catch (ParseException e) { - throw new IllegalArgumentException("Cannot parse expression: " + input, e); + throw new IllegalArgumentException("Cannot parse expression: " + input, + e); } } @@ -240,8 +241,9 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, Disposable } else { - logger.warn("BeanFactory was not a DefaultListableBeanFactory, so RefreshScope beans " - + "cannot be serialized reliably and passed to a remote JVM."); + logger.warn( + "BeanFactory was not a DefaultListableBeanFactory, so RefreshScope beans " + + "cannot be serialized reliably and passed to a remote JVM."); } } @@ -323,8 +325,12 @@ public class GenericScope implements Scope, BeanFactoryPostProcessor, Disposable @SuppressWarnings("unchecked") public Object getBean() { if (this.bean == null) { - this.bean = this.lifecycle.decorateBean(this.objectFactory.getObject(), - this.context); + synchronized (this.name) { + if (this.bean == null) { + this.bean = this.lifecycle.decorateBean( + this.objectFactory.getObject(), this.context); + } + } } return this.bean; } diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeConcurrencyTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeConcurrencyTests.java index 1aef5713..27d14d1e 100644 --- a/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeConcurrencyTests.java +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeConcurrencyTests.java @@ -15,10 +15,6 @@ */ package org.springframework.cloud.context.scope.refresh; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; - import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; @@ -37,19 +33,23 @@ import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfigurati import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration; +import org.springframework.cloud.context.config.annotation.RefreshScope; +import org.springframework.cloud.context.scope.refresh.RefreshScopeConcurrencyTests.TestConfiguration; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; import org.springframework.jmx.export.annotation.ManagedAttribute; import org.springframework.jmx.export.annotation.ManagedResource; -import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration; -import org.springframework.cloud.context.config.annotation.RefreshScope; -import org.springframework.cloud.context.scope.refresh.RefreshScopeConcurrencyTests.TestConfiguration; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.annotation.Repeat; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; -@SpringApplicationConfiguration(classes=TestConfiguration.class) +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +@SpringApplicationConfiguration(classes = TestConfiguration.class) @RunWith(SpringJUnit4ClassRunner.class) public class RefreshScopeConcurrencyTests { @@ -71,16 +71,18 @@ public class RefreshScopeConcurrencyTests { @DirtiesContext public void testConcurrentRefresh() throws Exception { - assertEquals("Hello scope!", service.getMessage()); - properties.setMessage("Foo"); - properties.setDelay(500); + assertEquals("Hello scope!", this.service.getMessage()); + this.properties.setMessage("Foo"); + this.properties.setDelay(500); final CountDownLatch latch = new CountDownLatch(1); - Future result = executor.submit(new Callable() { + Future result = this.executor.submit(new Callable() { + @Override public String call() throws Exception { logger.debug("Background started."); try { - return service.getMessage(); - } finally { + return RefreshScopeConcurrencyTests.this.service.getMessage(); + } + finally { latch.countDown(); logger.debug("Background done."); } @@ -88,11 +90,11 @@ public class RefreshScopeConcurrencyTests { }); assertTrue(latch.await(1500, TimeUnit.MILLISECONDS)); logger.info("Refreshing"); - scope.refreshAll(); - assertEquals("Foo", service.getMessage()); + this.scope.refreshAll(); + assertEquals("Foo", this.service.getMessage()); /* - * This is the most important assertion: we don't want a null value because that means the bean was destroyed - * and not re-initialized before we accessed it. + * This is the most important assertion: we don't want a null value because that + * means the bean was destroyed and not re-initialized before we accessed it. */ assertNotNull(result.get()); assertEquals("Hello scope!", result.get()); @@ -104,7 +106,8 @@ public class RefreshScopeConcurrencyTests { } - public static class ExampleService implements Service, InitializingBean, DisposableBean { + public static class ExampleService + implements Service, InitializingBean, DisposableBean { private static Log logger = LogFactory.getLog(ExampleService.class); @@ -115,13 +118,15 @@ public class RefreshScopeConcurrencyTests { this.delay = delay; } + @Override public void afterPropertiesSet() throws Exception { - logger.debug("Initializing message: " + message); + logger.debug("Initializing message: " + this.message); } + @Override public void destroy() throws Exception { - logger.debug("Destroying message: " + message); - message = null; + logger.debug("Destroying message: " + this.message); + this.message = null; } public void setMessage(String message) { @@ -129,36 +134,39 @@ public class RefreshScopeConcurrencyTests { this.message = message; } + @Override public String getMessage() { - logger.debug("Getting message: " + message); + logger.debug("Getting message: " + this.message); try { - Thread.sleep(delay); - } catch (InterruptedException e) { + Thread.sleep(this.delay); + } + catch (InterruptedException e) { Thread.currentThread().interrupt(); } - logger.info("Returning message: " + message); - return message; + logger.info("Returning message: " + this.message); + return this.message; } } - + @Configuration @EnableConfigurationProperties(TestProperties.class) - @Import({RefreshAutoConfiguration.class, PropertyPlaceholderAutoConfiguration.class}) + @Import({ RefreshAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class }) protected static class TestConfiguration { - + @Autowired private TestProperties properties; - + @Bean @RefreshScope public ExampleService service() { ExampleService service = new ExampleService(); - service.setMessage(properties.getMessage()); - service.setDelay(properties.getDelay()); + service.setMessage(this.properties.getMessage()); + service.setDelay(this.properties.getDelay()); return service; } - + } @ConfigurationProperties @@ -166,20 +174,24 @@ public class RefreshScopeConcurrencyTests { protected static class TestProperties { private String message; private int delay; + @ManagedAttribute public String getMessage() { - return message; + return this.message; } + public void setMessage(String message) { this.message = message; } + @ManagedAttribute public int getDelay() { - return delay; + return this.delay; } + public void setDelay(int delay) { this.delay = delay; } } - + } diff --git a/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeScaleTests.java b/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeScaleTests.java new file mode 100644 index 00000000..0663eacd --- /dev/null +++ b/spring-cloud-context/src/test/java/org/springframework/cloud/context/scope/refresh/RefreshScopeScaleTests.java @@ -0,0 +1,195 @@ +/* + * Copyright 2006-2007 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.cloud.context.scope.refresh; + +import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.springframework.beans.factory.DisposableBean; +import org.springframework.beans.factory.InitializingBean; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.autoconfigure.PropertyPlaceholderAutoConfiguration; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.EnableConfigurationProperties; +import org.springframework.boot.test.SpringApplicationConfiguration; +import org.springframework.cloud.autoconfigure.RefreshAutoConfiguration; +import org.springframework.cloud.context.config.annotation.RefreshScope; +import org.springframework.cloud.context.scope.refresh.RefreshScopeScaleTests.TestConfiguration; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.jmx.export.annotation.ManagedAttribute; +import org.springframework.jmx.export.annotation.ManagedResource; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.annotation.Repeat; +import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +@SpringApplicationConfiguration(classes = TestConfiguration.class) +@RunWith(SpringJUnit4ClassRunner.class) +public class RefreshScopeScaleTests { + + private static Log logger = LogFactory.getLog(RefreshScopeScaleTests.class); + + private ExecutorService executor = Executors.newFixedThreadPool(8); + + @Autowired + private ExampleService service; + + @Autowired + private TestProperties properties; + + @Test + @Repeat(10) + @DirtiesContext + public void testConcurrentRefresh() throws Exception { + + // overload the thread pool and try to force Spring to create too many instances + int n = 80; + ExampleService.count = 0; + this.properties.setMessage("Foo"); + this.properties.setDelay(500); + final CountDownLatch latch = new CountDownLatch(n); + Future result = null; + for (int i = 0; i < n; i++) { + result = this.executor.submit(new Callable() { + @Override + public String call() throws Exception { + logger.debug("Background started."); + try { + return RefreshScopeScaleTests.this.service.getMessage(); + } + finally { + latch.countDown(); + logger.debug("Background done."); + } + } + }); + } + assertTrue(latch.await(15000, TimeUnit.MILLISECONDS)); + assertEquals("Foo", this.service.getMessage()); + assertNotNull(result.get()); + assertEquals("Foo", result.get()); + assertEquals(1, ExampleService.count); + } + + public static interface Service { + + String getMessage(); + + } + + public static class ExampleService + implements Service, InitializingBean, DisposableBean { + + private static Log logger = LogFactory.getLog(ExampleService.class); + + private String message = null; + private volatile long delay = 0; + private static volatile int count; + + public void setDelay(long delay) { + this.delay = delay; + } + + @Override + public void afterPropertiesSet() throws Exception { + ExampleService.count++; + try { + Thread.sleep(this.delay); + } + catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + logger.debug("Initializing message: " + this.message); + } + + @Override + public void destroy() throws Exception { + logger.debug("Destroying message: " + this.message); + this.message = null; + } + + public void setMessage(String message) { + logger.debug("Setting message: " + message); + this.message = message; + } + + @Override + public String getMessage() { + logger.info("Returning message: " + this.message); + return this.message; + } + + } + + @Configuration + @EnableConfigurationProperties(TestProperties.class) + @Import({ RefreshAutoConfiguration.class, + PropertyPlaceholderAutoConfiguration.class }) + protected static class TestConfiguration { + + @Autowired + private TestProperties properties; + + @Bean + @RefreshScope + public ExampleService service() { + ExampleService service = new ExampleService(); + service.setMessage(this.properties.getMessage()); + service.setDelay(this.properties.getDelay()); + return service; + } + + } + + @ConfigurationProperties + @ManagedResource + protected static class TestProperties { + private String message; + private int delay; + + @ManagedAttribute + public String getMessage() { + return this.message; + } + + public void setMessage(String message) { + this.message = message; + } + + @ManagedAttribute + public int getDelay() { + return this.delay; + } + + public void setDelay(int delay) { + this.delay = delay; + } + } + +}