Updated Branches: refs/heads/master 7b995ad9a -> abba6b312
CAMEL-6576: Improved initializaiton logic of ManagementStrategy to avoid contention as well a potential NPE. Project: http://git-wip-us.apache.org/repos/asf/camel/repo Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/c6620dba Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/c6620dba Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/c6620dba Branch: refs/heads/master Commit: c6620dbace9ae1e15981f4c31b1a97b4c396cde6 Parents: 7b995ad Author: Claus Ibsen <davscl...@apache.org> Authored: Tue Jul 30 12:18:53 2013 +0200 Committer: Claus Ibsen <davscl...@apache.org> Committed: Tue Jul 30 16:27:59 2013 +0200 ---------------------------------------------------------------------- .../java/org/apache/camel/CamelContext.java | 10 ++++-- .../apache/camel/impl/DefaultCamelContext.java | 29 ++++++---------- .../management/DefaultManagementAgent.java | 3 ++ .../management/DefaultManagementStrategy.java | 24 ++++++++------ .../management/ManagedManagementStrategy.java | 6 ++++ .../management/ManagementStrategyFactory.java | 17 +++------- .../management/CamelContextDisableJmxTest.java | 35 ++++++++++++++++++++ .../xml/AbstractCamelContextFactoryBean.java | 3 -- 8 files changed, 80 insertions(+), 47 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/camel/blob/c6620dba/camel-core/src/main/java/org/apache/camel/CamelContext.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/CamelContext.java b/camel-core/src/main/java/org/apache/camel/CamelContext.java index 6db32ba..839b00d 100644 --- a/camel-core/src/main/java/org/apache/camel/CamelContext.java +++ b/camel-core/src/main/java/org/apache/camel/CamelContext.java @@ -18,7 +18,9 @@ package org.apache.camel; import java.io.IOException; import java.io.InputStream; -import java.util.*; +import java.util.Collection; +import java.util.List; +import java.util.Map; import java.util.Properties; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -1019,8 +1021,12 @@ public interface CamelContext extends SuspendableService, RuntimeConfiguration { /** * Disables using JMX as {@link org.apache.camel.spi.ManagementStrategy}. + * <p/> + * <b>Important:</b> This method must be called <b>before</b> the {@link CamelContext} is started. + * + * @throws IllegalStateException is thrown if the {@link CamelContext} is not in stopped state. */ - void disableJMX(); + void disableJMX() throws IllegalStateException; /** * Gets the inflight repository http://git-wip-us.apache.org/repos/asf/camel/blob/c6620dba/camel-core/src/main/java/org/apache/camel/impl/DefaultCamelContext.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/impl/DefaultCamelContext.java b/camel-core/src/main/java/org/apache/camel/impl/DefaultCamelContext.java index fdd238a..385a920 100644 --- a/camel-core/src/main/java/org/apache/camel/impl/DefaultCamelContext.java +++ b/camel-core/src/main/java/org/apache/camel/impl/DefaultCamelContext.java @@ -34,7 +34,6 @@ import java.util.Set; import java.util.TreeMap; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import javax.naming.Context; import javax.xml.bind.JAXBContext; @@ -74,6 +73,7 @@ import org.apache.camel.impl.converter.BaseTypeConverterRegistry; import org.apache.camel.impl.converter.DefaultTypeConverter; import org.apache.camel.impl.converter.LazyLoadingTypeConverter; import org.apache.camel.management.DefaultManagementMBeanAssembler; +import org.apache.camel.management.DefaultManagementStrategy; import org.apache.camel.management.JmxSystemPropertyKeys; import org.apache.camel.management.ManagementStrategyFactory; import org.apache.camel.model.Constants; @@ -167,7 +167,6 @@ public class DefaultCamelContext extends ServiceSupport implements ModelCamelCon private List<LifecycleStrategy> lifecycleStrategies = new ArrayList<LifecycleStrategy>(); private ManagementStrategy managementStrategy; private ManagementMBeanAssembler managementMBeanAssembler; - private final AtomicBoolean managementStrategyInitialized = new AtomicBoolean(false); private final List<RouteDefinition> routeDefinitions = new ArrayList<RouteDefinition>(); private List<InterceptStrategy> interceptStrategies = new ArrayList<InterceptStrategy>(); @@ -242,6 +241,10 @@ public class DefaultCamelContext extends ServiceSupport implements ModelCamelCon packageScanClassResolver = new DefaultPackageScanClassResolver(); } + // setup management strategy first since end users may use it to add event notifiers + // using the management strategy before the CamelContext has been started + this.managementStrategy = createManagementStrategy(); + Container.Instance.manage(this); } @@ -2436,26 +2439,11 @@ public class DefaultCamelContext extends ServiceSupport implements ModelCamelCon } public ManagementStrategy getManagementStrategy() { - synchronized (managementStrategyInitialized) { - if (!managementStrategyInitialized.get()) { - if (managementStrategyInitialized.compareAndSet(false, true)) { - managementStrategy = createManagementStrategy(); - } - } - } - return managementStrategy; } public void setManagementStrategy(ManagementStrategy managementStrategy) { - synchronized (managementStrategyInitialized) { - if (managementStrategyInitialized.get()) { - log.warn("Resetting ManagementStrategy for CamelContext: " + getName()); - } - - this.managementStrategy = managementStrategy; - managementStrategyInitialized.set(true); - } + this.managementStrategy = managementStrategy; } public InterceptStrategy getDefaultTracer() { @@ -2492,7 +2480,10 @@ public class DefaultCamelContext extends ServiceSupport implements ModelCamelCon } public void disableJMX() { - disableJMX = true; + if (isStarting() || isStarted()) { + throw new IllegalStateException("Disabling JMX can only be done when CamelContext has not been started"); + } + managementStrategy = new DefaultManagementStrategy(this); } public InflightRepository getInflightRepository() { http://git-wip-us.apache.org/repos/asf/camel/blob/c6620dba/camel-core/src/main/java/org/apache/camel/management/DefaultManagementAgent.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/management/DefaultManagementAgent.java b/camel-core/src/main/java/org/apache/camel/management/DefaultManagementAgent.java index d2e57fb..eeda9c9 100644 --- a/camel-core/src/main/java/org/apache/camel/management/DefaultManagementAgent.java +++ b/camel-core/src/main/java/org/apache/camel/management/DefaultManagementAgent.java @@ -265,6 +265,9 @@ public class DefaultManagementAgent extends ServiceSupport implements Management protected void doStart() throws Exception { ObjectHelper.notNull(camelContext, "CamelContext"); + // must add management lifecycle strategy + camelContext.getLifecycleStrategies().add(0, new DefaultManagementLifecycleStrategy(camelContext)); + // create mbean server if is has not be injected. if (server == null) { finalizeSettings(); http://git-wip-us.apache.org/repos/asf/camel/blob/c6620dba/camel-core/src/main/java/org/apache/camel/management/DefaultManagementStrategy.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/management/DefaultManagementStrategy.java b/camel-core/src/main/java/org/apache/camel/management/DefaultManagementStrategy.java index f71f9ba..63aa132 100644 --- a/camel-core/src/main/java/org/apache/camel/management/DefaultManagementStrategy.java +++ b/camel-core/src/main/java/org/apache/camel/management/DefaultManagementStrategy.java @@ -32,8 +32,11 @@ import org.apache.camel.spi.ManagementAgent; import org.apache.camel.spi.ManagementNamingStrategy; import org.apache.camel.spi.ManagementObjectStrategy; import org.apache.camel.spi.ManagementStrategy; +import org.apache.camel.support.ServiceSupport; import org.apache.camel.util.ObjectHelper; import org.apache.camel.util.ServiceHelper; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A default management strategy that does <b>not</b> manage. @@ -48,8 +51,9 @@ import org.apache.camel.util.ServiceHelper; * @see ManagedManagementStrategy * @version */ -public class DefaultManagementStrategy implements ManagementStrategy, CamelContextAware { +public class DefaultManagementStrategy extends ServiceSupport implements ManagementStrategy, CamelContextAware { + private static final transient Logger LOG = LoggerFactory.getLogger(DefaultManagementStrategy.class); private List<EventNotifier> eventNotifiers = new CopyOnWriteArrayList<EventNotifier>(); private EventFactory eventFactory = new DefaultEventFactory(); private ManagementNamingStrategy managementNamingStrategy; @@ -198,7 +202,12 @@ public class DefaultManagementStrategy implements ManagementStrategy, CamelConte this.loadStatisticsEnabled = loadStatisticsEnabled; } - public void start() throws Exception { + protected void doStart() throws Exception { + LOG.info("JMX is disabled"); + doStartManagementStrategy(); + } + + protected void doStartManagementStrategy() throws Exception { ObjectHelper.notNull(camelContext, "CamelContext"); if (eventNotifiers != null) { @@ -215,7 +224,7 @@ public class DefaultManagementStrategy implements ManagementStrategy, CamelConte } if (managementAgent != null) { - managementAgent.start(); + ServiceHelper.startService(managementAgent); // set the naming strategy using the domain name from the agent if (managementNamingStrategy == null) { setManagementNamingStrategy(new DefaultManagementNamingStrategy(managementAgent.getMBeanObjectDomainName())); @@ -226,13 +235,8 @@ public class DefaultManagementStrategy implements ManagementStrategy, CamelConte } } - public void stop() throws Exception { - if (managementAgent != null) { - managementAgent.stop(); - } - if (eventNotifiers != null) { - ServiceHelper.stopServices(eventNotifiers); - } + protected void doStop() throws Exception { + ServiceHelper.stopServices(managementAgent, eventNotifiers); } } http://git-wip-us.apache.org/repos/asf/camel/blob/c6620dba/camel-core/src/main/java/org/apache/camel/management/ManagedManagementStrategy.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/management/ManagedManagementStrategy.java b/camel-core/src/main/java/org/apache/camel/management/ManagedManagementStrategy.java index c16e5c1..f89ffb4 100644 --- a/camel-core/src/main/java/org/apache/camel/management/ManagedManagementStrategy.java +++ b/camel-core/src/main/java/org/apache/camel/management/ManagedManagementStrategy.java @@ -182,4 +182,10 @@ public class ManagedManagementStrategy extends DefaultManagementStrategy { return objectName; } + @Override + protected void doStart() throws Exception { + LOG.info("JMX is enabled"); + doStartManagementStrategy(); + } + } http://git-wip-us.apache.org/repos/asf/camel/blob/c6620dba/camel-core/src/main/java/org/apache/camel/management/ManagementStrategyFactory.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/management/ManagementStrategyFactory.java b/camel-core/src/main/java/org/apache/camel/management/ManagementStrategyFactory.java index 44b7602..50d55fa 100644 --- a/camel-core/src/main/java/org/apache/camel/management/ManagementStrategyFactory.java +++ b/camel-core/src/main/java/org/apache/camel/management/ManagementStrategyFactory.java @@ -18,7 +18,6 @@ package org.apache.camel.management; import org.apache.camel.CamelContext; import org.apache.camel.spi.ManagementStrategy; -import org.apache.camel.util.ServiceHelper; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -29,27 +28,19 @@ public class ManagementStrategyFactory { private final transient Logger log = LoggerFactory.getLogger(getClass()); public ManagementStrategy create(CamelContext context, boolean disableJMX) { - ManagementStrategy answer = null; + ManagementStrategy answer; if (disableJMX || Boolean.getBoolean(JmxSystemPropertyKeys.DISABLED)) { - log.info("JMX is disabled."); + answer = new DefaultManagementStrategy(context); } else { try { answer = new ManagedManagementStrategy(context, new DefaultManagementAgent(context)); - // must start it to ensure JMX works and can load needed Spring JARs - ServiceHelper.startService(answer); - // prefer to have it at first strategy - context.getLifecycleStrategies().add(0, new DefaultManagementLifecycleStrategy(context)); - log.info("JMX enabled."); } catch (Exception e) { - answer = null; log.warn("Cannot create JMX lifecycle strategy. Will fallback and disable JMX.", e); + answer = new DefaultManagementStrategy(context); } } - - if (answer == null) { - answer = new DefaultManagementStrategy(context); - } return answer; } + } http://git-wip-us.apache.org/repos/asf/camel/blob/c6620dba/camel-core/src/test/java/org/apache/camel/management/CamelContextDisableJmxTest.java ---------------------------------------------------------------------- diff --git a/camel-core/src/test/java/org/apache/camel/management/CamelContextDisableJmxTest.java b/camel-core/src/test/java/org/apache/camel/management/CamelContextDisableJmxTest.java new file mode 100644 index 0000000..2946e3c --- /dev/null +++ b/camel-core/src/test/java/org/apache/camel/management/CamelContextDisableJmxTest.java @@ -0,0 +1,35 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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.apache.camel.management; + +import junit.framework.TestCase; +import org.apache.camel.CamelContext; +import org.apache.camel.impl.DefaultCamelContext; + +public class CamelContextDisableJmxTest extends TestCase { + + public void testDisableJmx() throws Exception { + CamelContext context = new DefaultCamelContext(); + context.disableJMX(); + context.start(); + + // JMX should be disabled and therefore not a ManagedManagementStrategy instance + assertFalse(context.getManagementStrategy() instanceof ManagedManagementStrategy); + + context.stop(); + } +} http://git-wip-us.apache.org/repos/asf/camel/blob/c6620dba/components/camel-core-xml/src/main/java/org/apache/camel/core/xml/AbstractCamelContextFactoryBean.java ---------------------------------------------------------------------- diff --git a/components/camel-core-xml/src/main/java/org/apache/camel/core/xml/AbstractCamelContextFactoryBean.java b/components/camel-core-xml/src/main/java/org/apache/camel/core/xml/AbstractCamelContextFactoryBean.java index 05e6568..76b0c69 100644 --- a/components/camel-core-xml/src/main/java/org/apache/camel/core/xml/AbstractCamelContextFactoryBean.java +++ b/components/camel-core-xml/src/main/java/org/apache/camel/core/xml/AbstractCamelContextFactoryBean.java @@ -370,9 +370,6 @@ public abstract class AbstractCamelContextFactoryBean<T extends ModelCamelContex ManagementStrategy managementStrategy = new ManagedManagementStrategy(getContext(), agent); getContext().setManagementStrategy(managementStrategy); - // clear the existing lifecycle strategies define by the DefaultCamelContext constructor - getContext().getLifecycleStrategies().clear(); - getContext().addLifecycleStrategy(new DefaultManagementLifecycleStrategy(getContext())); // set additional configuration from camelJMXAgent boolean onlyId = agent.getOnlyRegisterProcessorWithCustomId() != null && agent.getOnlyRegisterProcessorWithCustomId(); getContext().getManagementStrategy().onlyManageProcessorWithCustomId(onlyId);
