Skip to content

Commit

Permalink
[COLLECTIONS-802] Fix remove failed by removing set null to currentKe…
Browse files Browse the repository at this point in the history
…y and currentValue.
  • Loading branch information
samabcde committed Apr 24, 2022
1 parent 9df6f64 commit 43e23dd
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 13 deletions.
Expand Up @@ -772,8 +772,8 @@ static class ReferenceBaseIterator<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()
Expand All @@ -794,23 +794,21 @@ static class ReferenceBaseIterator<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;
Expand All @@ -831,27 +829,27 @@ protected ReferenceEntry<K, V> nextEntry() {
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;
Expand Down
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -315,6 +316,24 @@ public void testDataSizeAfterSerialization() throws IOException, ClassNotFoundEx

}

/**
* 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 {
Expand Down

0 comments on commit 43e23dd

Please sign in to comment.