From b6809f6ce698fae50b39917a271cdade55b8f2cb Mon Sep 17 00:00:00 2001 From: "Kristian S. Stangeland" Date: Fri, 1 Nov 2013 04:45:30 +0100 Subject: [PATCH] Correctly remove packet listeners from the sorted lists. Essentially, the SortedCopyOnWriteArray.remove() method would remove any packet listeners following a match, instead of just the match itself. Thanks to Libraryaddict for discovering the bug. :) --- .../concurrency/SortedCopyOnWriteArray.java | 18 +++++++++++------- .../injector/SortedCopyOnWriteArrayTest.java | 5 +++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/SortedCopyOnWriteArray.java b/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/SortedCopyOnWriteArray.java index 19bbafa2..8f9e3f60 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/SortedCopyOnWriteArray.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/concurrency/SortedCopyOnWriteArray.java @@ -68,7 +68,6 @@ public class SortedCopyOnWriteArray> implements Iterable */ @Override public synchronized boolean add(T value) { - // We use NULL as a special marker, so we don't allow it if (value == null) throw new IllegalArgumentException("value cannot be NULL"); @@ -94,7 +93,6 @@ public class SortedCopyOnWriteArray> implements Iterable @Override public synchronized boolean addAll(Collection values) { - if (values == null) throw new IllegalArgumentException("values cannot be NULL"); if (values.size() == 0) @@ -120,20 +118,22 @@ public class SortedCopyOnWriteArray> implements Iterable @Override public synchronized boolean remove(Object value) { List copy = new ArrayList(); + boolean result = false; // Note that there's not much to be gained from using BinarySearch, as we // have to copy (and thus read) the entire list regardless. // Copy every element except the one given to us. for (T element : list) { - if (value != null && !Objects.equal(value, element)) { - copy.add(element); - value = null; + if (!Objects.equal(value, element)) { + copy.add(element); + } else { + result = true; } } list = copy; - return value == null; + return result; } @Override @@ -175,7 +175,6 @@ public class SortedCopyOnWriteArray> implements Iterable * @param index - index of the element to remove. */ public synchronized void remove(int index) { - List copy = new ArrayList(list); copy.remove(index); @@ -237,4 +236,9 @@ public class SortedCopyOnWriteArray> implements Iterable public T[] toArray(T[] a) { return list.toArray(a); } + + @Override + public String toString() { + return list.toString(); + } } diff --git a/ProtocolLib/src/test/java/com/comphenix/protocol/injector/SortedCopyOnWriteArrayTest.java b/ProtocolLib/src/test/java/com/comphenix/protocol/injector/SortedCopyOnWriteArrayTest.java index 6bc2cce1..610759c4 100644 --- a/ProtocolLib/src/test/java/com/comphenix/protocol/injector/SortedCopyOnWriteArrayTest.java +++ b/ProtocolLib/src/test/java/com/comphenix/protocol/injector/SortedCopyOnWriteArrayTest.java @@ -72,6 +72,11 @@ public class SortedCopyOnWriteArrayTest { // Make sure the normal's are in the right order assertEquals(2, test.get(0).id); assertEquals(3, test.get(1).id); + + // Test remove + test.remove(b); + assertEquals(2, test.size()); + assertFalse(test.contains(b)); } private static class PriorityStuff implements Comparable {