You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cs...@apache.org on 2015/07/31 13:59:35 UTC
activemq git commit: https://issues.apache.org/jira/browse/AMQ-5896
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) <ch...@gmail.com>
Authored: Thu Jul 30 19:37:21 2015 +0000
Committer: Christopher L. Shannon (cshannon) <ch...@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");