Name: vi73552 Date: 05/07/99


Recommend improving the Iterator regression tests to catch the
misbehaviors in WeakHashMap.entrySet().iterator() illustrated
by the following code.

import java.util.*;

/** The iterator returned by WeakHashMap.entrySet().iterator() has bugs:<ul>
   <li> hasNext() advances iterator.
   <li> remove will remove wrong one if hasNext has been called. **/
/**/public class WeakHashMapEntrySetIteratorBugs {
/**/ public static void main(String[] ignore) {
/**/ testMap(fillTestMap(new WeakHashMap()));
/**/ testMap(fillTestMap(new HashMap()));
/**/ }
/**/ static void testMap(Map map) {
/**/ System.out.println("\nMap: "+map.getClass().getName()+map);
/**/ System.out.println();
/**/ System.out.println(". With 1 call to hasNext per item:");
/**/ System.out.print (". Test Result: ");
/**/ for(Iterator iter = map.entrySet().iterator(); iter.hasNext(); ) {
/**/ Map.Entry entry = (Map.Entry) iter.next();
/**/ System.out.print(entry.getKey() + "=" + entry.getValue() + " ");
/**/ }
/**/ System.out.println();
/**/ System.out.println(". Expected : C=2 B=1 A=0");
/**/ System.out.println();
/**/ System.out.println(". Does iter.hasNext() advance iterator?");
/**/ System.out.println(". 2 hasNext calls for Commas between items");
/**/ System.out.print (". Test Result: ");
/**/ for(Iterator iter = map.entrySet().iterator(); iter.hasNext(); ) {
/**/ Map.Entry entry = (Map.Entry) iter.next();
/**/ System.out.print(entry.getKey() + "=" + entry.getValue());
/**/ if (iter.hasNext()) System.out.print(", ");
/**/ }
/**/ System.out.println();
/**/ System.out.println(". Expected : C=2, B=1, A=0");
/**/ System.out.println();
/**/ System.out.println(". Does iter.remove() remove correctly?");
/**/ System.out.println(". 2 hasNext calls for removing all but last");
/**/ for(Iterator iter = map.entrySet().iterator(); iter.hasNext(); ) {
/**/ Map.Entry entry = (Map.Entry) iter.next();
/**/ if (iter.hasNext()) {
/**/ System.out.println(". Removing "+
/**/ entry.getKey() + "=" + entry.getValue());
/**/ iter.remove();
/**/ }
/**/ }
/**/ System.out.println(". Test Result: "+map);
/**/ System.out.println(". Expected : {A=0}");
/**/ }
/**/ // use static var so keys aren't gc'ed for this test.
/**/ static Object[] keys = new Object[] {"A", "B", "C"};
/**/ static Map fillTestMap(Map map) {
/**/ for (int i = 0; i < keys.length; i++) map.put(keys[i], ""+i);
/**/ return map;
/**/ }
/**/}


/* Output:
Map: java.util.WeakHashMap{C=2, B=1, A=0}

. With 1 call to hasNext per item:
. Test Result: C=2 B=1 A=0
. Expected : C=2 B=1 A=0

. Does iter.hasNext() advance iterator?
. 2 hasNext calls for Commas between items
. Test Result: C=2, A=0
. Expected : C=2, B=1, A=0

. Does iter.remove() remove correctly?
. 2 hasNext calls for removing all but last
. Removing C=2
. Test Result: {C=2, A=0}
. Expected : {A=0}

Map: java.util.HashMap{C=2, B=1, A=0}

. With 1 call to hasNext per item:
. Test Result: C=2 B=1 A=0
. Expected : C=2 B=1 A=0

. Does iter.hasNext() advance iterator?
. 2 hasNext calls for Commas between items
. Test Result: C=2, B=1, A=0
. Expected : C=2, B=1, A=0

. Does iter.remove() remove correctly?
. 2 hasNext calls for removing all but last
. Removing C=2
. Removing B=1
. Test Result: {A=0}
. Expected : {A=0}

*/
(Review ID: 57463)
======================================================================

Name: krT82822 Date: 10/07/99


See the description for bug number 4236533 for a detailed
explanation of the problem.

The first issue, that hasNext() advances the iterator if called
a second time, can be corrected by checking the next field for
a non-null value. If one is found, then hasNext() has already
been called once and determined that there is a next object
available.

The problem with remove() is corrected by storing the WeakKey
for the object that was returned by the last call to next().
  To remove the mapping, call enqueue() and clear() on the WeakKey
so that it will be removed from the underlying hashtable the next
time processQueue() is invoked.

For convenience, I added a new method to the Entry class called
"getWeakKey" to retrieve the WeakKey for the map entry. Its declared
package private so it doesn't modify the class' public API. (It
could have been private but the compiler would just add a package
private accessor anyway.)

PS - I think my IDE did something funny with the whitespace in
the source code. diff is reporting changed lines that really didn't change.

TEST CASE:
See the test case provided in bug #4236533. I've tested my changes
and they pass this test case.
RELATED BUGS:
4236533
(Review ID: 96253)
======================================================================