On 19/12/06, Eric Dalquist <[EMAIL PROTECTED]> wrote:
No problem, the questions are appreciated.
The method signatures had to change. Since the get/set/init and default
constructor are not longer actually doing reflection there is no chance
of those exceptions being thrown. Java won't let you declare a throws
clause for a checked exception if there is no chance the method will
throw that exception.
Indeed, but currently the BeanShellInterpreter constructors only throw
ClassNotFoundException - why does the non-default constructor now need
to throw JMeterException and IOException as well?
Also, it's important that the classes behave sensibly even if the
beanshell jar cannot be found. At present, the code can be built and
run without needing the jars, though of course there will be errors
reported at run-time.
I can change that variable name back if you would like.
Please.
I hadn't checked the to make sure the global methods are preserved as
well, I don't think they will be with the current patch. I'll check that
later today and see about a fix.
Thanks for the feedback,
-Eric
sebb wrote:
> Thanks for the update.
>
> Not sure why various method signatures were changed nor why the
> methods no longer handle certain exceptions themselves. Could you
> explain?
>
> [Also, changing the temporary variable name "clazz" to
> "interpreterClass" may make the code a bit clearer, but makes
> reviewing the changes a lot harder ..]
>
> Another matter I forgot to mention: the bshinit files can contain
> global variable definitions and methods. Are these preserved across
> invocations?
>
> Sorry to keep going on about this but it is important to maintain
> backwards compatibility.
>
> S
> On 18/12/06, Eric Dalquist <[EMAIL PROTECTED]> wrote:
>> 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]
>> >
>>
>>
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
