Mailing-List: contact rpc-dev-help@xml.apache.org; run by ezmlm
Sender: dlr@despot.finemaltcoding.com
Cc: rpc-dev@xml.apache.org
To: Timothy Peierls <tim@peierls.net>
Subject: Re: cleanup in XmlRpcServer.Worker
References: <3C4471B5.44631D15@peierls.net>
Content-Type: text/plain; charset=US-ASCII
From: Daniel Rall <dlr@finemaltcoding.com>
Date: Fri, 25 Jan 2002 15:05:17 -0800
In-Reply-To: <3C4471B5.44631D15@peierls.net> (Timothy Peierls's message of
 "Tue, 15 Jan 2002 13:15:17 -0500")
Message-ID: <m3y9imhw0i.fsf@despot.finemaltcoding.com>
Lines: 46
User-Agent: Gnus/5.090004 (Oort Gnus v0.04) XEmacs/21.1 (Cuyahoga Valley)
MIME-Version: 1.0

Hi Tim.  I've committed code implementing the general idea behind your
patch to CVS.  Thanks much!

Dan


Timothy Peierls <tim@peierls.net> writes:

> The XmlRpcServer.Worker fields inParams, outParam, result, and
> strbuf are not being cleared on exit from the execute() method, and
> I believe they should be. All it takes is to wrap the execute() body
> with
>
>     try {
>         ...
>     }
>     finally {
>         inParams = null;
>         outParam = null;
>         result = null;
>         strBuf.setLength(0);
>     }
>
> and remove the existing else clause that has "strBuf.setLength(0)".
>
> Even better, if outParam and result were made local variables
> instead of fields of Worker, you could remove the second and third
> lines of the finally clause.
>
> Why does this matter? Because there is no guarantee that the
> execute() method will be called again any time soon, or ever. Since
> there is no size restriction on arguments and results in the XML-RPC
> spec, you can have arbitrarily large objects hanging around for
> arbitrarily long intervals, a classic Java memory leak.
>
> I saw this happen in practice: The service defined a method to
> return the state of the entire system (as XML-RPC structs and
> arrays). Huge chunks of memory that could have been freed up on
> return were left dangling uselessly until the Worker was next used.
> Performance noticeably improved when we made a local fix to the
> Worker class.
>
> I asked about this over a year ago, but I think it got lost in the
> cracks.
>
> Tim Peierls <mailto:tim@peierls.net>