Vector: Rework CME checks in hasNext() and next().
Port over commit b10b2a3ab693cfd6156d06ffe4e00ce69b9c9194
from java.util.ArrayList to java.util.Vector.
Also adds the same set of tests to Vector / ArrayList so
to make sure their iterators behave identically in this regard.
bug: 27430229
Change-Id: Idd8e44737a216ac5ea9f1e6fed8892b364997292
diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/ArrayListTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/ArrayListTest.java
index 87b1cd9..3ec7c86 100644
--- a/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/ArrayListTest.java
+++ b/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/ArrayListTest.java
@@ -1023,7 +1023,7 @@
// http://b/25867131 et al.
public void testIteratorAddAfterCompleteIteration() {
- ArrayList<String> strings = new ArrayList<String>();
+ ArrayList<String> strings = new ArrayList<>();
strings.add("string1");
Iterator<String> it = strings.iterator();
assertTrue(it.hasNext());
@@ -1053,6 +1053,16 @@
assertEquals("string2", it.next());
}
+ // http://b/27430229
+ public void testRemoveAllDuringIteration() {
+ ArrayList<String> list = new ArrayList<>();
+ list.add("food");
+ Iterator<String> iterator = list.iterator();
+ iterator.next();
+ list.clear();
+ assertFalse(iterator.hasNext());
+ }
+
public void test_forEach() throws Exception {
ArrayList<Integer> list = new ArrayList<>();
list.add(0);
diff --git a/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/VectorTest.java b/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/VectorTest.java
index a6f5626..a9f64a2 100644
--- a/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/VectorTest.java
+++ b/harmony-tests/src/test/java/org/apache/harmony/tests/java/util/VectorTest.java
@@ -1475,6 +1475,48 @@
testRemoveIfCME(Vector<Integer>::new);
}
+ // http://b/25867131 et al.
+ public void testIteratorAddAfterCompleteIteration() {
+ Vector<String> strings = new Vector<>();
+ strings.add("string1");
+ Iterator<String> it = strings.iterator();
+ assertTrue(it.hasNext());
+ assertEquals("string1", it.next());
+ assertFalse(it.hasNext());
+ strings.add("string2");
+ // The value of hasNext() must not flap between true and false. If we returned "true"
+ // here, we'd fail with a CME on the next call to next() anyway.
+ assertFalse(it.hasNext());
+ }
+
+ public void testHasNextAfterRemoval() {
+ Vector<String> strings = new Vector<>();
+ strings.add("string1");
+ Iterator<String> it = strings.iterator();
+ it.next();
+ it.remove();
+ assertFalse(it.hasNext());
+
+ strings = new Vector<>();
+ strings.add("string1");
+ strings.add("string2");
+ it = strings.iterator();
+ it.next();
+ it.remove();
+ assertTrue(it.hasNext());
+ assertEquals("string2", it.next());
+ }
+
+ // http://b/27430229
+ public void testRemoveAllDuringIteration() {
+ Vector<String> vector = new Vector<>();
+ vector.add("food");
+ Iterator<String> vectorIterator = vector.iterator();
+ vectorIterator.next();
+ vector.removeAllElements();
+ assertFalse(vectorIterator.hasNext());
+ }
+
/**
* Sets up the fixture, for example, open a network connection. This method
* is called before a test is executed.
diff --git a/ojluni/src/main/java/java/util/Vector.java b/ojluni/src/main/java/java/util/Vector.java
index 6afdfbd..d51c47e 100755
--- a/ojluni/src/main/java/java/util/Vector.java
+++ b/ojluni/src/main/java/java/util/Vector.java
@@ -1121,21 +1121,26 @@
* An optimized version of AbstractList.Itr
*/
private class Itr implements Iterator<E> {
+ // The "limit" of this iterator. This is the size of the list at the time the
+ // iterator was created. Adding & removing elements will invalidate the iteration
+ // anyway (and cause next() to throw) so saving this value will guarantee that the
+ // value of hasNext() remains stable and won't flap between true and false when elements
+ // are added and removed from the list.
+ protected int limit = Vector.this.elementCount;
+
int cursor; // index of next element to return
int lastRet = -1; // index of last element returned; -1 if no such
int expectedModCount = modCount;
public boolean hasNext() {
- // Racy but within spec, since modifications are checked
- // within or after synchronization in next/previous
- return cursor != elementCount;
+ return cursor < limit;
}
public E next() {
synchronized (Vector.this) {
checkForComodification();
int i = cursor;
- if (i >= elementCount)
+ if (i >= limit)
throw new NoSuchElementException();
cursor = i + 1;
return elementData(lastRet = i);
@@ -1149,6 +1154,7 @@
checkForComodification();
Vector.this.remove(lastRet);
expectedModCount = modCount;
+ limit--;
}
cursor = lastRet;
lastRet = -1;
@@ -1158,7 +1164,7 @@
public void forEachRemaining(Consumer<? super E> action) {
Objects.requireNonNull(action);
synchronized (Vector.this) {
- final int size = elementCount;
+ final int size = limit;
int i = cursor;
if (i >= size) {
return;
