ARTEMIS-2015 PriorityLinkedListImpl::isEmpty is not thread-safe PriorityLinkedListImpl::size access is changed to be safely observable by a thread different from the one allowed to write the list.
Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/64cf5357 Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/64cf5357 Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/64cf5357 Branch: refs/heads/master Commit: 64cf5357e1a88d98b4e2c3569bb10d489f55e404 Parents: 70f5512 Author: Francesco Nigro <nigro....@gmail.com> Authored: Tue Aug 7 18:53:13 2018 +0200 Committer: Clebert Suconic <clebertsuco...@apache.org> Committed: Wed Aug 8 14:25:28 2018 -0400 ---------------------------------------------------------------------- .../utils/collections/PriorityLinkedList.java | 11 +++++++++- .../collections/PriorityLinkedListImpl.java | 23 +++++++++++++++----- 2 files changed, 27 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/64cf5357/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedList.java ---------------------------------------------------------------------- diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedList.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedList.java index 79a99f3..19e58c2 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedList.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedList.java @@ -18,7 +18,8 @@ package org.apache.activemq.artemis.utils.collections; /** * A type of linked list which maintains items according to a priority - * and allows adding and removing of elements at both ends, and peeking + * and allows adding and removing of elements at both ends, and peeking.<br> + * Only {@link #size()} and {@link #isEmpty()} are safe to be called concurrently. */ public interface PriorityLinkedList<T> { @@ -30,9 +31,17 @@ public interface PriorityLinkedList<T> { void clear(); + /** + * Returns the size of this list.<br> + * It is safe to be called concurrently. + */ int size(); LinkedListIterator<T> iterator(); + /** + * Returns {@code true} if empty, {@code false} otherwise.<br> + * It is safe to be called concurrently. + */ boolean isEmpty(); } http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/64cf5357/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedListImpl.java ---------------------------------------------------------------------- diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedListImpl.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedListImpl.java index 39d6b6d..00cf046 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedListImpl.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/PriorityLinkedListImpl.java @@ -18,6 +18,7 @@ package org.apache.activemq.artemis.utils.collections; import java.lang.reflect.Array; import java.util.NoSuchElementException; +import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; /** * A priority linked list implementation @@ -26,9 +27,11 @@ import java.util.NoSuchElementException; */ public class PriorityLinkedListImpl<T> implements PriorityLinkedList<T> { + private static final AtomicIntegerFieldUpdater<PriorityLinkedListImpl> SIZE_UPDATER = AtomicIntegerFieldUpdater.newUpdater(PriorityLinkedListImpl.class, "size"); + protected LinkedListImpl<T>[] levels; - private int size; + private volatile int size; private int lastReset; @@ -65,7 +68,7 @@ public class PriorityLinkedListImpl<T> implements PriorityLinkedList<T> { levels[priority].addHead(t); - size++; + exclusiveIncrementSize(1); } @Override @@ -74,7 +77,7 @@ public class PriorityLinkedListImpl<T> implements PriorityLinkedList<T> { levels[priority].addTail(t); - size++; + exclusiveIncrementSize(1); } @Override @@ -94,7 +97,7 @@ public class PriorityLinkedListImpl<T> implements PriorityLinkedList<T> { t = ll.poll(); if (t != null) { - size--; + exclusiveIncrementSize(-1); if (ll.size() == 0) { if (highestPriority == i) { @@ -116,7 +119,15 @@ public class PriorityLinkedListImpl<T> implements PriorityLinkedList<T> { list.clear(); } - size = 0; + exclusiveSetSize(0); + } + + private void exclusiveIncrementSize(int amount) { + SIZE_UPDATER.lazySet(this, this.size + amount); + } + + private void exclusiveSetSize(int value) { + SIZE_UPDATER.lazySet(this, value); } @Override @@ -242,7 +253,7 @@ public class PriorityLinkedListImpl<T> implements PriorityLinkedList<T> { highestPriority = i; } - size--; + exclusiveIncrementSize(-1); } } }