* Renamed Concurrent*Weak*HashMap to Concurrent*WeakKey*HashMap to avoid confusion

* Added some FIXMEs related with potential memory leak

src/main/java/org/jboss/netty/channel/ChannelLocal.java

@@ -24,7 +24,7 @@ package org.jboss.netty.channel;
 
 import java.util.concurrent.ConcurrentMap;
 
-import org.jboss.netty.util.ConcurrentIdentityWeakHashMap;
+import org.jboss.netty.util.ConcurrentIdentityWeakKeyHashMap;
 
 /**
  * @author The Netty Project (netty-dev@lists.jboss.org)
@@ -35,7 +35,7 @@ import org.jboss.netty.util.ConcurrentIdentityWeakHashMap;
  */
 public class ChannelLocal<T> {
     private final ConcurrentMap<Channel, T> map =
-        new ConcurrentIdentityWeakHashMap<Channel, T>();
+        new ConcurrentIdentityWeakKeyHashMap<Channel, T>();
 
     /**
      * Creates a {@link Channel} local variable.

src/main/java/org/jboss/netty/channel/group/ChannelGroupFactory.java

@@ -37,6 +37,7 @@ import org.jboss.netty.util.ConcurrentHashMap;
  */
 public class ChannelGroupFactory {
 
+    // FIXME: Memory leak - use ConcurrentWeakValueHashMap
     private static final ConcurrentMap<String, ChannelGroup> groups =
         new ConcurrentHashMap<String, ChannelGroup>();
 

src/main/java/org/jboss/netty/channel/local/LocalAddress.java

@@ -26,7 +26,7 @@ import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.atomic.AtomicInteger;
 
 import org.jboss.netty.channel.ChannelException;
-import org.jboss.netty.util.ConcurrentWeakHashMap;
+import org.jboss.netty.util.ConcurrentHashMap;
 
 /**
  * @author The Netty Project (netty-dev@lists.jboss.org)
@@ -37,8 +37,9 @@ import org.jboss.netty.util.ConcurrentWeakHashMap;
 public final class LocalAddress extends SocketAddress implements Comparable<LocalAddress> {
     private static final long serialVersionUID = -3601961747680808645L;
 
+    // FIXME: Memory leak - use ConcurrentWeakValueHashMap
     private static final ConcurrentMap<String, LocalAddress> addresses =
-        new ConcurrentWeakHashMap<String, LocalAddress>();
+        new ConcurrentHashMap<String, LocalAddress>();
 
     private static final AtomicInteger nextEphemeralPort = new AtomicInteger();
 

src/main/java/org/jboss/netty/handler/execution/DefaultObjectSizeEstimator.java

@@ -31,7 +31,7 @@ import java.util.concurrent.ConcurrentMap;
 
 import org.jboss.netty.buffer.ChannelBuffer;
 import org.jboss.netty.channel.MessageEvent;
-import org.jboss.netty.util.ConcurrentHashMap;
+import org.jboss.netty.util.ConcurrentIdentityWeakKeyHashMap;
 
 /**
  * The default {@link ObjectSizeEstimator} implementation for general purpose.
@@ -45,7 +45,7 @@ import org.jboss.netty.util.ConcurrentHashMap;
 public class DefaultObjectSizeEstimator implements ObjectSizeEstimator {
 
     private final ConcurrentMap<Class<?>, Integer> class2size =
-        new ConcurrentHashMap<Class<?>, Integer>();
+        new ConcurrentIdentityWeakKeyHashMap<Class<?>, Integer>();
 
     /**
      * Creates a new instance.

src/main/java/org/jboss/netty/util/ConcurrentIdentityWeakKeyHashMap.java

@@ -58,7 +58,7 @@ import java.util.concurrent.locks.ReentrantLock;
  * @param <K> the type of keys maintained by this map
  * @param <V> the type of mapped values
  */
-public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V> implements ConcurrentMap<K, V> {
+public final class ConcurrentIdentityWeakKeyHashMap<K, V> extends AbstractMap<K, V> implements ConcurrentMap<K, V> {
 
     /*
      * The basic strategy is to subdivide the table among Segments,
@@ -672,7 +672,7 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
      *                                  the load factor or concurrencyLevel are
      *                                  nonpositive.
      */
-    public ConcurrentIdentityWeakHashMap(
+    public ConcurrentIdentityWeakKeyHashMap(
             int initialCapacity, float loadFactor, int concurrencyLevel) {
         if (!(loadFactor > 0) || initialCapacity < 0 || concurrencyLevel <= 0) {
             throw new IllegalArgumentException();
@@ -724,7 +724,7 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
      *                                  negative or the load factor is
      *                                  nonpositive
      */
-    public ConcurrentIdentityWeakHashMap(int initialCapacity, float loadFactor) {
+    public ConcurrentIdentityWeakKeyHashMap(int initialCapacity, float loadFactor) {
         this(initialCapacity, loadFactor, DEFAULT_CONCURRENCY_LEVEL);
     }
 
@@ -738,7 +738,7 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
      * @throws IllegalArgumentException if the initial capacity of elements is
      *                                  negative.
      */
-    public ConcurrentIdentityWeakHashMap(int initialCapacity) {
+    public ConcurrentIdentityWeakKeyHashMap(int initialCapacity) {
         this(initialCapacity, DEFAULT_LOAD_FACTOR, DEFAULT_CONCURRENCY_LEVEL);
     }
 
@@ -747,7 +747,7 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
      * types (weak keys, strong values), default load factor (0.75) and
      * concurrencyLevel (16).
      */
-    public ConcurrentIdentityWeakHashMap() {
+    public ConcurrentIdentityWeakKeyHashMap() {
         this(DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR, DEFAULT_CONCURRENCY_LEVEL);
     }
 
@@ -759,7 +759,7 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
      *
      * @param m the map
      */
-    public ConcurrentIdentityWeakHashMap(Map<? extends K, ? extends V> m) {
+    public ConcurrentIdentityWeakKeyHashMap(Map<? extends K, ? extends V> m) {
         this(Math.max((int) (m.size() / DEFAULT_LOAD_FACTOR) + 1,
              DEFAULT_INITIAL_CAPACITY), DEFAULT_LOAD_FACTOR,
              DEFAULT_CONCURRENCY_LEVEL);
@@ -1273,7 +1273,7 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
             if (lastReturned == null) {
                 throw new IllegalStateException();
             }
-            ConcurrentIdentityWeakHashMap.this.remove(currentKey);
+            ConcurrentIdentityWeakKeyHashMap.this.remove(currentKey);
             lastReturned = null;
         }
     }
@@ -1391,7 +1391,7 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
                 throw new NullPointerException();
             }
             V v = super.setValue(value);
-            ConcurrentIdentityWeakHashMap.this.put(getKey(), value);
+            ConcurrentIdentityWeakKeyHashMap.this.put(getKey(), value);
             return v;
         }
 
@@ -1414,28 +1414,28 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
 
         @Override
         public int size() {
-            return ConcurrentIdentityWeakHashMap.this.size();
+            return ConcurrentIdentityWeakKeyHashMap.this.size();
         }
 
         @Override
         public boolean isEmpty() {
-            return ConcurrentIdentityWeakHashMap.this.isEmpty();
+            return ConcurrentIdentityWeakKeyHashMap.this.isEmpty();
         }
 
         @Override
         public boolean contains(Object o) {
-            return ConcurrentIdentityWeakHashMap.this.containsKey(o);
+            return ConcurrentIdentityWeakKeyHashMap.this.containsKey(o);
         }
 
         @Override
         public boolean remove(Object o) {
-            return ConcurrentIdentityWeakHashMap.this.remove(o) != null;
+            return ConcurrentIdentityWeakKeyHashMap.this.remove(o) != null;
 
         }
 
         @Override
         public void clear() {
-            ConcurrentIdentityWeakHashMap.this.clear();
+            ConcurrentIdentityWeakKeyHashMap.this.clear();
         }
     }
 
@@ -1447,22 +1447,22 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
 
         @Override
         public int size() {
-            return ConcurrentIdentityWeakHashMap.this.size();
+            return ConcurrentIdentityWeakKeyHashMap.this.size();
         }
 
         @Override
         public boolean isEmpty() {
-            return ConcurrentIdentityWeakHashMap.this.isEmpty();
+            return ConcurrentIdentityWeakKeyHashMap.this.isEmpty();
         }
 
         @Override
         public boolean contains(Object o) {
-            return ConcurrentIdentityWeakHashMap.this.containsValue(o);
+            return ConcurrentIdentityWeakKeyHashMap.this.containsValue(o);
         }
 
         @Override
         public void clear() {
-            ConcurrentIdentityWeakHashMap.this.clear();
+            ConcurrentIdentityWeakKeyHashMap.this.clear();
         }
     }
 
@@ -1478,7 +1478,7 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
                 return false;
             }
             Map.Entry<?, ?> e = (Map.Entry<?, ?>) o;
-            V v = ConcurrentIdentityWeakHashMap.this.get(e.getKey());
+            V v = ConcurrentIdentityWeakKeyHashMap.this.get(e.getKey());
             return v != null && v.equals(e.getValue());
         }
 
@@ -1488,22 +1488,22 @@ public final class ConcurrentIdentityWeakHashMap<K, V> extends AbstractMap<K, V>
                 return false;
             }
             Map.Entry<?, ?> e = (Map.Entry<?, ?>) o;
-            return ConcurrentIdentityWeakHashMap.this.remove(e.getKey(), e.getValue());
+            return ConcurrentIdentityWeakKeyHashMap.this.remove(e.getKey(), e.getValue());
         }
 
         @Override
         public int size() {
-            return ConcurrentIdentityWeakHashMap.this.size();
+            return ConcurrentIdentityWeakKeyHashMap.this.size();
         }
 
         @Override
         public boolean isEmpty() {
-            return ConcurrentIdentityWeakHashMap.this.isEmpty();
+            return ConcurrentIdentityWeakKeyHashMap.this.isEmpty();
         }
 
         @Override
         public void clear() {
-            ConcurrentIdentityWeakHashMap.this.clear();
+            ConcurrentIdentityWeakKeyHashMap.this.clear();
         }
     }
 }

