You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by rg...@apache.org on 2013/09/25 16:51:17 UTC

svn commit: r1526198 - in /qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec: StringTypeConstructor.java SymbolTypeConstructor.java

Author: rgodfrey
Date: Wed Sep 25 14:51:17 2013
New Revision: 1526198

URL: http://svn.apache.org/r1526198
Log:
QPID-5172 :  Thread safety issue in StringTypeConstructor.construct and SymbolTypeConstructor.construct  (patch from dingham@microsoft.com)

Modified:
    qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/StringTypeConstructor.java
    qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/SymbolTypeConstructor.java

Modified: qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/StringTypeConstructor.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/StringTypeConstructor.java?rev=1526198&r1=1526197&r2=1526198&view=diff
==============================================================================
--- qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/StringTypeConstructor.java (original)
+++ qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/StringTypeConstructor.java Wed Sep 25 14:51:17 2013
@@ -32,30 +32,6 @@ public class StringTypeConstructor exten
 {
     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 exten
         }
 
         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);
 

Modified: qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/SymbolTypeConstructor.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/SymbolTypeConstructor.java?rev=1526198&r1=1526197&r2=1526198&view=diff
==============================================================================
--- qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/SymbolTypeConstructor.java (original)
+++ qpid/trunk/qpid/java/amqp-1-0-common/src/main/java/org/apache/qpid/amqp_1_0/codec/SymbolTypeConstructor.java Wed Sep 25 14:51:17 2013
@@ -32,8 +32,6 @@ public class SymbolTypeConstructor exten
 {
     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 exten
             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)



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@qpid.apache.org
For additional commands, e-mail: commits-help@qpid.apache.org