Skip to content

Commit

Permalink
6312706: Map entrySet iterators should return different entries on ea…
Browse files Browse the repository at this point in the history 
…ch call to next()

Reviewed-by: mduigou, alanb
Contributed-by: Neil Richards <neil.richards@ngmr.net>
  • Loading branch information
mduigou committed Apr 6, 2011
1 parent 57fd2c7 commit 39048f2
Show file tree
Hide file tree
Showing 8 changed files with 568 additions and 107 deletions.
176 changes: 119 additions & 57 deletions src/share/classes/java/util/EnumMap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public class EnumMap<K extends Enum<K>, V> extends AbstractMap<K, V>
/**
* Distinguished non-null value for representing null values.
*/
private static final Object NULL = new Object();
private static final Object NULL = new Integer(0);

private Object maskNull(Object value) {
return (value == null ? NULL : value);
Expand All @@ -116,7 +116,7 @@ private V unmaskNull(Object value) {
return (V) (value == NULL ? null : value);
}

private static Enum[] ZERO_LENGTH_ENUM_ARRAY = new Enum[0];
private static final Enum[] ZERO_LENGTH_ENUM_ARRAY = new Enum[0];

/**
* Creates an empty enum map with the specified key type.
Expand Down Expand Up @@ -464,6 +464,7 @@ private class EntrySet extends AbstractSet<Map.Entry<K,V>> {
public Iterator<Map.Entry<K,V>> iterator() {
return new EntryIterator();
}

public boolean contains(Object o) {
if (!(o instanceof Map.Entry))
return false;
Expand Down Expand Up @@ -552,70 +553,82 @@ public V next() {
}
}

/**
* Since we don't use Entry objects, we use the Iterator itself as entry.
*/
private class EntryIterator extends EnumMapIterator<Map.Entry<K,V>>
implements Map.Entry<K,V>
{
private class EntryIterator extends EnumMapIterator<Map.Entry<K,V>> {
private Entry lastReturnedEntry = null;

public Map.Entry<K,V> next() {
if (!hasNext())
throw new NoSuchElementException();
lastReturnedIndex = index++;
return this;
lastReturnedEntry = new Entry(index++);
return lastReturnedEntry;
}

public K getKey() {
checkLastReturnedIndexForEntryUse();
return keyUniverse[lastReturnedIndex];
public void remove() {
lastReturnedIndex =
((null == lastReturnedEntry) ? -1 : lastReturnedEntry.index);
super.remove();
lastReturnedEntry.index = lastReturnedIndex;
lastReturnedEntry = null;
}

public V getValue() {
checkLastReturnedIndexForEntryUse();
return unmaskNull(vals[lastReturnedIndex]);
}
private class Entry implements Map.Entry<K,V> {
private int index;

public V setValue(V value) {
checkLastReturnedIndexForEntryUse();
V oldValue = unmaskNull(vals[lastReturnedIndex]);
vals[lastReturnedIndex] = maskNull(value);
return oldValue;
}
private Entry(int index) {
this.index = index;
}

public boolean equals(Object o) {
if (lastReturnedIndex < 0)
return o == this;
public K getKey() {
checkIndexForEntryUse();
return keyUniverse[index];
}

if (!(o instanceof Map.Entry))
return false;
Map.Entry e = (Map.Entry)o;
V ourValue = unmaskNull(vals[lastReturnedIndex]);
Object hisValue = e.getValue();
return e.getKey() == keyUniverse[lastReturnedIndex] &&
(ourValue == hisValue ||
(ourValue != null && ourValue.equals(hisValue)));
}
public V getValue() {
checkIndexForEntryUse();
return unmaskNull(vals[index]);
}

public int hashCode() {
if (lastReturnedIndex < 0)
return super.hashCode();
public V setValue(V value) {
checkIndexForEntryUse();
V oldValue = unmaskNull(vals[index]);
vals[index] = maskNull(value);
return oldValue;
}

Object value = vals[lastReturnedIndex];
return keyUniverse[lastReturnedIndex].hashCode()
^ (value == NULL ? 0 : value.hashCode());
}
public boolean equals(Object o) {
if (index < 0)
return o == this;

public String toString() {
if (lastReturnedIndex < 0)
return super.toString();
if (!(o instanceof Map.Entry))
return false;

return keyUniverse[lastReturnedIndex] + "="
+ unmaskNull(vals[lastReturnedIndex]);
}
Map.Entry e = (Map.Entry)o;
V ourValue = unmaskNull(vals[index]);
Object hisValue = e.getValue();
return (e.getKey() == keyUniverse[index] &&
(ourValue == hisValue ||
(ourValue != null && ourValue.equals(hisValue))));
}

private void checkLastReturnedIndexForEntryUse() {
if (lastReturnedIndex < 0)
throw new IllegalStateException("Entry was removed");
public int hashCode() {
if (index < 0)
return super.hashCode();

return entryHashCode(index);
}

public String toString() {
if (index < 0)
return super.toString();

return keyUniverse[index] + "="
+ unmaskNull(vals[index]);
}

private void checkIndexForEntryUse() {
if (index < 0)
throw new IllegalStateException("Entry was removed");
}
}
}

Expand All @@ -631,10 +644,35 @@ private void checkLastReturnedIndexForEntryUse() {
* @return <tt>true</tt> if the specified object is equal to this map
*/
public boolean equals(Object o) {
if (!(o instanceof EnumMap))
return super.equals(o);
if (this == o)
return true;
if (o instanceof EnumMap)
return equals((EnumMap)o);
if (!(o instanceof Map))
return false;

Map<K,V> m = (Map<K,V>)o;
if (size != m.size())
return false;

for (int i = 0; i < keyUniverse.length; i++) {
if (null != vals[i]) {
K key = keyUniverse[i];
V value = unmaskNull(vals[i]);
if (null == value) {
if (!((null == m.get(key)) && m.containsKey(key)))
return false;
} else {
if (!value.equals(m.get(key)))
return false;
}
}
}

return true;
}

EnumMap em = (EnumMap)o;
private boolean equals(EnumMap em) {
if (em.keyType != keyType)
return size == 0 && em.size == 0;

Expand All @@ -649,6 +687,26 @@ public boolean equals(Object o) {
return true;
}

/**
* Returns the hash code value for this map. The hash code of a map is
* defined to be the sum of the hash codes of each entry in the map.
*/
public int hashCode() {
int h = 0;

for (int i = 0; i < keyUniverse.length; i++) {
if (null != vals[i]) {
h += entryHashCode(i);
}
}

return h;
}

private int entryHashCode(int index) {
return (keyUniverse[index].hashCode() ^ vals[index].hashCode());
}

/**
* Returns a shallow copy of this enum map. (The values themselves
* are not cloned.
Expand Down Expand Up @@ -705,9 +763,13 @@ private void writeObject(java.io.ObjectOutputStream s)
s.writeInt(size);

// Write out keys and values (alternating)
for (Map.Entry<K,V> e : entrySet()) {
s.writeObject(e.getKey());
s.writeObject(e.getValue());
int entriesToBeWritten = size;
for (int i = 0; entriesToBeWritten > 0; i++) {
if (null != vals[i]) {
s.writeObject(keyUniverse[i]);
s.writeObject(unmaskNull(vals[i]));
entriesToBeWritten--;
}
}
}

Expand Down
111 changes: 61 additions & 50 deletions src/share/classes/java/util/IdentityHashMap.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2000, 2008, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -829,71 +829,82 @@ public V next() {
}
}

/**
* Since we don't use Entry objects, we use the Iterator
* itself as an entry.
*/
private class EntryIterator
extends IdentityHashMapIterator<Map.Entry<K,V>>
implements Map.Entry<K,V>
{
private Entry lastReturnedEntry = null;

public Map.Entry<K,V> next() {
nextIndex();
return this;
lastReturnedEntry = new Entry(nextIndex());
return lastReturnedEntry;
}

public K getKey() {
// Provide a better exception than out of bounds index
if (lastReturnedIndex < 0)
throw new IllegalStateException("Entry was removed");

return (K) unmaskNull(traversalTable[lastReturnedIndex]);
public void remove() {
lastReturnedIndex =
((null == lastReturnedEntry) ? -1 : lastReturnedEntry.index);
super.remove();
lastReturnedEntry.index = lastReturnedIndex;
lastReturnedEntry = null;
}

public V getValue() {
// Provide a better exception than out of bounds index
if (lastReturnedIndex < 0)
throw new IllegalStateException("Entry was removed");
private class Entry implements Map.Entry<K,V> {
private int index;

return (V) traversalTable[lastReturnedIndex+1];
}
private Entry(int index) {
this.index = index;
}

public V setValue(V value) {
// It would be mean-spirited to proceed here if remove() called
if (lastReturnedIndex < 0)
throw new IllegalStateException("Entry was removed");
V oldValue = (V) traversalTable[lastReturnedIndex+1];
traversalTable[lastReturnedIndex+1] = value;
// if shadowing, force into main table
if (traversalTable != IdentityHashMap.this.table)
put((K) traversalTable[lastReturnedIndex], value);
return oldValue;
}
public K getKey() {
checkIndexForEntryUse();
return (K) unmaskNull(traversalTable[index]);
}

public boolean equals(Object o) {
if (lastReturnedIndex < 0)
return super.equals(o);
public V getValue() {
checkIndexForEntryUse();
return (V) traversalTable[index+1];
}

if (!(o instanceof Map.Entry))
return false;
Map.Entry e = (Map.Entry)o;
return e.getKey() == getKey() &&
e.getValue() == getValue();
}
public V setValue(V value) {
checkIndexForEntryUse();
V oldValue = (V) traversalTable[index+1];
traversalTable[index+1] = value;
// if shadowing, force into main table
if (traversalTable != IdentityHashMap.this.table)
put((K) traversalTable[index], value);
return oldValue;
}

public int hashCode() {
if (lastReturnedIndex < 0)
return super.hashCode();
public boolean equals(Object o) {
if (index < 0)
return super.equals(o);

return System.identityHashCode(getKey()) ^
System.identityHashCode(getValue());
}
if (!(o instanceof Map.Entry))
return false;
Map.Entry e = (Map.Entry)o;
return (e.getKey() == unmaskNull(traversalTable[index]) &&
e.getValue() == traversalTable[index+1]);
}

public String toString() {
if (lastReturnedIndex < 0)
return super.toString();
public int hashCode() {
if (lastReturnedIndex < 0)
return super.hashCode();

return getKey() + "=" + getValue();
return (System.identityHashCode(unmaskNull(traversalTable[index])) ^
System.identityHashCode(traversalTable[index+1]));
}

public String toString() {
if (index < 0)
return super.toString();

return (unmaskNull(traversalTable[index]) + "="
+ traversalTable[index+1]);
}

private void checkIndexForEntryUse() {
if (index < 0)
throw new IllegalStateException("Entry was removed");
}
}
}

Expand Down
Loading

0 comments on commit 39048f2

Please sign in to comment.