TreeList iterator previous() broken as remove(int) could break invariants

bug 35258, reported by and test case from Tomas D.

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@370952 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Stephen Colebourne 2006-01-21 01:49:21 +00:00
parent 28749901a9
commit 22aec774e6
3 changed files with 41 additions and 2 deletions

View File

@ -87,6 +87,7 @@ If this causes major headaches to anyone please contact commons-dev at jakarta.a
<li>FastArrayList - Fix iterator remove where ConcurrentModificationException not as expected [34690]</li>
<li>CursorableLinkedList (list subpackage) - Fix iterator remove/set not throwing IllegalStateException after next-previous-removeByIndex [35766]</li>
<li>TreeList/CursorableLinkedList/NodeCachingLinkedList/AbstractLinkedList - Fix iterator remove not working properly when called after previous [35258]</li>
<li>TreeList - remove(int) could break class invariants, breaking iterator previous [35258]</li>
<li>SetUniqueList.set(int,Object) - Destroyed set status in certain circumstances [33294]</li>
<li>AbstractLinkedMap.init() - Now calls createEntry() to create the map entry object [33706]</li>
<li>AbstractHashedMap deserialization - Fix to prevent doubling of internal data array [34265]</li>

View File

@ -564,7 +564,14 @@ public class TreeList extends AbstractList {
if (rightIsNext) {
right = leftMax.right;
}
AVLNode leftPrevious = left.left;
left = left.removeMax();
if (left == null) {
// special case where left that was deleted was a double link
// only occurs when height difference is equal
left = leftPrevious;
leftIsPrevious = true;
}
if (relativePosition > 0) {
relativePosition--;
}

View File

@ -1,5 +1,5 @@
/*
* Copyright 2004 The Apache Software Foundation
* Copyright 2004,2006 The Apache Software Foundation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -16,6 +16,7 @@
package org.apache.commons.collections.list;
import java.util.List;
import java.util.ListIterator;
import junit.framework.Test;
@ -52,7 +53,6 @@ public class TestTreeList extends AbstractTestList {
}
public static void benchmark(List l) {
StringBuffer sb = new StringBuffer();
long start = System.currentTimeMillis();
for (int i = 0; i < 100000; i++) {
l.add(new Integer(i));
@ -210,4 +210,35 @@ public class TestTreeList extends AbstractTestList {
// l.add("A5");
// l.add("A6");
// }
public void testBug35258() {
Object objectToRemove = new Integer(3);
List treelist = new TreeList();
treelist.add(new Integer(0));
treelist.add(new Integer(1));
treelist.add(new Integer(2));
treelist.add(new Integer(3));
treelist.add(new Integer(4));
// this cause inconsistence of ListIterator()
treelist.remove(objectToRemove);
ListIterator li = treelist.listIterator();
assertEquals(new Integer(0), li.next());
assertEquals(new Integer(0), li.previous());
assertEquals(new Integer(0), li.next());
assertEquals(new Integer(1), li.next());
// this caused error in bug 35258
assertEquals(new Integer(1), li.previous());
assertEquals(new Integer(1), li.next());
assertEquals(new Integer(2), li.next());
assertEquals(new Integer(2), li.previous());
assertEquals(new Integer(2), li.next());
assertEquals(new Integer(4), li.next());
assertEquals(new Integer(4), li.previous());
assertEquals(new Integer(4), li.next());
assertEquals(false, li.hasNext());
}
}