Repository: activemq-artemis Updated Branches: refs/heads/master 682216a3e -> c85f0f383
ARTEMIS-1345 ConcurrentModificationException after copy Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/8831a570 Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/8831a570 Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/8831a570 Branch: refs/heads/master Commit: 8831a570de575f19cbb238e8e1956119061057f2 Parents: 682216a Author: Clebert Suconic <clebertsuco...@apache.org> Authored: Wed Mar 7 15:04:39 2018 -0500 Committer: Clebert Suconic <clebertsuco...@apache.org> Committed: Wed Mar 7 18:05:35 2018 -0500 ---------------------------------------------------------------------- .../utils/collections/TypedProperties.java | 6 ++-- .../artemis/utils/TypedPropertiesTest.java | 34 ++++++++++++++++++++ .../artemis/core/server/impl/QueueImpl.java | 10 +++++- 3 files changed, 47 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/8831a570/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/TypedProperties.java ---------------------------------------------------------------------- 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 25c3638..aa2d551 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 class TypedProperties { } 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() { http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/8831a570/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/TypedPropertiesTest.java ---------------------------------------------------------------------- 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 ea044ac..1e0d566 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 class TypedPropertiesTest { 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()); + } } http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/8831a570/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java ---------------------------------------------------------------------- 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 b9adf33..0d018dc 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 @@ public class QueueImpl extends CriticalComponentImpl implements Queue { 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) {
