Sounds good - maybe the XmlRpc class could even have a configurable maximum size for the CDATA buffer.
-----Original Message----- From: [EMAIL PROTECTED] [mailto:dlr@;finemaltcoding.com] Sent: Monday, November 11, 2002 2:50 PM To: Glen Lewis Cc: [EMAIL PROTECTED] Subject: Re: CDATA leaks in XmlRpc client classes "Glen Lewis" <[EMAIL PROTECTED]> writes: > XmlRpc.parse() initializes the "cdata" field and then clears it ( by > calling setLength(0) ) upon subsequent calls to parse(). This > results in a memory leak between calls to parse(), since the "cdata" > field persists as long as the XmlRpc object. In my particular case, > a large logfile was being passed as a parameter in a response, and > the "cdata" leak was many megs in size - it ended up consuming all > the memory available to my client application. > > I fixed it (somewhat aggressively) by changing the line in > XmlRpc.java that reads: > > parser.parse (new InputSource (is)); > to: > try > { > parser.parse (new InputSource (is)); > } > finally > { > cdata = null; > } Hey Glen. What you describe is definitely a problem. Releasing the buffer every time doesn't seem like the Right solution either, since if it is used once it's very likely to be used again. Setting a "resource release threshold" of, say, 4 times the the original allocation seems like reasonable behavior. What do you say to this patch? Index: XmlRpc.java =================================================================== RCS file: /home/cvs/xml-rpc/src/java/org/apache/xmlrpc/XmlRpc.java,v retrieving revision 1.33 diff -u -u -r1.33 XmlRpc.java --- XmlRpc.java 10 Oct 2002 00:33:24 -0000 1.33 +++ XmlRpc.java 11 Nov 2002 22:47:33 -0000 @@ -414,7 +414,20 @@ { System.out.println("Beginning parsing XML input stream"); } - parser.parse(new InputSource (is)); + try + { + parser.parse(new InputSource (is)); + } + finally + { + // Clear any huge buffers. + if (cdata.length() > 128 * 4) + { + // Exceeded original capacity by greater than 4x; release + // buffer to prevent leakage. + cdata = null; + } + } if (debug) { System.out.println ("Spent " + (System.currentTimeMillis() - now) Some sort of StringBuffer pool might make for a better long-term solution, but this ought to fix the immediate problem. -- Daniel Rall <[EMAIL PROTECTED]>
