Fix for:

https://issues.apache.org/activemq/browse/AMQ-1255


git-svn-id: https://svn.apache.org/repos/asf/activemq/trunk@647872 13f79535-47bb-0310-9956-ffa450edef68

activemq-core/src/main/java/org/apache/activemq/advisory/AdvisoryBroker.java

@@ -203,6 +203,7 @@ public class AdvisoryBroker extends BrokerFilter {
             }
             try {
                 next.removeDestination(context, AdvisorySupport.getProducerAdvisoryTopic(info.getDestination()), -1);
+            
             } catch (Exception expectedIfDestinationDidNotExistYet) {
             }
         }
@@ -221,10 +222,13 @@ public class AdvisoryBroker extends BrokerFilter {
         next.removeConsumer(context, info);
 
         // Don't advise advisory topics.
-        if (!AdvisorySupport.isAdvisoryTopic(info.getDestination())) {
+        ActiveMQDestination dest = info.getDestination();
-            ActiveMQTopic topic = AdvisorySupport.getConsumerAdvisoryTopic(info.getDestination());
+        if (!AdvisorySupport.isAdvisoryTopic(dest)) {
+            ActiveMQTopic topic = AdvisorySupport.getConsumerAdvisoryTopic(dest);
             consumers.remove(info.getConsumerId());
-            fireConsumerAdvisory(context,info.getDestination(), topic, info.createRemoveCommand());
+            if (!dest.isTemporary() || destinations.contains(dest)) {
+            	fireConsumerAdvisory(context,dest, topic, info.createRemoveCommand());
+            }
         }
     }
 
@@ -232,10 +236,13 @@ public class AdvisoryBroker extends BrokerFilter {
         next.removeProducer(context, info);
 
         // Don't advise advisory topics.
-        if (info.getDestination() != null && !AdvisorySupport.isAdvisoryTopic(info.getDestination())) {
+        ActiveMQDestination dest = info.getDestination();
-            ActiveMQTopic topic = AdvisorySupport.getProducerAdvisoryTopic(info.getDestination());
+        if (info.getDestination() != null && !AdvisorySupport.isAdvisoryTopic(dest)) {
+            ActiveMQTopic topic = AdvisorySupport.getProducerAdvisoryTopic(dest);
             producers.remove(info.getProducerId());
-            fireProducerAdvisory(context, info.getDestination(),topic, info.createRemoveCommand());
+            if (!dest.isTemporary() || destinations.contains(dest)) {
+                fireProducerAdvisory(context, dest,topic, info.createRemoveCommand());
+            }
         }
     }
 

activemq-core/src/main/java/org/apache/activemq/broker/region/AbstractRegion.java

@@ -16,7 +16,6 @@
  */
 package org.apache.activemq.broker.region;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -24,9 +23,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-
 import javax.jms.JMSException;
-
 import org.apache.activemq.broker.ConnectionContext;
 import org.apache.activemq.broker.ConsumerBrokerExchange;
 import org.apache.activemq.broker.DestinationAlreadyExistsException;
@@ -96,19 +93,22 @@ public abstract class AbstractRegion implements Region {
             context.setSecurityContext(SecurityContext.BROKER_SECURITY_CONTEXT);
             context.getBroker().addDestination(context, dest);
         }
-
+        synchronized (destinationsMutex) {
             for (Iterator<Destination> i = destinations.values().iterator(); i.hasNext();) {
                 Destination dest = i.next();
                 dest.start();
             }
         }
+    }
 
     public void stop() throws Exception {
         started = false;
+        synchronized (destinationsMutex) {
             for (Iterator<Destination> i = destinations.values().iterator(); i.hasNext();) {
                 Destination dest = i.next();
                 dest.stop();
             }
+        }
         destinations.clear();
     }
 
@@ -169,10 +169,10 @@ public abstract class AbstractRegion implements Region {
         }
 
         LOG.debug("Removing destination: " + destination);
+        
         synchronized (destinationsMutex) {
             Destination dest = destinations.remove(destination);
             if (dest != null) {
-
                 // timeout<0 or we timed out, we now force any remaining
                 // subscriptions to un-subscribe.
                 for (Iterator<Subscription> iter = subscriptions.values().iterator(); iter.hasNext();) {
@@ -181,7 +181,6 @@ public abstract class AbstractRegion implements Region {
                         dest.removeSubscription(context, sub);
                     }
                 }
-
                 destinationMap.removeAll(destination);
                 dispose(context,dest);
 
@@ -259,10 +258,13 @@ public abstract class AbstractRegion implements Region {
             // so everything after this point would be leaked.
 
             // Add the subscription to all the matching queues.
+            
+            synchronized(destinationsMutex) {
                 for (Iterator iter = destinationMap.get(info.getDestination()).iterator(); iter.hasNext();) {
                     Destination dest = (Destination)iter.next();
                     dest.addSubscription(context, sub);
                 }
+            }
 
             if (info.isBrowser()) {
                 ((QueueBrowserSubscription)sub).destinationsAdded();
@@ -286,7 +288,9 @@ public abstract class AbstractRegion implements Region {
      */
     protected Set<ActiveMQDestination> getInactiveDestinations() {
         Set<ActiveMQDestination> inactiveDests = destinationFactory.getDestinations();
+        synchronized (destinationsMutex) {
             inactiveDests.removeAll(destinations.keySet());
+        }
         return inactiveDests;
     }
 
@@ -298,11 +302,13 @@ public abstract class AbstractRegion implements Region {
         if (sub != null) {
 
             // remove the subscription from all the matching queues.
+            synchronized (destinationsMutex) {
                 for (Iterator iter = destinationMap.get(info.getDestination())
                         .iterator(); iter.hasNext();) {
                     Destination dest = (Destination) iter.next();
                     dest.removeSubscription(context, sub);
                 }
+            }
 
             destroySubscription(sub);
         }
@@ -396,11 +402,13 @@ public abstract class AbstractRegion implements Region {
             Subscription sub = iter.next();
             sub.gc();
         }
+        synchronized (destinationsMutex) {
             for (Iterator<Destination> iter = destinations.values().iterator(); iter.hasNext();) {
                 Destination dest = iter.next();
                 dest.gc();
             }
         }
+    }
 
     protected abstract Subscription createSubscription(ConnectionContext context, ConsumerInfo info) throws Exception;
 
@@ -417,11 +425,13 @@ public abstract class AbstractRegion implements Region {
     }
     
     public void addProducer(ConnectionContext context, ProducerInfo info) throws Exception{
+        synchronized (destinationsMutex) {
             for (Iterator iter = destinationMap.get(info.getDestination()).iterator(); iter.hasNext();) {
                 Destination dest = (Destination) iter.next();
                 dest.addProducer(context, info);
             }
         }
+    }
 
     /**
      * Removes a Producer.
@@ -429,11 +439,13 @@ public abstract class AbstractRegion implements Region {
      * @throws Exception TODO
      */
     public void removeProducer(ConnectionContext context, ProducerInfo info) throws Exception{
+        synchronized (destinationsMutex) {
             for (Iterator iter = destinationMap.get(info.getDestination()).iterator(); iter.hasNext();) {
                 Destination dest = (Destination)iter.next();
                 dest.removeProducer(context, info);
             }
         }
+    }
     
     protected void dispose(ConnectionContext context,Destination dest) throws Exception {
         dest.dispose(context);

activemq-core/src/main/java/org/apache/activemq/broker/region/TopicRegion.java

@@ -78,10 +78,12 @@ public class TopicRegion extends AbstractRegion {
                 if (hasDurableSubChanged(info, sub.getConsumerInfo())) {
                     // Remove the consumer first then add it.
                     durableSubscriptions.remove(key);
+                    synchronized (destinationsMutex) {
                         for (Iterator<Destination> iter = destinations.values().iterator(); iter.hasNext();) {
                             Topic topic = (Topic)iter.next();
                             topic.deleteSubscription(context, key);
                         }
+                    }
                     super.removeConsumer(context, sub.getConsumerInfo());
                     super.addConsumer(context, info);
                     sub = durableSubscriptions.get(key);
@@ -132,10 +134,12 @@ public class TopicRegion extends AbstractRegion {
         }
 
         durableSubscriptions.remove(key);
+        synchronized (destinationsMutex) {
             for (Iterator<Destination> iter = destinations.values().iterator(); iter.hasNext();) {
                 Topic topic = (Topic)iter.next();
                 topic.deleteSubscription(context, key);
             }
+        }
         super.removeConsumer(context, sub.getConsumerInfo());
     }