[COLLECTIONS-802] Fix remove failed by removing set null to currentKey and currentValue.

This commit is contained in:
samabcde 2022-04-25 00:07:27 +08:00
parent 9df6f64b7e
commit 43e23dd658
2 changed files with 30 additions and 13 deletions

View File

@ -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. // These fields keep track of where we are in the table.
int index; int index;
ReferenceEntry<K, V> entry; ReferenceEntry<K, V> next;
ReferenceEntry<K, V> previous; ReferenceEntry<K, V> current;
// These Object fields provide hard references to the // These Object fields provide hard references to the
// current and next entry; this assures that if hasNext() // 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() { public boolean hasNext() {
checkMod(); checkMod();
while (nextNull()) { while (nextNull()) {
ReferenceEntry<K, V> e = entry; ReferenceEntry<K, V> e = next;
int i = index; int i = index;
while (e == null && i > 0) { while (e == null && i > 0) {
i--; i--;
e = (ReferenceEntry<K, V>) parent.data[i]; e = (ReferenceEntry<K, V>) parent.data[i];
} }
entry = e; next = e;
index = i; index = i;
if (e == null) { if (e == null) {
currentKey = null;
currentValue = null;
return false; return false;
} }
nextKey = e.getKey(); nextKey = e.getKey();
nextValue = e.getValue(); nextValue = e.getValue();
if (nextNull()) { if (nextNull()) {
entry = entry.next(); next = next.next();
} }
} }
return true; return true;
@ -831,27 +829,27 @@ public abstract class AbstractReferenceMap<K, V> extends AbstractHashedMap<K, V>
if (nextNull() && !hasNext()) { if (nextNull() && !hasNext()) {
throw new NoSuchElementException(); throw new NoSuchElementException();
} }
previous = entry; current = next;
entry = entry.next(); next = next.next();
currentKey = nextKey; currentKey = nextKey;
currentValue = nextValue; currentValue = nextValue;
nextKey = null; nextKey = null;
nextValue = null; nextValue = null;
return previous; return current;
} }
protected ReferenceEntry<K, V> currentEntry() { protected ReferenceEntry<K, V> currentEntry() {
checkMod(); checkMod();
return previous; return current;
} }
public void remove() { public void remove() {
checkMod(); checkMod();
if (previous == null) { if (current == null) {
throw new IllegalStateException(); throw new IllegalStateException();
} }
parent.remove(currentKey); parent.remove(currentKey);
previous = null; current = null;
currentKey = null; currentKey = null;
currentValue = null; currentValue = null;
expectedModCount = parent.modCount; expectedModCount = parent.modCount;

View File

@ -26,6 +26,7 @@ import java.io.ObjectOutputStream;
import java.io.Serializable; import java.io.Serializable;
import java.lang.ref.WeakReference; import java.lang.ref.WeakReference;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.function.Consumer; 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") @SuppressWarnings("unused")
private static void gc() { private static void gc() {
try { try {