Simplify code in iterator remove to avoid incorrect ConcurrentModificationException

bug 34690, from Guilhem Lavaux at Kaffe

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/collections/trunk@171360 13f79535-47bb-0310-9956-ffa450edef68

RELEASE-NOTES.html

@@ -61,6 +61,7 @@ If this causes major headaches to anyone please contact commons-dev at jakarta.a
 <center><h3>BUG FIXES</h3></center>
 <ul>
 <li>FastArrayList - Fix iterators and views to work better in multithreaded environments</li>
+<li>FastArrayList - Fix iterator remove where ConcurrentModificationException not as expected [34690]</li>
 <li>BoundedFifoBuffer/CircularFifoBuffer - Fix serialization to work in case where buffer serialized when full [31433]</li>
 <li>BoundedFifoBuffer - Fix iterator remove bug causing ArrayIndexOutOfBounds error [33071]</li>
 <li>MultiHashMap.remove(key, item) - Was returning the item even when nothing was removed [32366]</li>

src/java/org/apache/commons/collections/FastArrayList.java

@@ -1,5 +1,5 @@
 /*
- *  Copyright 2001-2004 The Apache Software Foundation
+ *  Copyright 2001-2005 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.
@@ -1305,7 +1305,7 @@ public class FastArrayList extends ArrayList {
             }
             get().remove(lastReturnedIndex);
             expected = list;
-            iter = get().listIterator(previousIndex());
+            iter = get().listIterator(lastReturnedIndex);
             lastReturnedIndex = -1;
         }
 

src/test/org/apache/commons/collections/TestFastArrayList.java

@@ -1,5 +1,5 @@
 /*
- *  Copyright 2001-2004 The Apache Software Foundation
+ *  Copyright 2001-2005 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,7 +16,9 @@
 package org.apache.commons.collections;
 
 import java.util.ArrayList;
+import java.util.ConcurrentModificationException;
 import java.util.List;
+import java.util.ListIterator;
 
 import junit.framework.Test;
 
@@ -52,4 +54,111 @@ public class TestFastArrayList extends TestArrayList {
         return (fal);
     }
 
+    public void testConcurrentModification_alwaysFast() {
+        FastArrayList list = new FastArrayList();
+        list.setFast(true);
+        list.add("a");
+        list.add("b");
+        list.add("c");
+        ListIterator iter = list.listIterator();
+        assertEquals("a", iter.next());
+        assertEquals("b", iter.next());
+        iter.remove();  // checking for no ConcurrentModificationException
+        assertEquals("c", iter.next());
+        assertEquals(false, iter.hasNext());
+        assertEquals("c", iter.previous());
+        assertEquals("a", iter.previous());
+        assertEquals(false, iter.hasPrevious());
+    }
+
+    public void testConcurrentModification_alwaysFastModError() {
+        FastArrayList list = new FastArrayList();
+        list.setFast(true);
+        list.add("a");
+        list.add("b");
+        list.add("c");
+        ListIterator iter = list.listIterator();
+        assertEquals("a", iter.next());
+        assertEquals("b", iter.next());
+        list.remove(1);
+        try {
+            iter.remove();
+        } catch (ConcurrentModificationException ex) {
+            // expected
+        }
+        // iterator state now invalid
+    }
+
+    public void testConcurrentModification_delayedFast() {
+        FastArrayList list = new FastArrayList();
+        list.add("a");
+        list.add("b");
+        list.add("c");
+        ListIterator iter = list.listIterator();
+        assertEquals("a", iter.next());
+        assertEquals("b", iter.next());
+        list.setFast(true);
+        iter.remove();  // checking for no ConcurrentModificationException
+        assertEquals("c", iter.next());
+        assertEquals(false, iter.hasNext());
+        assertEquals("c", iter.previous());
+        assertEquals("a", iter.previous());
+        assertEquals(false, iter.hasPrevious());
+    }
+
+    public void testConcurrentModification_delayedFastModError() {
+        FastArrayList list = new FastArrayList();
+        list.add("a");
+        list.add("b");
+        list.add("c");
+        ListIterator iter = list.listIterator();
+        assertEquals("a", iter.next());
+        assertEquals("b", iter.next());
+        list.setFast(true);
+        list.remove(1);
+        try {
+            iter.remove();
+        } catch (ConcurrentModificationException ex) {
+            // expected
+        }
+        // iterator state now invalid
+    }
+
+    public void testConcurrentModification_alwaysFastPrevious() {
+        FastArrayList list = new FastArrayList();
+        list.setFast(true);
+        list.add("a");
+        list.add("b");
+        list.add("c");
+        ListIterator iter = list.listIterator();
+        assertEquals("a", iter.next());
+        assertEquals("b", iter.next());
+        assertEquals("b", iter.previous());
+        iter.remove();  // checking for no ConcurrentModificationException
+        assertEquals("c", iter.next());
+        assertEquals(false, iter.hasNext());
+        assertEquals("c", iter.previous());
+        assertEquals("a", iter.previous());
+        assertEquals(false, iter.hasPrevious());
+    }
+
+    public void testConcurrentModification_alwaysFastModErrorPrevious() {
+        FastArrayList list = new FastArrayList();
+        list.setFast(true);
+        list.add("a");
+        list.add("b");
+        list.add("c");
+        ListIterator iter = list.listIterator();
+        assertEquals("a", iter.next());
+        assertEquals("b", iter.next());
+        assertEquals("b", iter.previous());
+        list.remove(1);
+        try {
+            iter.remove();
+        } catch (ConcurrentModificationException ex) {
+            // expected
+        }
+        // iterator state now invalid
+    }
+
 }