activemq-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cshan...@apache.org
Subject activemq git commit: https://issues.apache.org/jira/browse/AMQ-5896
Date Fri, 31 Jul 2015 11:59:35 GMT
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) <christopher.l.shannon@gmail.com>
Authored: Thu Jul 30 19:37:21 2015 +0000
Committer: Christopher L. Shannon (cshannon) <christopher.l.shannon@gmail.com>
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");


Mime
View raw message