From 8831a570de575f19cbb238e8e1956119061057f2 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Wed, 7 Mar 2018 15:04:39 -0500 Subject: [PATCH 1/2] ARTEMIS-1345 ConcurrentModificationException after copy --- .../utils/collections/TypedProperties.java | 6 ++-- .../artemis/utils/TypedPropertiesTest.java | 34 +++++++++++++++++++ .../artemis/core/server/impl/QueueImpl.java | 10 +++++- 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/TypedProperties.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/TypedProperties.java index 25c3638edc1..aa2d551834d 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/TypedProperties.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/TypedProperties.java @@ -80,8 +80,10 @@ public int getMemoryOffset() { } public TypedProperties(final TypedProperties other) { - properties = other.properties == null ? null : new HashMap<>(other.properties); - size = other.size; + synchronized (other) { + properties = other.properties == null ? null : new HashMap<>(other.properties); + size = other.size; + } } public boolean hasInternalProperties() { diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TypedPropertiesTest.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TypedPropertiesTest.java index ea044ac9da2..1e0d566f0ea 100644 --- a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TypedPropertiesTest.java +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TypedPropertiesTest.java @@ -17,6 +17,8 @@ package org.apache.activemq.artemis.utils; import java.util.Iterator; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; @@ -255,4 +257,36 @@ public void testByteBufStringValuePoolTooLong() { final TypedProperties.StringValue.ByteBufStringValuePool pool = new TypedProperties.StringValue.ByteBufStringValuePool(1, tooLong.length() - 1); Assert.assertNotSame(pool.getOrCreate(bb), pool.getOrCreate(bb.resetReaderIndex())); } + + @Test + public void testCopyingWhileMessingUp() throws Exception { + TypedProperties properties = new TypedProperties(); + AtomicBoolean running = new AtomicBoolean(true); + AtomicLong copies = new AtomicLong(0); + AtomicBoolean error = new AtomicBoolean(false); + Thread t = new Thread() { + @Override + public void run() { + while (running.get() && !error.get()) { + try { + copies.incrementAndGet(); + TypedProperties copiedProperties = new TypedProperties(properties); + } catch (Throwable e) { + e.printStackTrace(); + error.set(true); + } + } + } + }; + t.start(); + for (int i = 0; !error.get() && (i < 100 || copies.get() < 5000); i++) { + properties.putIntProperty(SimpleString.toSimpleString("key" + i), i); + } + + running.set(false); + + t.join(); + + Assert.assertFalse(error.get()); + } } diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java index b9adf338915..0d018dcfd42 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java @@ -2324,7 +2324,15 @@ private void deliver() { handled++; } else if (status == HandleStatus.BUSY) { - holder.iter.repeat(); + try { + holder.iter.repeat(); + } catch (NoSuchElementException e) { + // this could happen if there was an exception on the queue handling + // and it returned BUSY because of that exception + // + // We will just log it as there's nothing else we can do now. + logger.warn(e.getMessage(), e); + } noDelivery++; } else if (status == HandleStatus.NO_MATCH) { From 2a9381cd26f5c95e403d1d8ae507f7507d7fd616 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Wed, 7 Mar 2018 18:14:51 -0500 Subject: [PATCH 2/2] ARTEMIS-1669 Fixing test --- .../tests/unit/ra/ActiveMQResourceAdapterConfigTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ActiveMQResourceAdapterConfigTest.java b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ActiveMQResourceAdapterConfigTest.java index 9afefd99ca0..96992aa2559 100644 --- a/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ActiveMQResourceAdapterConfigTest.java +++ b/tests/unit-tests/src/test/java/org/apache/activemq/artemis/tests/unit/ra/ActiveMQResourceAdapterConfigTest.java @@ -408,6 +408,12 @@ public class ActiveMQResourceAdapterConfigTest extends ActiveMQTestBase { " DeserializationBlackList" + " java.lang.String" + " " + + " " + + " " + + " ***add***" + + " IgnoreJTA" + + " boolean" + + " " + " "; private static String rootConfig = "" + config + commentedOutConfigs + "";