I have a fix that I think will work. a bsh.Interpreter is created each time BeanShellInterpreter.eval() is called. There is an Map member variable that is used to store all of the variables set in by the calls to the set and get method of the BeanShellInterpreter class. Before each Interpreter.eval() all of the variables in the member map are set on the Interpreter, after all variables defined in the NameSpace are copied into the member Map. This should maintain state for the scripts between runs. Looking at the profiler there aren't any leaking BSH objects any more.

I did my best to make a minimal set of changes. Because of the checked exceptions declared on many of the methods I had to make minor exception handling updates to all of the classes that use the BeanShellInterpreter. If this bug is ever fixed in BSH it should be easy to revert back to the old usage pattern.

-Eric

The patch is attached to the issue: http://issues.apache.org/bugzilla/show_bug.cgi?id=40850

sebb wrote:
On 18/12/06, Eric Dalquist <[EMAIL PROTECTED]> wrote:
sebb,

We believe we're running into this bean shell memory leak:
http://issues.apache.org/bugzilla/show_bug.cgi?id=40850

We have a script we're trying to use to monitor a cluster of servers so
it will be running indefinitely. There is a BeanShell listener that runs
command line tasks based on server responses. The script runs out of
memory rather quickly with the BeanShell listener enabled.

I'd like to get your thoughts on the issue. I'm planning on taking the
posted source files and making an actual patch out of them to post.

A patch would definitely be better than the full source files as
currently provided - this should be against branch/rel-2-2.

However, I have some issues with the existing proposals.

The example BeanShellPreProcessor clears the interpreter variable each
time, so there little point in the init() method - the process()
method might as well just create the interpreter each time. [But see
below]

The BeanShellInterpreter cleanup() method should probably not throw
any Exceptions - it has already logged any problems. But I like the
idea of having such a method - perhaps it should be called reset()
instead.

The other major issue is that setting BeanShellInterpreter to null
will lose any saved context. Seems to me that this is a serious
restriction.

Not sure if it is possible to prevent the memory leak whilst still
keeping the interpreter available, but if so, that would be the best
way to go.

All BeanShell elements could be changed to call cleanup()/reset()
after each use.

The bshInterpreter itself could be cleared in the BeanShellInterpreter
class if necessary - as adding it to the other classes makes
maintenance a bit harder if the leak is later fixed.

To sum up, I think it would be worth trying:
- add cleanup()/reset() method to BeanShellInterpreter. If necessary
that can manage resetting the actual interpreter.

- call the cleanup()/reset() method from all the other BeanShell
classes.  Do not clear the BeanShell interpreter instance variable.

S///

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature

Reply via email to