QPID-5172 : Thread safety issue in StringTypeConstructor.construct and SymbolTypeConstructor.construct (patch from dingham@microsoft.com)

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

2 files changed, 9 insertions, 62 deletions

diff --git a/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/StringTypeConstructor.java b/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/StringTypeConstructor.java
index e89f038f23..cc788f72c5 100644
--- a/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/StringTypeConstructor.java
+++ b/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/StringTypeConstructor.java
@@ -32,30 +32,6 @@ public class StringTypeConstructor extends VariableWidthTypeConstructor
 {
     private Charset _charSet;
 
-    private BinaryString _defaultBinaryString = new BinaryString();
-    private ValueCache<BinaryString, String> _cachedValues = new ValueCache<BinaryString, String>(10);
-
-    private static final class ValueCache<K,V> extends LinkedHashMap<K,V>
-    {
-        private final int _cacheSize;
-
-        public ValueCache(int cacheSize)
-        {
-            _cacheSize = cacheSize;
-        }
-
-        @Override
-        protected boolean removeEldestEntry(Map.Entry<K, V> eldest)
-        {
-            return size() > _cacheSize;
-        }
-
-        public boolean isFull()
-        {
-            return size() == _cacheSize;
-        }
-    }
-
 
     public static StringTypeConstructor getInstance(int i, Charset c)
     {
@@ -84,44 +60,19 @@ public class StringTypeConstructor extends VariableWidthTypeConstructor
         }
 
         int origPosition = in.position();
-        _defaultBinaryString.setData(in.array(), in.arrayOffset()+ origPosition, size);
-
-        BinaryString binaryStr = _defaultBinaryString;
 
-        boolean isFull = _cachedValues.isFull();
-
-        String str = isFull ? _cachedValues.remove(binaryStr) : _cachedValues.get(binaryStr);
-
-        if(str == null)
+        ByteBuffer dup = in.duplicate();
+        try
         {
-
-            ByteBuffer dup = in.duplicate();
-            try
-            {
-                dup.limit(dup.position()+size);
-            }
-            catch(IllegalArgumentException e)
-            {
-                throw new IllegalArgumentException("position: " + dup.position() + "size: " + size + " capacity: " + dup.capacity());
-            }
-            CharBuffer charBuf = _charSet.decode(dup);
-
-            str = charBuf.toString();
-
-            byte[] data = new byte[size];
-            in.get(data);
-            binaryStr = new BinaryString(data, 0, size);
-
-            _cachedValues.put(binaryStr, str);
+            dup.limit(dup.position()+size);
         }
-        else if(isFull)
+        catch(IllegalArgumentException e)
         {
-            byte[] data = new byte[size];
-            in.get(data);
-            binaryStr = new BinaryString(data, 0, size);
-
-            _cachedValues.put(binaryStr, str);
+            throw new IllegalArgumentException("position: " + dup.position() + "size: " + size + " capacity: " + dup.capacity());
         }
+        CharBuffer charBuf = _charSet.decode(dup);
+
+        String str = charBuf.toString();
 
         in.position(origPosition+size);
 
diff --git a/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/SymbolTypeConstructor.java b/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/SymbolTypeConstructor.java
index 4049a40448..b846b16722 100644
--- a/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/SymbolTypeConstructor.java
+++ b/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/SymbolTypeConstructor.java
@@ -32,8 +32,6 @@ public class SymbolTypeConstructor extends VariableWidthTypeConstructor
 {
     private static final Charset ASCII = Charset.forName("US-ASCII");
 
-    private BinaryString _defaultBinaryString = new BinaryString();
-
     private static final ConcurrentHashMap<BinaryString, Symbol> SYMBOL_MAP =
             new ConcurrentHashMap<BinaryString, Symbol>(2048);
 
@@ -62,9 +60,7 @@ public class SymbolTypeConstructor extends VariableWidthTypeConstructor
             size = in.getInt();
         }
 
-        _defaultBinaryString.setData(in.array(), in.arrayOffset()+in.position(), size);
-
-        BinaryString binaryStr = _defaultBinaryString;
+        BinaryString binaryStr = new BinaryString(in.array(), in.arrayOffset()+in.position(), size);
 
         Symbol symbolVal = SYMBOL_MAP.get(binaryStr);
         if(symbolVal == null)