qpid-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From oru...@apache.org
Subject 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/
Date Wed, 29 Oct 2014 15:56:56 GMT
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


Mime
View raw message