svn commit: r1222397 - /thrift/trunk/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java

[br...@apache.org]

Author: bryanduxbury
Date: Thu Dec 22 19:04:16 2011
New Revision: 1222397

URL: http://svn.apache.org/viewvc?rev=1222397&view=rev
Log:
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"

Modified:
    thrift/trunk/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java

Modified: thrift/trunk/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java
URL: http://svn.apache.org/viewvc/thrift/trunk/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java?rev=1222397&r1=1222396&r2=1222397&view=diff
==============================================================================
--- thrift/trunk/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java (original)
+++ thrift/trunk/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java Thu Dec 22 19:04:16 2011
@@ -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 extend
      * 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 extend
      */
     @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();
     }
   }
 }