QPID-6196: Add synchronization lock into AbstractConfiguredObject.unregisterChild

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

2 files changed, 6 insertions, 121 deletions

diff --git a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
index d4a11290a9..a2f4f4ee0a 100644
--- a/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
+++ b/qpid/java/broker-core/src/main/java/org/apache/qpid/server/model/AbstractConfiguredObject.java
@@ -1443,9 +1443,12 @@ public abstract class AbstractConfiguredObject<X extends ConfiguredObject<X>> im
     private <C extends ConfiguredObject> void unregisterChild(final C child)
     {
         Class categoryClass = child.getCategoryClass();
-        _children.get(categoryClass).remove(child);
-        _childrenById.get(categoryClass).remove(child.getId());
-        _childrenByName.get(categoryClass).remove(child.getName());
+        synchronized(_children)
+        {
+            _children.get(categoryClass).remove(child);
+            _childrenById.get(categoryClass).remove(child.getId());
+            _childrenByName.get(categoryClass).remove(child.getName());
+        }
     }
 
     @Override
diff --git a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
index 07dce0f8a1..3ffb4c42c2 100644
--- a/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
+++ b/qpid/java/broker-core/src/test/java/org/apache/qpid/server/model/AbstractConfiguredObjectTest.java
@@ -615,122 +615,4 @@ public class AbstractConfiguredObjectTest extends QpidTestCase
             assertTrue(obj.getEnumSetValues().containsAll(Arrays.asList(TestEnum.TEST_ENUM2, TestEnum.TEST_ENUM3)));
         }
     }
-
-    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());
-    }
 }