Skip to content

Commit

Permalink
https://issues.apache.org/jira/browse/AMQ-5896
Browse files Browse the repository at this point in the history 
Fixing a potential race condition with BrokerFacacdeSupport that could
cause an InstanceNotFoundException when getting a destination from
a RemoteJMXBrokerFacade instance
  • Loading branch information
@cshannon
cshannon committed Jul 31, 2015
1 parent b84413a commit ac3d088
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 13 deletions.
18 changes: 14 additions & 4 deletions activemq-web/src/main/java/org/apache/activemq/web/BrokerFacadeSupport.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand Down Expand Up @@ -127,9 +129,17 @@ protected DestinationViewMBean getDestinationByName(Collection<? extends Destina
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;
Expand Down
93 changes: 84 additions & 9 deletions activemq-web/src/test/java/org/apache/activemq/web/RemoteJMXBrokerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
*
Expand All @@ -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();
}

/**
Expand All @@ -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();
Expand All @@ -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<? 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");
Expand Down

0 comments on commit ac3d088

Please sign in to comment.