From 43e23dd6584ba81ef1e57554248441f35bee0195 Mon Sep 17 00:00:00 2001
From: samabcde <tszsumng90@gmail.com>
Date: Mon, 25 Apr 2022 00:07:27 +0800
Subject: [PATCH] [COLLECTIONS-802] Fix remove failed by removing set null to
 currentKey and currentValue.

---
 .../map/AbstractReferenceMap.java             | 24 +++++++++----------
 .../collections4/map/ReferenceMapTest.java    | 19 +++++++++++++++
 2 files changed, 30 insertions(+), 13 deletions(-)

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<K, V> extends AbstractHashedMap<K, V>
 
         // These fields keep track of where we are in the table.
         int index;
-        ReferenceEntry<K, V> entry;
-        ReferenceEntry<K, V> previous;
+        ReferenceEntry<K, V> next;
+        ReferenceEntry<K, V> 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<K, V> extends AbstractHashedMap<K, V>
         public boolean hasNext() {
             checkMod();
             while (nextNull()) {
-                ReferenceEntry<K, V> e = entry;
+                ReferenceEntry<K, V> e = next;
                 int i = index;
                 while (e == null && i > 0) {
                     i--;
                     e = (ReferenceEntry<K, V>) 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<K, V> extends AbstractHashedMap<K, V>
             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<K, V> 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<K, V> extends AbstractIterableMapTest<K, V> {
 
     }
 
+    /**
+     * Test whether remove is not removing last entry after calling hasNext.
+     * <p>
+     * See <a href="https://issues.apache.org/jira/browse/COLLECTIONS-802">COLLECTIONS-802: ReferenceMap iterator remove violates contract</a>
+     */
+    @Test
+    public void testIteratorLastEntryCanBeRemovedAfterHasNext() {
+        ReferenceMap<Integer, Integer> map = new ReferenceMap<>();
+        map.put(1, 2);
+        Iterator<Map.Entry<Integer, Integer>> 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 {