You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by or...@apache.org on 2014/10/29 16:56:56 UTC
svn commit: r1635184 - in /qpid/trunk/qpid/java/broker-core/src:
main/java/org/apache/qpid/server/model/
test/java/org/apache/qpid/server/model/
test/java/org/apache/qpid/server/model/testmodel/
Author: orudyy
Date: Wed Oct 29 15:56:56 2014
New Revision: 1635184
URL: http://svn.apache.org/r1635184
Log:
QPID-6196: Add synchronization on CO registration to avoid creation of objects with duplicate names and ids
Modified:
qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java
Modified: qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java?rev=1635184&r1=1635183&r2=1635184&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java Wed Oct 29 15:56:56 2014
@@ -1379,25 +1379,26 @@ public abstract class AbstractConfigured
private <C extends ConfiguredObject> void registerChild(final C child)
{
-
- Class categoryClass = child.getCategoryClass();
- UUID childId = child.getId();
- String name = child.getName();
- if(_childrenById.get(categoryClass).containsKey(childId))
+ synchronized(_children)
{
- throw new DuplicateIdException(child);
- }
- if(getModel().getParentTypes(categoryClass).size() == 1)
- {
- if (_childrenByName.get(categoryClass).containsKey(name))
+ Class categoryClass = child.getCategoryClass();
+ UUID childId = child.getId();
+ String name = child.getName();
+ if(_childrenById.get(categoryClass).containsKey(childId))
{
- throw new DuplicateNameException(child);
+ throw new DuplicateIdException(child);
}
- _childrenByName.get(categoryClass).put(name, child);
+ if(getModel().getParentTypes(categoryClass).size() == 1)
+ {
+ if (_childrenByName.get(categoryClass).containsKey(name))
+ {
+ throw new DuplicateNameException(child);
+ }
+ _childrenByName.get(categoryClass).put(name, child);
+ }
+ _children.get(categoryClass).add(child);
+ _childrenById.get(categoryClass).put(childId,child);
}
- _children.get(categoryClass).add(child);
- _childrenById.get(categoryClass).put(childId,child);
-
}
public final void stop()
Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java?rev=1635184&r1=1635183&r2=1635184&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java Wed Oct 29 15:56:56 2014
@@ -24,6 +24,12 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.qpid.server.configuration.IllegalConfigurationException;
import org.apache.qpid.server.model.testmodel.TestChildCategory;
@@ -610,4 +616,121 @@ public class AbstractConfiguredObjectTes
}
}
+ public void testCreateConcurrentlyChildrenWithTheSameName() throws Exception
+ {
+ final TestConfiguredObject parent = new TestConfiguredObject("parent");
+ parent.create();
+
+ short numberOfThreads = 300;
+ ExecutorService executor = Executors.newFixedThreadPool(numberOfThreads);
+
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final CountDownLatch endLatch = new CountDownLatch(numberOfThreads);
+ final AtomicInteger duplicateNameExceptionCounter = new AtomicInteger();
+ final AtomicInteger successCounter = new AtomicInteger();
+ try
+ {
+ for (int i = 0; i < numberOfThreads; i++)
+ {
+ executor.submit(new Runnable()
+ {
+ @Override
+ public void run()
+ {
+ TestConfiguredObject child = new TestConfiguredObject("child", parent, parent.getTaskExecutor());
+ try
+ {
+ startLatch.await();
+ child.create();
+ successCounter.incrementAndGet();
+ }
+ catch(AbstractConfiguredObject.DuplicateNameException e)
+ {
+ duplicateNameExceptionCounter.incrementAndGet();
+ }
+ catch (InterruptedException e)
+ {
+ // ignore
+ }
+ finally
+ {
+ endLatch.countDown();
+ }
+ }
+ });
+ }
+ startLatch.countDown();
+ assertTrue("Waiting interval expired", endLatch.await(10, TimeUnit.SECONDS));
+ }
+ finally
+ {
+ executor.shutdownNow();
+ }
+
+ assertEquals("Unexpected number of children", 1, parent.getChildren(TestConfiguredObject.class).size());
+ assertEquals("Unexpected number of successful creations", 1, successCounter.get());
+ assertEquals("Unexpected number of DuplicateNameException", numberOfThreads - 1, duplicateNameExceptionCounter.get());
+ }
+
+ public void testCreateConcurrentlyChildrenWithTheSameId() throws Exception
+ {
+ final TestConfiguredObject parent = new TestConfiguredObject("parent");
+ parent.create();
+
+ short numberOfThreads = 300;
+ ExecutorService executor = Executors.newFixedThreadPool(numberOfThreads);
+
+ final CountDownLatch startLatch = new CountDownLatch(1);
+ final CountDownLatch endLatch = new CountDownLatch(numberOfThreads);
+ final AtomicInteger duplicateIdExceptionCounter = new AtomicInteger();
+ final AtomicInteger successCounter = new AtomicInteger();
+ final UUID id = UUID.randomUUID();
+ try
+ {
+ for (int i = 0; i < numberOfThreads; i++)
+ {
+ final int iteration = i;
+ executor.submit(new Runnable()
+ {
+ @Override
+ public void run()
+ {
+ Map<String, Object> attributes = new HashMap<>();
+ attributes.put(ConfiguredObject.NAME, "child-" + iteration);
+ attributes.put(ConfiguredObject.ID, id);
+ TestConfiguredObject child = new TestConfiguredObject(parent, attributes);
+
+ try
+ {
+ startLatch.await();
+ child.create();
+ successCounter.incrementAndGet();
+ }
+ catch(AbstractConfiguredObject.DuplicateIdException e)
+ {
+ duplicateIdExceptionCounter.incrementAndGet();
+ }
+ catch (InterruptedException e)
+ {
+ // ignore
+ }
+ finally
+ {
+ endLatch.countDown();
+ }
+ }
+ });
+ }
+ startLatch.countDown();
+ assertTrue("Waiting interval expired", endLatch.await(10, TimeUnit.SECONDS));
+ }
+ finally
+ {
+ executor.shutdownNow();
+ }
+
+ assertEquals("Unexpected number of children", 1, parent.getChildren(TestConfiguredObject.class).size());
+ assertEquals("Unexpected number of successful creations", 1, successCounter.get());
+ assertEquals("Unexpected number of DuplicateIdException", numberOfThreads - 1, duplicateIdExceptionCounter.get());
+ }
}
Modified: qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java?rev=1635184&r1=1635183&r2=1635184&view=diff
==============================================================================
--- qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java (original)
+++ qpid/trunk/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/testmodel/TestConfiguredObject.java Wed Oct 29 15:56:56 2014
@@ -66,6 +66,11 @@ public class TestConfiguredObject extend
this(createParents(parent), Collections.<String, Object>singletonMap(ConfiguredObject.NAME, name), taskExecutor, TestConfiguredObjectModel.INSTANCE);
}
+ public TestConfiguredObject(ConfiguredObject<?> parent, Map<String, Object> attributes)
+ {
+ this(createParents(parent), attributes, parent.getTaskExecutor(), TestConfiguredObjectModel.INSTANCE);
+ }
+
public TestConfiguredObject(Map parents, Map<String, Object> attributes, TaskExecutor taskExecutor, Model model)
{
super(parents, attributes, taskExecutor, model);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org