American in Spain

Bug in java.util.EnumMap.clone()

May 8, 2009

At work today we found ourselves totally confused by the behavior of one of our EnumMap instances. No matter what we did to it (setting new values, clearing, etc.), when we iterated through the entry set, the values were the same. After investigating the source code for java.util.EnumMap, it became clear that the problem was based on the fact that the EnumMap instance we were working with had been generated by calling EnumMap.clone() rather than using the constructor. I've written a test program to demonstrate this bug. It should NOT behave like this! code span.keyword { font-weight:bold; color: #000080; } code span.member { font-weight:bold; color: #660e7a; } code span.static { font-style:italic; font-weight:bold; color: #660e7a; } code span.comment { font-style:italic; color: #006600; }

The Code

public class EnumMapBug {   private static enum E { A,B,C }   public static void main(final String[] args)   {     final EnumMap<E ,String> map =         new EnumMap<E, String>(E.class);     map.put(E.A, "apple");     map.put(E.B, "ball");     map.put(E.C, "cat");     // call entrySet() to generate internal entrySet     map.entrySet();     // print original values     System.out.println("\nOriginal values:");     System.out.println("\nA is for " + map.get(E.A));     System.out.println("Entries: " + map.entrySet());     // clone the map.     System.out.println("\nCloning...now using clone.");     final EnumMap<E, String> clone = map.clone();     // from now on, any time we access the clone's entry set,     // it will WRONGLY give us the original map's entry set.     System.out.println("\nA is for " + clone.get(E.A));     System.out.println("Entries: " + clone.entrySet());     // no matter what we do to the clone's values,     // the clone's entry set never changes.     System.out.println("\nSetting new A value...");     clone.put(E.A, "aardvark");     // but get() still works     System.out.println("\nA is for " + clone.get(E.A));     System.out.println("Entries: " + clone.entrySet());     // clear the clone     System.out.println("\nClearing...");     clone.clear();     // get() properly returns null     System.out.println("\nA is for " + clone.get(E.A));     // but the entry set still contains our original entries     System.out.println("Entries: " + clone.entrySet());     // rather amazingly, when we modify the original     // map's contents, the entry set for the clone changes     System.out.println("\nSetting new A value...");     map.put(E.A, "algorithm");     System.out.println("\nA is for " + clone.get(E.A));     System.out.println("Entries: " + clone.entrySet());   } } There is a more easily copied and pasted version of this code here.

The Output

The code above results in the following output:

Original values: A is for apple Entries: [A=apple, B=ball, C=cat] Cloning...now using clone. A is for apple Entries: [A=apple, B=ball, C=cat] Setting new A value... A is for aardvark Entries: [A=apple, B=ball, C=cat] Clearing... A is for null Entries: [A=apple, B=ball, C=cat] Updating original map... A is for null Entries: [A=algorithm, B=ball, C=cat]

Explanation

When I said "rather amazingly" in the comment, it's only really amazing when you don't understand what is going on. When you clone an EnumMap, because of the entry set caching and the bug in clone(), the cloned instance's entrySet() method will always give you a reference to the original map's entry set, never to its own. The version of EnumMap that I'm using contains the javadoc info "@version 1.22, 05/05/07". It's clear enough to see the bug in clone(). The EnumMap clone() method is:

public EnumMap<K, V> clone() {   EnumMap<K, V> result = null;   try {     result = (EnumMap<K, V>) super.clone();   } catch(CloneNotSupportedException e) {     throw new AssertionError();   }   result.vals = (Object[]) result.vals.clone();   return result; } But it needs one more line:

public EnumMap<K, V> clone() {   EnumMap<K, V> result = null;   try {     result = (EnumMap<K, V>) super.clone();   } catch(CloneNotSupportedException e) {     throw new AssertionError();   }   result.vals = (Object[]) result.vals.clone();   result.entrySet = null;   return result; } This will force regeneration and caching of the entry set in the clone.

I'm rather surprised that I cannot find any reference to this bug on the internet. It's such an easy fix! The workaround is to never use EnumMap.clone(), but to construct a new one with its copy constructor. Like so, with the key and value in my example code:

final EnumMap<E, String> clone = new EnumMap<E, String>(map);