0xCAFEFEED

Rejeev made some interesting comments on my previous post at javalobby. The memory leak we had faced was with JDK 1.3_02. The java bug database has a couple of bugs reported for the sameBug: 4414045 and Bug: 4455134. As per JDK 1.4 beta release notes they are now fixed.Looking at JDK 1.4 ThreadLocal sources it appears that they have made the threadLocals Map hold weak references … which was as per my expectation! 

The ThreadLocal should have taken care of cleaning itself up when no user code is referring to it. The way to achieve this is to make the threadLocals map of the Thread class a WeakHashMap.

Second, consider the case where the Object being put in the Map [in this case a CachedObject] has overridden the equals and hashcode methods. Say, the hashcode method returns a different hashcode based on the current fields of the CachedObject class. Now, in the suggested implementation, this CachedObject is being put in a Map. What if the hashcode of the object changes [beacause the value of some field changed], while it is in the Map. We will not be able to remove the object from the Map. The memory leak will remain.

When we decided to make the ThreadLocal a static field from non-static field we changed the value of the ThreadLocal to a Map which holds the instance of CachedObject vs Context. The assumption was that the look up’s to the Map will be based on the instance of the key. However, making the Map an instance of a HashMap broke this assumption, as the lookup’s are based on the hashcode of key. We need a Map implementation that does instance-based lookups. One way to achieve this would be to change the following HashMap methods:

260       static int hash(Object x) {
261           /* 
262           int h = x.hashCode(); 
263
264           h += ~(h << 9); 
265           h ^=  (h >>> 14); 
266           h +=  (h << 4); 
267           h ^=  (h >>> 10); 
268           return h; 
269           */ 
270           return System.identityHashCode(x);
271       }
272
273       static boolean eq(Object x, Object y) {
274           //return x == y || x.equals(y);
275           return x==y;
276       } 

We change the hash method to return the identityHashCode of the object. The identityHashCodeof an Object is constant. We change the eq methodto do an instance check. However, since these are static methods we cannot override them and make these changes. So to achieve this we will have to make a copy of the HashMap and make the changes in it.

Another alternative is to wrap the key being added into the Map. This wrapper would return the identityHashCode of the key as the hashcode and check instance equals of the keys in its equals. So we write the InstanceMap and use an InstanceMap in the ThreadLocalMap instead of a HashMap.

1    import java.util.HashMap;
2
3    public class InstanceMap
4            extends HashMap
5    {
6        private IdentityWrapper lookup = new IdentityWrapper();
7
8        public Object get(Object key)
9        {
10           lookup.key=key;
11           return super.get(key);
12       }
13
14       public Object put(Object key, Object value)
15       {
16           return super.put(new IdentityWrapper(key), value);
17       }
18
19       public boolean containsKey(Object key)
20       {
21           lookup.key=key;
22           return super.containsKey(lookup);
23       }
24       //other overridden method not listed for brevity ...
25       private static class IdentityWrapper
26       {
27           private Object key;
28
29           public IdentityWrapper()
30           {
31           }
32
33           public IdentityWrapper(Object key)
34           {
35               this.key = key;
36           }
37
38           public int hashCode()
39           {
40               return System.identityHashCode(key);
41           }
42
43           public boolean equals(Object obj)
44           {
45               return (obj instanceof IdentityWrapper)?((IdentityWrapper)obj).key==key:false;
46           }
47       }
48   }

Since this class is being used only in our ThreadLocalMap, it will never be accessed by two threads. We have tried to optimize the number of the wrapper instances created by using a single wrapper [the field called lookup] for all the lookup methods like get and containsKey. However, every call to the put method will create a new instance and every remove call will make it GC’able.