Skip to content
Permalink
Browse files
ARTEMIS-3802 Avoiding races on deleteWithID and Iterator.remove
  • Loading branch information
clebertsuconic committed Apr 26, 2022
1 parent 502461f commit 1946351fcef535e86547bea1f9394936e9a62dc3
Showing 2 changed files with 129 additions and 36 deletions.
@@ -103,7 +103,7 @@ public void addHead(E e) {
}

@Override
public E removeWithID(String listID, long id) {
public synchronized E removeWithID(String listID, long id) {
assert nodeStore != null; // it is assumed the code will call setNodeStore before callin removeWithID

Node<E> node = nodeStore.getNode(listID, id);
@@ -422,67 +422,77 @@ public void repeat() {

@Override
public boolean hasNext() {
Node<E> e = getNode();
synchronized (LinkedListImpl.this) {
Node<E> e = getNode();

if (e != null && (e != last || repeat)) {
return true;
}
if (e != null && (e != last || repeat)) {
return true;
}

return canAdvance();
return canAdvance();
}
}

@Override
public E next() {
Node<E> e = getNode();
synchronized (LinkedListImpl.this) {
Node<E> e = getNode();

if (repeat) {
repeat = false;
if (repeat) {
repeat = false;

if (e != null) {
return e.val();
} else {
if (e != null) {
return e.val();
} else {
if (canAdvance()) {
advance();

e = getNode();

return e.val();
} else {
throw new NoSuchElementException();
}
}
}

if (e == null || e == last) {
if (canAdvance()) {
advance();

e = getNode();

return e.val();
} else {
throw new NoSuchElementException();
}
}
}

if (e == null || e == last) {
if (canAdvance()) {
advance();
last = e;

e = getNode();
} else {
throw new NoSuchElementException();
}
}

last = e;

repeat = false;
repeat = false;

return e.val();
return e.val();
}
}

@Override
public void remove() {
if (last == null) {
throw new NoSuchElementException();
}
synchronized (LinkedListImpl.this) {
if (last == null) {
throw new NoSuchElementException();
}

if (current == null) {
throw new NoSuchElementException();
}
if (current == null) {
return;
}

Node<E> prev = current.prev;

LinkedListImpl.this.removeAfter(current.prev);
if (prev != null) {
LinkedListImpl.this.removeAfter(prev);

last = null;
last = null;
}
}
}

@Override
@@ -22,6 +22,12 @@
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.CyclicBarrier;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

import io.netty.util.collection.LongObjectHashMap;
import org.apache.activemq.artemis.tests.util.ActiveMQTestBase;
@@ -313,6 +319,83 @@ private void internalAddWithID(boolean deferSupplier) {
}
}


@Test
public void testRaceRemovingID() throws Exception {
ExecutorService executor = Executors.newFixedThreadPool(2);

int elements = 1000;
try {
LinkedListImpl<ObservableNode> objs = new LinkedListImpl<>();
ListNodeStore nodeStore = new ListNodeStore();
objs.setNodeStore(nodeStore);
final String serverID = RandomUtil.randomString();

for (int i = 0; i < elements; i++) {
objs.addHead(new ObservableNode(serverID, i));
}
Assert.assertEquals(elements, objs.size());

CyclicBarrier barrier = new CyclicBarrier(2);

CountDownLatch latch = new CountDownLatch(2);
AtomicInteger errors = new AtomicInteger(0);

executor.execute(() -> {
try {
barrier.await(10, TimeUnit.SECONDS);
} catch (Exception e) {
e.printStackTrace();
}

try {
for (int i = 0; i < elements; i++) {
objs.removeWithID(serverID, i);
}
} catch (Exception e) {
e.printStackTrace();
errors.incrementAndGet();
} finally {
latch.countDown();
}
});

executor.execute(() -> {
LinkedListIterator iterator = objs.iterator();

try {
barrier.await(10, TimeUnit.SECONDS);
} catch (Exception e) {
e.printStackTrace();
}

try {
while (iterator.hasNext()) {
Object value = iterator.next();
iterator.remove();
}
} catch (Exception e) {
if (e instanceof NoSuchElementException) {
// this is ok
} else {
e.printStackTrace();
errors.incrementAndGet();
}
} finally {
latch.countDown();
}
});

Assert.assertTrue(latch.await(10, TimeUnit.SECONDS));

Assert.assertEquals(0, objs.size());

Assert.assertEquals(0, errors.get());
} finally {
executor.shutdownNow();
}
}

@Test
public void testAddHeadAndRemove() {
LinkedListImpl<ObservableNode> objs = new LinkedListImpl<>();

0 comments on commit 1946351

Please sign in to comment.