Tuesday, July 29

IdentityHashMap is broken in IBM JDK 5!

So there I was, happily running some tests with the latest copy of Guice when blam - I got a NullPointerException from inside the injector. Hmmm, I thought, perhaps the latest trunk code is unstable. So I pulled down a stable snapshot... and got the same exception.

Looked at location of the NPE and got even more confused:

   CreationTimeMemberInjector.java:87

where it iterates over the cached entry set of an identity hash-map. Added some debug code, and yes - the entry set is valid to begin with, but after stashing it in a list, the list elements are all null. Weird!

Why hadn't I seen this before, I've run the same test many times - then it struck me, today I happened to have JAVA_HOME set to the IBM JDK, while previously I've used the default Sun JDK.

Time to crack open the src.jar from the IBM JDK - lo and behold in IdentityHashMap.java:1142

public T[] toArray(T[] a) {
return (T[])toArray(); // !!!!
}

that is plain wrong - the specification for toArray(T[] a) clearly states that the provided array must be used if it's large enough:

Returns an array containing all of the elements in this collection; the runtime type of the returned array is that of the specified array. If the collection fits in the specified array, it is returned therein. Otherwise, a new array is allocated with the runtime type of the specified array and the size of this collection.

this behaviour is relied on in other classes from java.util, where they create a large enough array and use c.toArray(elements); to initialize the array contents. Hence the null array when creating a list based on the entry set.

I was going to report this on the "IBM Java Runtimes and SDKs" forum as the problem still exists in the latest service release, but it appears to be offline today! For those interested, here is a simple testcase to recreate the bug:

import java.util.*;

public class IdentityHashMapTest {
@SuppressWarnings("unchecked")
public static void main(String[] args) {
Map map1 = new HashMap();
Map map2 = new IdentityHashMap();
map1.put("Hello", "World");
map2.put("Hello", "World");
System.out.println("MAP 1 : " + map1.entrySet());
System.out.println("MAP 2 : " + map2.entrySet());
System.out.println("LIST 1 : " + new ArrayList(map1.entrySet()));
System.out.println("LIST 2 : " + new ArrayList(map2.entrySet()));
}
}

BTW, that comment // !!!! is actually in the original source code, which suggests someone out there knows this is wrong but couldn't be bothered to fix it...

1 comment:

mcculls said...

Note that this appears to be fixed in their early access release of Java 6.