Updated Branches: refs/heads/camel-2.10.x 44905ef4d -> 650d8e262 refs/heads/camel-2.11.x bae0c7e1f -> e2b2f6cd0
CAMEL-6724: RegistryBean should avoid synchronization in getBean Project: http://git-wip-us.apache.org/repos/asf/camel/repo Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/e2b2f6cd Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/e2b2f6cd Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/e2b2f6cd Branch: refs/heads/camel-2.11.x Commit: e2b2f6cd0fe7e6144340ee9c192eb602bc88ee49 Parents: bae0c7e Author: Claus Ibsen <davscl...@apache.org> Authored: Tue Sep 10 17:13:20 2013 +0200 Committer: Claus Ibsen <davscl...@apache.org> Committed: Tue Sep 10 17:18:20 2013 +0200 ---------------------------------------------------------------------- .../apache/camel/component/bean/BeanHolder.java | 24 ++++++++ .../camel/component/bean/BeanProcessor.java | 7 ++- .../component/bean/ConstantBeanHolder.java | 4 ++ .../component/bean/ConstantTypeBeanHolder.java | 4 ++ .../camel/component/bean/RegistryBean.java | 61 +++++++++++++------- 5 files changed, 78 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/camel/blob/e2b2f6cd/camel-core/src/main/java/org/apache/camel/component/bean/BeanHolder.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/component/bean/BeanHolder.java b/camel-core/src/main/java/org/apache/camel/component/bean/BeanHolder.java index dc52026..7f26df3 100644 --- a/camel-core/src/main/java/org/apache/camel/component/bean/BeanHolder.java +++ b/camel-core/src/main/java/org/apache/camel/component/bean/BeanHolder.java @@ -26,9 +26,33 @@ import org.apache.camel.Processor; */ public interface BeanHolder { + /** + * Gets the bean. + * + * @throws NoSuchBeanException is thrown if the bean cannot be found. + */ Object getBean() throws NoSuchBeanException; + /** + * Gets a {@link Processor} for this bean, if supported. + * + * @return the {@link Processor}, or <tt>null</tt> if not supported. + */ Processor getProcessor(); + /** + * Gets bean info for the bean. + */ BeanInfo getBeanInfo(); + + /** + * Gets bean info for the given bean. + * <p/> + * This implementation allows a thread safe usage for {@link BeanHolder} implementations + * such as the {@link RegistryBean}. + * + * @param bean the bean + * @return <tt>null</tt> if not supported, then use {@link #getBeanInfo()} instead. + */ + BeanInfo getBeanInfo(Object bean); } http://git-wip-us.apache.org/repos/asf/camel/blob/e2b2f6cd/camel-core/src/main/java/org/apache/camel/component/bean/BeanProcessor.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/component/bean/BeanProcessor.java b/camel-core/src/main/java/org/apache/camel/component/bean/BeanProcessor.java index ea36799..06a9af1 100644 --- a/camel-core/src/main/java/org/apache/camel/component/bean/BeanProcessor.java +++ b/camel-core/src/main/java/org/apache/camel/component/bean/BeanProcessor.java @@ -75,7 +75,12 @@ public class BeanProcessor extends ServiceSupport implements AsyncProcessor { BeanInfo beanInfo; try { bean = beanHolder.getBean(); - beanInfo = beanHolder.getBeanInfo(); + // get bean info for this bean instance (to avoid thread issue) + beanInfo = beanHolder.getBeanInfo(bean); + if (beanInfo == null) { + // fallback and use old way + beanInfo = beanHolder.getBeanInfo(); + } } catch (Throwable e) { exchange.setException(e); callback.done(true); http://git-wip-us.apache.org/repos/asf/camel/blob/e2b2f6cd/camel-core/src/main/java/org/apache/camel/component/bean/ConstantBeanHolder.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/component/bean/ConstantBeanHolder.java b/camel-core/src/main/java/org/apache/camel/component/bean/ConstantBeanHolder.java index ddaa966..2cac840 100644 --- a/camel-core/src/main/java/org/apache/camel/component/bean/ConstantBeanHolder.java +++ b/camel-core/src/main/java/org/apache/camel/component/bean/ConstantBeanHolder.java @@ -65,4 +65,8 @@ public class ConstantBeanHolder implements BeanHolder { public BeanInfo getBeanInfo() { return beanInfo; } + + public BeanInfo getBeanInfo(Object bean) { + return null; + } } http://git-wip-us.apache.org/repos/asf/camel/blob/e2b2f6cd/camel-core/src/main/java/org/apache/camel/component/bean/ConstantTypeBeanHolder.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/component/bean/ConstantTypeBeanHolder.java b/camel-core/src/main/java/org/apache/camel/component/bean/ConstantTypeBeanHolder.java index 9fcb44e..515272b 100644 --- a/camel-core/src/main/java/org/apache/camel/component/bean/ConstantTypeBeanHolder.java +++ b/camel-core/src/main/java/org/apache/camel/component/bean/ConstantTypeBeanHolder.java @@ -62,6 +62,10 @@ public class ConstantTypeBeanHolder implements BeanTypeHolder { return beanInfo; } + public BeanInfo getBeanInfo(Object bean) { + return null; + } + public Class<?> getType() { return type; } http://git-wip-us.apache.org/repos/asf/camel/blob/e2b2f6cd/camel-core/src/main/java/org/apache/camel/component/bean/RegistryBean.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/component/bean/RegistryBean.java b/camel-core/src/main/java/org/apache/camel/component/bean/RegistryBean.java index ef5ce88..0c6e774 100644 --- a/camel-core/src/main/java/org/apache/camel/component/bean/RegistryBean.java +++ b/camel-core/src/main/java/org/apache/camel/component/bean/RegistryBean.java @@ -21,7 +21,6 @@ import org.apache.camel.NoSuchBeanException; import org.apache.camel.Processor; import org.apache.camel.spi.Registry; import org.apache.camel.util.CamelContextHelper; -import org.apache.camel.util.ObjectHelper; /** * An implementation of a {@link BeanHolder} which will look up a bean from the registry and act as a cache of its metadata @@ -29,12 +28,13 @@ import org.apache.camel.util.ObjectHelper; * @version */ public class RegistryBean implements BeanHolder { + private final Object lock = new Object(); private final CamelContext context; private final String name; private final Registry registry; - private Processor processor; - private BeanInfo beanInfo; - private Object bean; + private volatile Processor processor; + private volatile BeanInfo beanInfo; + private volatile Object bean; private ParameterMappingStrategy parameterMappingStrategy; public RegistryBean(CamelContext context, String name) { @@ -55,34 +55,45 @@ public class RegistryBean implements BeanHolder { } public ConstantBeanHolder createCacheHolder() throws Exception { - return new ConstantBeanHolder(getBean(), getBeanInfo()); + Object bean = getBean(); + BeanInfo info = createBeanInfo(bean); + return new ConstantBeanHolder(bean, info); } - public synchronized Object getBean() throws NoSuchBeanException { + public Object getBean() throws NoSuchBeanException { + // must always lookup bean first Object value = lookupBean(); - if (value == null) { - // maybe its a class - value = context.getClassResolver().resolveClass(name); - if (value == null) { - // no its not a class then we cannot find the bean - throw new NoSuchBeanException(name); + + if (value != null) { + // could be a class then create an instance of it + if (value instanceof Class) { + // bean is a class so create an instance of it + value = context.getInjector().newInstance((Class<?>)value); } + bean = value; + return value; } - if (value != bean) { - if (!ObjectHelper.equal(ObjectHelper.type(bean), ObjectHelper.type(value))) { - beanInfo = null; + + // okay bean is not in registry, so try to resolve if its a class name and create a shared instance + synchronized (lock) { + if (bean != null) { + return bean; } - bean = value; - processor = null; + // maybe its a class + bean = context.getClassResolver().resolveClass(name); + if (bean == null) { + // no its not a class then we cannot find the bean + throw new NoSuchBeanException(name); + } // could be a class then create an instance of it if (bean instanceof Class) { // bean is a class so create an instance of it bean = context.getInjector().newInstance((Class<?>)bean); - value = bean; } } - return value; + + return bean; } public Processor getProcessor() { @@ -94,11 +105,19 @@ public class RegistryBean implements BeanHolder { public BeanInfo getBeanInfo() { if (beanInfo == null && bean != null) { - this.beanInfo = createBeanInfo(); + this.beanInfo = createBeanInfo(bean); } return beanInfo; } + public BeanInfo getBeanInfo(Object bean) { + if (this.bean == bean) { + return getBeanInfo(); + } else { + return createBeanInfo(bean); + } + } + public String getName() { return name; } @@ -124,7 +143,7 @@ public class RegistryBean implements BeanHolder { // Implementation methods //------------------------------------------------------------------------- - protected BeanInfo createBeanInfo() { + protected BeanInfo createBeanInfo(Object bean) { return new BeanInfo(context, bean.getClass(), getParameterMappingStrategy()); }