Archiviert
13
0

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. :)
Dieser Commit ist enthalten in:
Kristian S. Stangeland 2013-11-01 04:45:30 +01:00
Ursprung c416877f35
Commit b6809f6ce6
2 geänderte Dateien mit 16 neuen und 7 gelöschten Zeilen

Datei anzeigen

@ -68,7 +68,6 @@ public class SortedCopyOnWriteArray<T extends Comparable<T>> implements Iterable
*/ */
@Override @Override
public synchronized boolean add(T value) { public synchronized boolean add(T value) {
// We use NULL as a special marker, so we don't allow it // We use NULL as a special marker, so we don't allow it
if (value == null) if (value == null)
throw new IllegalArgumentException("value cannot be NULL"); throw new IllegalArgumentException("value cannot be NULL");
@ -94,7 +93,6 @@ public class SortedCopyOnWriteArray<T extends Comparable<T>> implements Iterable
@Override @Override
public synchronized boolean addAll(Collection<? extends T> values) { public synchronized boolean addAll(Collection<? extends T> values) {
if (values == null) if (values == null)
throw new IllegalArgumentException("values cannot be NULL"); throw new IllegalArgumentException("values cannot be NULL");
if (values.size() == 0) if (values.size() == 0)
@ -120,20 +118,22 @@ public class SortedCopyOnWriteArray<T extends Comparable<T>> implements Iterable
@Override @Override
public synchronized boolean remove(Object value) { public synchronized boolean remove(Object value) {
List<T> copy = new ArrayList<T>(); List<T> copy = new ArrayList<T>();
boolean result = false;
// Note that there's not much to be gained from using BinarySearch, as we // Note that there's not much to be gained from using BinarySearch, as we
// have to copy (and thus read) the entire list regardless. // have to copy (and thus read) the entire list regardless.
// Copy every element except the one given to us. // Copy every element except the one given to us.
for (T element : list) { for (T element : list) {
if (value != null && !Objects.equal(value, element)) { if (!Objects.equal(value, element)) {
copy.add(element); copy.add(element);
value = null; } else {
result = true;
} }
} }
list = copy; list = copy;
return value == null; return result;
} }
@Override @Override
@ -175,7 +175,6 @@ public class SortedCopyOnWriteArray<T extends Comparable<T>> implements Iterable
* @param index - index of the element to remove. * @param index - index of the element to remove.
*/ */
public synchronized void remove(int index) { public synchronized void remove(int index) {
List<T> copy = new ArrayList<T>(list); List<T> copy = new ArrayList<T>(list);
copy.remove(index); copy.remove(index);
@ -237,4 +236,9 @@ public class SortedCopyOnWriteArray<T extends Comparable<T>> implements Iterable
public <T> T[] toArray(T[] a) { public <T> T[] toArray(T[] a) {
return list.toArray(a); return list.toArray(a);
} }
@Override
public String toString() {
return list.toString();
}
} }

Datei anzeigen

@ -72,6 +72,11 @@ public class SortedCopyOnWriteArrayTest {
// Make sure the normal's are in the right order // Make sure the normal's are in the right order
assertEquals(2, test.get(0).id); assertEquals(2, test.get(0).id);
assertEquals(3, test.get(1).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<PriorityStuff> { private static class PriorityStuff implements Comparable<PriorityStuff> {