diff --git a/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java b/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java index 684ed58a2..a55825e00 100644 --- a/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java +++ b/src/main/java/org/apache/commons/collections4/map/AbstractReferenceMap.java @@ -772,8 +772,8 @@ public abstract class AbstractReferenceMap extends AbstractHashedMap // These fields keep track of where we are in the table. int index; - ReferenceEntry entry; - ReferenceEntry previous; + ReferenceEntry next; + ReferenceEntry current; // These Object fields provide hard references to the // current and next entry; this assures that if hasNext() @@ -794,23 +794,21 @@ public abstract class AbstractReferenceMap extends AbstractHashedMap public boolean hasNext() { checkMod(); while (nextNull()) { - ReferenceEntry e = entry; + ReferenceEntry e = next; int i = index; while (e == null && i > 0) { i--; e = (ReferenceEntry) parent.data[i]; } - entry = e; + next = e; index = i; if (e == null) { - currentKey = null; - currentValue = null; return false; } nextKey = e.getKey(); nextValue = e.getValue(); if (nextNull()) { - entry = entry.next(); + next = next.next(); } } return true; @@ -831,27 +829,27 @@ public abstract class AbstractReferenceMap extends AbstractHashedMap if (nextNull() && !hasNext()) { throw new NoSuchElementException(); } - previous = entry; - entry = entry.next(); + current = next; + next = next.next(); currentKey = nextKey; currentValue = nextValue; nextKey = null; nextValue = null; - return previous; + return current; } protected ReferenceEntry currentEntry() { checkMod(); - return previous; + return current; } public void remove() { checkMod(); - if (previous == null) { + if (current == null) { throw new IllegalStateException(); } parent.remove(currentKey); - previous = null; + current = null; currentKey = null; currentValue = null; expectedModCount = parent.modCount; diff --git a/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java b/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java index 352448e82..509ac5148 100644 --- a/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java +++ b/src/test/java/org/apache/commons/collections4/map/ReferenceMapTest.java @@ -26,6 +26,7 @@ import java.io.ObjectOutputStream; import java.io.Serializable; import java.lang.ref.WeakReference; import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.function.Consumer; @@ -315,6 +316,24 @@ public class ReferenceMapTest extends AbstractIterableMapTest { } + /** + * Test whether remove is not removing last entry after calling hasNext. + *

+ * See COLLECTIONS-802: ReferenceMap iterator remove violates contract + */ + @Test + public void testIteratorLastEntryCanBeRemovedAfterHasNext() { + ReferenceMap map = new ReferenceMap<>(); + map.put(1, 2); + Iterator> iter = map.entrySet().iterator(); + assertTrue(iter.hasNext()); + iter.next(); + // below line should not affect remove + assertFalse(iter.hasNext()); + iter.remove(); + assertTrue("Expect empty but have entry: " + map, map.isEmpty()); + } + @SuppressWarnings("unused") private static void gc() { try {