ARTEMIS-177 create/destroy subscription race
Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/454e5b66 Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/454e5b66 Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/454e5b66 Branch: refs/heads/master Commit: 454e5b66ff0863f32305c6eaaa618fda7b364e1f Parents: 86934c9 Author: jbertram <[email protected]> Authored: Mon Jan 4 14:13:23 2016 -0600 Committer: Clebert Suconic <[email protected]> Committed: Tue Jan 5 10:44:13 2016 -0500 ---------------------------------------------------------------------- .../postoffice/impl/SimpleAddressManager.java | 24 +-------- .../jms/tests/DurableSubscriptionTest.java | 57 ++++++++++++++++++++ 2 files changed, 58 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/454e5b66/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java ---------------------------------------------------------------------- diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java index 8bfe00d..0349f4f 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/SimpleAddressManager.java @@ -31,8 +31,6 @@ import org.apache.activemq.artemis.core.postoffice.BindingsFactory; import org.apache.activemq.artemis.core.server.ActiveMQMessageBundle; import org.apache.activemq.artemis.core.server.ActiveMQServerLogger; import org.apache.activemq.artemis.core.transaction.Transaction; -import org.apache.activemq.artemis.core.transaction.TransactionOperationAbstract; -import org.apache.activemq.artemis.utils.ConcurrentHashSet; /** * A simple address manager that maintains the addresses and bindings. @@ -49,8 +47,6 @@ public class SimpleAddressManager implements AddressManager { */ private final ConcurrentMap<SimpleString, Binding> nameMap = new ConcurrentHashMap<>(); - private final ConcurrentHashSet<SimpleString> pendingDeletes = new ConcurrentHashSet<>(); - private final BindingsFactory bindingsFactory; public SimpleAddressManager(final BindingsFactory bindingsFactory) { @@ -59,7 +55,7 @@ public class SimpleAddressManager implements AddressManager { @Override public boolean addBinding(final Binding binding) throws Exception { - if (nameMap.putIfAbsent(binding.getUniqueName(), binding) != null || pendingDeletes.contains(binding.getUniqueName())) { + if (nameMap.putIfAbsent(binding.getUniqueName(), binding) != null) { throw ActiveMQMessageBundle.BUNDLE.bindingAlreadyExists(binding); } @@ -78,24 +74,6 @@ public class SimpleAddressManager implements AddressManager { return null; } - if (tx != null) { - pendingDeletes.add(uniqueName); - tx.addOperation(new TransactionOperationAbstract() { - - @Override - public void afterCommit(Transaction tx) { - pendingDeletes.remove(uniqueName); - } - - @Override - public void afterRollback(Transaction tx) { - nameMap.put(uniqueName, binding); - pendingDeletes.remove(uniqueName); - } - - }); - } - removeBindingInternal(binding.getAddress(), uniqueName); return binding; http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/454e5b66/tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/DurableSubscriptionTest.java ---------------------------------------------------------------------- diff --git a/tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/DurableSubscriptionTest.java b/tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/DurableSubscriptionTest.java index f75fa31..c89eb5f 100644 --- a/tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/DurableSubscriptionTest.java +++ b/tests/jms-tests/src/test/java/org/apache/activemq/artemis/jms/tests/DurableSubscriptionTest.java @@ -28,6 +28,7 @@ import javax.jms.Session; import javax.jms.TextMessage; import javax.jms.Topic; import javax.jms.TopicSubscriber; +import javax.naming.InitialContext; import java.util.List; import org.apache.activemq.artemis.jms.tests.util.ProxyAssertSupport; @@ -107,6 +108,62 @@ public class DurableSubscriptionTest extends JMSTestCase { } } + // https://issues.apache.org/jira/browse/ARTEMIS-177 + @Test + public void testDurableSubscriptionRemovalRaceCondition() throws Exception { + final String topicName = "myTopic"; + final String clientID = "myClientID"; + final String subscriptionName = "mySub"; + createTopic(topicName); + InitialContext ic = getInitialContext(); + Topic myTopic = (Topic) ic.lookup("/topic/" + topicName); + + Connection conn = null; + + for (int i = 0; i < 1000; i++) { + try { + conn = createConnection(); + + conn.setClientID(clientID); + + Session s = conn.createSession(false, Session.AUTO_ACKNOWLEDGE); + MessageProducer prod = s.createProducer(myTopic); + prod.setDeliveryMode(DeliveryMode.PERSISTENT); + + s.createDurableSubscriber(myTopic, subscriptionName); + + prod.send(s.createTextMessage("k")); + + conn.close(); + + destroyTopic(topicName); + + createTopic(topicName); + + conn = createConnection(); + conn.setClientID(clientID); + + s = conn.createSession(false, Session.AUTO_ACKNOWLEDGE); + + MessageConsumer durable = s.createDurableSubscriber(myTopic, subscriptionName); + + conn.start(); + + TextMessage tm = (TextMessage) durable.receiveNoWait(); + ProxyAssertSupport.assertNull(tm); + + durable.close(); + + s.unsubscribe(subscriptionName); + } + finally { + if (conn != null) { + conn.close(); + } + } + } + } + /** * JMS 1.1 6.11.1: A client can change an existing durable subscription by creating a durable * TopicSubscriber with the same name and a new topic and/or message selector, or NoLocal
