Repository: activemq Updated Branches: refs/heads/master b84413a31 -> ac3d08864
https://issues.apache.org/jira/browse/AMQ-5896 Fixing a potential race condition with BrokerFacacdeSupport that could cause an InstanceNotFoundException when getting a destination from a RemoteJMXBrokerFacade instance Project: http://git-wip-us.apache.org/repos/asf/activemq/repo Commit: http://git-wip-us.apache.org/repos/asf/activemq/commit/ac3d0886 Tree: http://git-wip-us.apache.org/repos/asf/activemq/tree/ac3d0886 Diff: http://git-wip-us.apache.org/repos/asf/activemq/diff/ac3d0886 Branch: refs/heads/master Commit: ac3d088647cacfe298a61ba5b9f1b3941249142d Parents: b84413a Author: Christopher L. Shannon (cshannon) <[email protected]> Authored: Thu Jul 30 19:37:21 2015 +0000 Committer: Christopher L. Shannon (cshannon) <[email protected]> Committed: Fri Jul 31 11:56:40 2015 +0000 ---------------------------------------------------------------------- .../activemq/web/BrokerFacadeSupport.java | 18 +++- .../activemq/web/RemoteJMXBrokerTest.java | 93 ++++++++++++++++++-- 2 files changed, 98 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/activemq/blob/ac3d0886/activemq-web/src/main/java/org/apache/activemq/web/BrokerFacadeSupport.java ---------------------------------------------------------------------- diff --git a/activemq-web/src/main/java/org/apache/activemq/web/BrokerFacadeSupport.java b/activemq-web/src/main/java/org/apache/activemq/web/BrokerFacadeSupport.java index 69e8696..fedc6f2 100644 --- a/activemq-web/src/main/java/org/apache/activemq/web/BrokerFacadeSupport.java +++ b/activemq-web/src/main/java/org/apache/activemq/web/BrokerFacadeSupport.java @@ -23,6 +23,7 @@ import java.util.Iterator; import java.util.List; import java.util.Set; +import javax.management.InstanceNotFoundException; import javax.management.ObjectName; import javax.management.QueryExp; import javax.management.openmbean.CompositeData; @@ -37,10 +38,11 @@ import org.apache.activemq.broker.jmx.JobSchedulerViewMBean; import org.apache.activemq.broker.jmx.ManagementContext; import org.apache.activemq.broker.jmx.NetworkBridgeViewMBean; import org.apache.activemq.broker.jmx.NetworkConnectorViewMBean; +import org.apache.activemq.broker.jmx.ProducerViewMBean; import org.apache.activemq.broker.jmx.QueueViewMBean; import org.apache.activemq.broker.jmx.SubscriptionViewMBean; -import org.apache.activemq.broker.jmx.ProducerViewMBean; import org.apache.activemq.broker.jmx.TopicViewMBean; +import org.apache.commons.lang.exception.ExceptionUtils; import org.springframework.util.StringUtils; /** @@ -127,9 +129,17 @@ public abstract class BrokerFacadeSupport implements BrokerFacade { String name) { Iterator<? extends DestinationViewMBean> iter = collection.iterator(); while (iter.hasNext()) { - DestinationViewMBean destinationViewMBean = iter.next(); - if (name.equals(destinationViewMBean.getName())) { - return destinationViewMBean; + try { + DestinationViewMBean destinationViewMBean = iter.next(); + if (name.equals(destinationViewMBean.getName())) { + return destinationViewMBean; + } + } catch (Exception ex) { + Class<InstanceNotFoundException> infe = InstanceNotFoundException.class; + if (!infe.isInstance(ex) && !infe.isInstance(ExceptionUtils.getRootCause(ex))) { + // Only throw if not an expected InstanceNotFoundException exception + throw ex; + } } } return null; http://git-wip-us.apache.org/repos/asf/activemq/blob/ac3d0886/activemq-web/src/test/java/org/apache/activemq/web/RemoteJMXBrokerTest.java ---------------------------------------------------------------------- diff --git a/activemq-web/src/test/java/org/apache/activemq/web/RemoteJMXBrokerTest.java b/activemq-web/src/test/java/org/apache/activemq/web/RemoteJMXBrokerTest.java index bb3b8c7..3a4e570 100644 --- a/activemq-web/src/test/java/org/apache/activemq/web/RemoteJMXBrokerTest.java +++ b/activemq-web/src/test/java/org/apache/activemq/web/RemoteJMXBrokerTest.java @@ -16,20 +16,29 @@ */ package org.apache.activemq.web; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +import java.lang.reflect.Field; +import java.util.Collection; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import javax.management.ObjectName; +import javax.management.remote.JMXConnectorServer; + import org.apache.activemq.broker.BrokerFactory; import org.apache.activemq.broker.BrokerService; +import org.apache.activemq.broker.jmx.DestinationViewMBean; import org.apache.activemq.broker.jmx.ManagementContext; +import org.apache.activemq.command.ActiveMQQueue; import org.apache.activemq.web.config.SystemPropertiesConfiguration; +import org.junit.After; import org.junit.Before; import org.junit.Test; -import javax.management.ObjectName; -import javax.management.remote.JMXConnectorServer; -import java.lang.reflect.Field; -import java.util.Set; - -import static org.junit.Assert.assertEquals; - /** * @author <a href="http://www.christianposta.com/blog";>Christian Posta</a> * @@ -50,6 +59,15 @@ public class RemoteJMXBrokerTest { brokerService.start(); brokerService.waitUntilStarted(); + String jmxUri = getJmxUri(); + System.setProperty("webconsole.jmx.url", jmxUri); + + } + + @After + public void after() throws Exception { + brokerService.stop(); + brokerService.waitUntilStopped(); } /** @@ -60,8 +78,6 @@ public class RemoteJMXBrokerTest { */ @Test public void testConnectRemoteBrokerFacade() throws Exception { - String jmxUri = getJmxUri(); - System.setProperty("webconsole.jmx.url", jmxUri); RemoteJMXBrokerFacade brokerFacade = new RemoteJMXBrokerFacade(); SystemPropertiesConfiguration configuration = new SystemPropertiesConfiguration(); @@ -75,6 +91,65 @@ public class RemoteJMXBrokerTest { } + /** + * Before AMQ-5896 there was the possibility of an InstanceNotFoundException when + * brokerFacade.getQueue if a destination was deleted after the initial list was looked + * up but before iterating over the list to find the right destination by name. + * + */ + @Test(timeout=10000) + public void testGetDestinationRaceCondition() throws Exception { + final CountDownLatch getQueuesLatch = new CountDownLatch(1); + final CountDownLatch destDeletionLatch = new CountDownLatch(1); + // Adding a pause so we can test the case where the destination is + // deleted in between calling getQueues() and iterating over the list + //and calling getName() on the DestinationViewMBean + // See AMQ-5896 + RemoteJMXBrokerFacade brokerFacade = new RemoteJMXBrokerFacade() { + @Override + protected DestinationViewMBean getDestinationByName( + Collection<? extends DestinationViewMBean> collection, + String name) { + try { + //we are done getting the queue collection so let thread know + //to remove destination + getQueuesLatch.countDown(); + //wait until other thread is done removing destination + destDeletionLatch.await(); + } catch (InterruptedException e) { + } + return super.getDestinationByName(collection, name); + } + + }; + + SystemPropertiesConfiguration configuration = new SystemPropertiesConfiguration(); + brokerFacade.setConfiguration(configuration); + //Create the destination + final ActiveMQQueue queue = new ActiveMQQueue("queue.test"); + brokerService.getDestination(queue); + + //after 1 second delete + ExecutorService service = Executors.newCachedThreadPool(); + service.submit(new Runnable() { + @Override + public void run() { + try { + //wait for confirmation that the queue list was obtained + getQueuesLatch.await(); + brokerService.removeDestination(queue); + //let original thread know destination was deleted + destDeletionLatch.countDown(); + } catch (Exception e) { + } + } + }); + + //Assert that the destination is now null because it was deleted in another thread + //during iteration + assertNull(brokerFacade.getQueue(queue.getPhysicalName())); + service.shutdown(); + } public String getJmxUri() throws NoSuchFieldException, IllegalAccessException { Field field = ManagementContext.class.getDeclaredField("connectorServer");