src/main/java/org/jboss/netty/util/ConcurrentWeakKeyHashMap.java

@@ -58,7 +58,7 @@ import java.util.concurrent.locks.ReentrantLock;
  * @param <K> the type of keys maintained by this map
  * @param <V> the type of mapped values
  */
-public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> implements ConcurrentMap<K, V> {
+public final class ConcurrentWeakKeyHashMap<K, V> extends AbstractMap<K, V> implements ConcurrentMap<K, V> {
 
     /*
      * The basic strategy is to subdivide the table among Segments,
@@ -672,7 +672,7 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
      *                                  the load factor or concurrencyLevel are
      *                                  nonpositive.
      */
-    public ConcurrentWeakHashMap(
+    public ConcurrentWeakKeyHashMap(
             int initialCapacity, float loadFactor, int concurrencyLevel) {
         if (!(loadFactor > 0) || initialCapacity < 0 || concurrencyLevel <= 0) {
             throw new IllegalArgumentException();
@@ -724,7 +724,7 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
      *                                  negative or the load factor is
      *                                  nonpositive
      */
-    public ConcurrentWeakHashMap(int initialCapacity, float loadFactor) {
+    public ConcurrentWeakKeyHashMap(int initialCapacity, float loadFactor) {
         this(initialCapacity, loadFactor, DEFAULT_CONCURRENCY_LEVEL);
     }
 
@@ -738,7 +738,7 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
      * @throws IllegalArgumentException if the initial capacity of elements is
      *                                  negative.
      */
-    public ConcurrentWeakHashMap(int initialCapacity) {
+    public ConcurrentWeakKeyHashMap(int initialCapacity) {
         this(initialCapacity, DEFAULT_LOAD_FACTOR, DEFAULT_CONCURRENCY_LEVEL);
     }
 
@@ -747,7 +747,7 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
      * types (weak keys, strong values), default load factor (0.75) and
      * concurrencyLevel (16).
      */
-    public ConcurrentWeakHashMap() {
+    public ConcurrentWeakKeyHashMap() {
         this(DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR, DEFAULT_CONCURRENCY_LEVEL);
     }
 
@@ -759,7 +759,7 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
      *
      * @param m the map
      */
-    public ConcurrentWeakHashMap(Map<? extends K, ? extends V> m) {
+    public ConcurrentWeakKeyHashMap(Map<? extends K, ? extends V> m) {
         this(Math.max((int) (m.size() / DEFAULT_LOAD_FACTOR) + 1,
              DEFAULT_INITIAL_CAPACITY), DEFAULT_LOAD_FACTOR,
              DEFAULT_CONCURRENCY_LEVEL);
@@ -1273,7 +1273,7 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
             if (lastReturned == null) {
                 throw new IllegalStateException();
             }
-            ConcurrentWeakHashMap.this.remove(currentKey);
+            ConcurrentWeakKeyHashMap.this.remove(currentKey);
             lastReturned = null;
         }
     }
@@ -1391,7 +1391,7 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
                 throw new NullPointerException();
             }
             V v = super.setValue(value);
-            ConcurrentWeakHashMap.this.put(getKey(), value);
+            ConcurrentWeakKeyHashMap.this.put(getKey(), value);
             return v;
         }
 
@@ -1414,28 +1414,28 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
 
         @Override
         public int size() {
-            return ConcurrentWeakHashMap.this.size();
+            return ConcurrentWeakKeyHashMap.this.size();
         }
 
         @Override
         public boolean isEmpty() {
-            return ConcurrentWeakHashMap.this.isEmpty();
+            return ConcurrentWeakKeyHashMap.this.isEmpty();
         }
 
         @Override
         public boolean contains(Object o) {
-            return ConcurrentWeakHashMap.this.containsKey(o);
+            return ConcurrentWeakKeyHashMap.this.containsKey(o);
         }
 
         @Override
         public boolean remove(Object o) {
-            return ConcurrentWeakHashMap.this.remove(o) != null;
+            return ConcurrentWeakKeyHashMap.this.remove(o) != null;
 
         }
 
         @Override
         public void clear() {
-            ConcurrentWeakHashMap.this.clear();
+            ConcurrentWeakKeyHashMap.this.clear();
         }
     }
 
@@ -1447,22 +1447,22 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
 
         @Override
         public int size() {
-            return ConcurrentWeakHashMap.this.size();
+            return ConcurrentWeakKeyHashMap.this.size();
         }
 
         @Override
         public boolean isEmpty() {
-            return ConcurrentWeakHashMap.this.isEmpty();
+            return ConcurrentWeakKeyHashMap.this.isEmpty();
         }
 
         @Override
         public boolean contains(Object o) {
-            return ConcurrentWeakHashMap.this.containsValue(o);
+            return ConcurrentWeakKeyHashMap.this.containsValue(o);
         }
 
         @Override
         public void clear() {
-            ConcurrentWeakHashMap.this.clear();
+            ConcurrentWeakKeyHashMap.this.clear();
         }
     }
 
@@ -1478,7 +1478,7 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
                 return false;
             }
             Map.Entry<?, ?> e = (Map.Entry<?, ?>) o;
-            V v = ConcurrentWeakHashMap.this.get(e.getKey());
+            V v = ConcurrentWeakKeyHashMap.this.get(e.getKey());
             return v != null && v.equals(e.getValue());
         }
 
@@ -1488,22 +1488,22 @@ public final class ConcurrentWeakHashMap<K, V> extends AbstractMap<K, V> impleme
                 return false;
             }
             Map.Entry<?, ?> e = (Map.Entry<?, ?>) o;
-            return ConcurrentWeakHashMap.this.remove(e.getKey(), e.getValue());
+            return ConcurrentWeakKeyHashMap.this.remove(e.getKey(), e.getValue());
         }
 
         @Override
         public int size() {
-            return ConcurrentWeakHashMap.this.size();
+            return ConcurrentWeakKeyHashMap.this.size();
         }
 
         @Override
         public boolean isEmpty() {
-            return ConcurrentWeakHashMap.this.isEmpty();
+            return ConcurrentWeakKeyHashMap.this.isEmpty();
         }
 
         @Override
         public void clear() {
-            ConcurrentWeakHashMap.this.clear();
+            ConcurrentWeakKeyHashMap.this.clear();
         }
     }
 }