THRIFT-1468. java: Memory leak in TSaslServerTransport

This patch changes a particular map element to a WeakReference and thus avoids some awkward un-GC-ableness.

Patch: Mithun Radhakrishnan"

git-svn-id: https://svn.apache.org/repos/asf/thrift/trunk@1222397 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java b/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java
index 8abcf36..1a74d53 100644 (file)
--- a/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java
+++ b/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java
@@ -19,6 +19,7 @@
 
 package org.apache.thrift.transport;
 
+import java.lang.ref.WeakReference;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -158,8 +159,8 @@ public class TSaslServerTransport extends TSaslTransport {
      * This is the implementation of the awful hack described above.
      * <code>WeakHashMap</code> is used to ensure that we don't leak memory.
      */
-    private static Map<TTransport, TSaslServerTransport> transportMap =
-      Collections.synchronizedMap(new WeakHashMap<TTransport, TSaslServerTransport>());
+    private static Map<TTransport, WeakReference<TSaslServerTransport>> transportMap =
+      Collections.synchronizedMap(new WeakHashMap<TTransport, WeakReference<TSaslServerTransport>>());
 
     /**
      * Mapping from SASL mechanism name -> all the parameters required to
@@ -207,21 +208,22 @@ public class TSaslServerTransport extends TSaslTransport {
      */
     @Override
     public TTransport getTransport(TTransport base) {
-      TSaslServerTransport ret = transportMap.get(base);
-      if (ret == null) {
+      WeakReference<TSaslServerTransport> ret = transportMap.get(base);
+      if (ret == null || ret.get() == null) {
         LOGGER.debug("transport map does not contain key", base);
-        ret = new TSaslServerTransport(serverDefinitionMap, base);
+        ret = new WeakReference<TSaslServerTransport>(new TSaslServerTransport(serverDefinitionMap, base));
         try {
-          ret.open();
+          ret.get().open();
         } catch (TTransportException e) {
           LOGGER.debug("failed to open server transport", e);
           throw new RuntimeException(e);
         }
-        transportMap.put(base, ret);
+        transportMap.put(base, ret); // No need for putIfAbsent().
+                                     // Concurrent calls to getTransport() will pass in different TTransports.
       } else {
         LOGGER.debug("transport map does contain key {}", base);
       }
-      return ret;
+      return ret.get();
     }
   }
 }