From e7bec408825d53bf9d3e8a10c8afd889cdb28814 Mon Sep 17 00:00:00 2001 From: Bryan Duxbury Date: Thu, 22 Dec 2011 19:04:16 +0000 Subject: [PATCH] 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 --- .../thrift/transport/TSaslServerTransport.java | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java b/lib/java/src/org/apache/thrift/transport/TSaslServerTransport.java index 8abcf360..1a74d53a 100644 --- 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. * WeakHashMap is used to ensure that we don't leak memory. */ - private static Map transportMap = - Collections.synchronizedMap(new WeakHashMap()); + private static Map> transportMap = + Collections.synchronizedMap(new WeakHashMap>()); /** * 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 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(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(); } } } -- 2.17.1