From ac3d088647cacfe298a61ba5b9f1b3941249142d Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon (cshannon)" Date: Thu, 30 Jul 2015 19:37:21 +0000 Subject: [PATCH] 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 --- .../activemq/web/BrokerFacadeSupport.java | 18 +++- .../activemq/web/RemoteJMXBrokerTest.java | 93 +++++++++++++++++-- 2 files changed, 98 insertions(+), 13 deletions(-) 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 69e8696d13e..fedc6f23932 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.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.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 @@ protected DestinationViewMBean getDestinationByName(Collection 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 infe = InstanceNotFoundException.class; + if (!infe.isInstance(ex) && !infe.isInstance(ExceptionUtils.getRootCause(ex))) { + // Only throw if not an expected InstanceNotFoundException exception + throw ex; + } } } return null; 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 bb3b8c7eb48..3a4e570017f 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 Christian Posta * @@ -50,6 +59,15 @@ public void startUp() throws Exception { 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 void startUp() throws Exception { */ @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 void testConnectRemoteBrokerFacade() throws Exception { } + /** + * 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 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");