[PATCH] StringBuffers should not be blindly re-used in XmlRpc.java

[Aren Sandersen <ar...@danger.com>]

This fixes a memory leak for those using Sun's 1.4.1 JRE.  See a previous email by me for details.  In a nutshell, StringBuffers should not be blindly re-used.  If they are re-used, they should be destroyed and re-created after their underlying arrays have grown.

Aren

--------

--- src/java/org/apache/xmlrpc/XmlRpc.java.orig	Fri May  9 14:24:55 2003
+++ src/java/org/apache/xmlrpc/XmlRpc.java	Fri May  9 17:46:32 2003
@@ -102,6 +102,14 @@
      */
     private static int maxThreads = 100;
 
+    /**
+     * The initial size of the cdata StringBuffer
+     *
+     * 32 seems like a good average between integer and string data
+     * lengths
+     */
+    private static final int cdataSize = 32;
+
     String methodName;
 
     /**
@@ -358,6 +366,32 @@
     }
 
     /**
+     * Clean the cdata StringBuffer object.  If it has grown since we
+     * created it, re-create one at the original size
+     */
+    private void cleanCData()
+    {        
+        // It is dangerous to re-use StringBuffers without checking the
+        // underlying capacity, due to a change in SUN JRE 1.4.1.  This code
+        // will allow the StringBuffer to be re-used if the underlying 
+        // capacity isn't too large, otherwise a new StringBuffer object
+        // is created.
+        //
+        // For details, please see:
+        //
+        // http://developer.java.sun.com/developer/bugParade/bugs/4724129.html
+
+        if (cdata.capacity() > cdataSize)
+        {
+            cdata = new StringBuffer(cdataSize);
+        }
+        else
+        {
+            cdata.setLength(0);
+        }
+    }
+
+    /**
      * Parse the input stream. For each root level object, method
      * <code>objectParsed</code> is called.
      */
@@ -369,11 +403,7 @@
         values = new Stack ();
         if (cdata == null)
         {
-            cdata = new StringBuffer(128);
-        }
-        else
-        {
-            cdata.setLength(0);
+            cdata = new StringBuffer(cdataSize);
         }
         readCdata = false;
         currentValue = null;
@@ -420,11 +450,10 @@
         }
         finally
         {
-            // Clear any huge buffers.
-            if (cdata.length() > 128 * 4)
+            // Don't keep StringBuffer around if it has a large underlying
+            // array.  It's a waste of memory.
+            if (cdata.capacity() > cdataSize)
             {
-                // Exceeded original capacity by greater than 4x; release
-                // buffer to prevent leakage.
                 cdata = null;
             }
         }
@@ -473,7 +502,7 @@
         if (currentValue != null && readCdata)
         {
             currentValue.characterData(cdata.toString());
-            cdata.setLength(0);
+            cleanCData();
             readCdata = false;
         }
 
@@ -515,7 +544,7 @@
         else if ("methodName".equals(name))
         {
             methodName = cdata.toString();
-            cdata.setLength(0);
+            cleanCData();
             readCdata = false;
         }
     }
@@ -536,54 +565,53 @@
             Value v = new Value();
             values.push(v);
             currentValue = v;
-            // cdata object is reused
-            cdata.setLength(0);
+            cleanCData();
             readCdata = true;
         }
         else if ("methodName".equals(name))
         {
-            cdata.setLength(0);
+            cleanCData();
             readCdata = true;
         }
         else if ("name".equals(name))
         {
-            cdata.setLength(0);
+            cleanCData();
             readCdata = true;
         }
         else if ("string".equals(name))
         {
             // currentValue.setType (STRING);
-            cdata.setLength(0);
+            cleanCData();
             readCdata = true;
         }
         else if ("i4".equals(name) || "int".equals(name))
         {
             currentValue.setType(INTEGER);
-            cdata.setLength(0);
+            cleanCData();
             readCdata = true;
         }
         else if ("boolean".equals(name))
         {
             currentValue.setType(BOOLEAN);
-            cdata.setLength(0);
+            cleanCData();
             readCdata = true;
         }
         else if ("double".equals(name))
         {
             currentValue.setType(DOUBLE);
-            cdata.setLength(0);
+            cleanCData();
             readCdata = true;
         }
         else if ("dateTime.iso8601".equals(name))
         {
             currentValue.setType(DATE);
-            cdata.setLength(0);
+            cleanCData();
             readCdata = true;
         }
         else if ("base64".equals(name))
         {
             currentValue.setType(BASE64);
-            cdata.setLength(0);
+            cleanCData();
             readCdata = true;
         }
         else if ("struct".equals(name))