[logback-user] JaninoEventEvaluator and class loader

[ceki]

Hi Glyn,

We started moved away from ExpressionEvaluator and started using 
ScriptEvaluator because the latter allows for java blocks whereas the 
former allows only boolean expressions. The ability to parse java blocks 
reduces the ease-of-use gap between JaninoEvaluator and GEvaluator. See 
also:
http://mailman.qos.ch/pipermail/logback-user/2011-January/002002.html

Class loading issues were not taken into consideration when making this 
change.

More in line.

On 05.01.2012 13:09, Glyn Normington wrote:
> I am attempting to get Janino 2.6.1 working with Logback 0.9.28 (or
> later, but that's the version we are using in Eclipse Virgo right now) -
> see [0] for background. Unfortunately Janino ends up using the thread
> context class loader as its "parent" class loader and fails with a
> runtime exception. I have discussed this ([1]) on the Janino mailing
> list and it seems that it is necessary to set the parent class loader in
> Janino. I am somewhat at the mercy of Logback here.

OK.

> The surprising thing is that the code in Logback 0.9.24 looked pretty
> usable in this respect. JaninoEventEvaluator.start had the following
> sequence:
>
> ClassLoader cl = context.getClass().getClassLoader();
> ee = new ExpressionEvaluator(getDecoratedExpression(), EXPRESSION_TYPE,
> getParameterNames(), getParameterTypes(), THROWN_EXCEPTIONS, cl);
>
> thus setting the parent class loader to a value which I could ensure
> would be capable of loading the necessary types.
>
> In 0.9.28 this code has been replaced by:
>
> scriptEvaluator = new ScriptEvaluator(getDecoratedExpression(),
> EXPRESSION_TYPE,
> getParameterNames(), getParameterTypes(), THROWN_EXCEPTIONS);
>
> which causes the current TCCL to be used as the parent class loader and
> ultimately results in Janino failing.

Well, as mentioned above, class loading issues were not taken into 
consideration when making the change. I was not aware that 
ScriptEvaluator used the TCCL by default.

> I can't control the TCCL that happens to be in use when start is called
> as that is driven out of a logging call which can come from an arbitrary
> thread.

Right.

> I found the relevant commit ([2]), but I can't tell from that why this
> specific change was made.

Explanation give above.

> If it is absolutely necessary to use ScriptEvaluator rather than
> ExpressionEvaluator, the following code sequence (based on a suggestion
> from Arno Unkrig) would reproduce the parent class loader behaviour of
> 0.9.24:
>
> ClassLoader cl = context.getClass().getClassLoader();
> scriptEvaluator = new ScriptEvaluator();
> scriptEvaluator.setParentClassLoader(cl);
> scriptEvaluator.setReturnType(EXPRESSION_TYPE);
> scriptEvaluator.setParameterNames(getParameterNames());
> scriptEvaluator.setParameterTypes(getParameterTypes());
> scriptEvaluator.setThrownExceptions(THROWN_EXCEPTIONS);
> scriptEvaluator.cook(getDecoratedExpression());

The above looks good to me. Please create a jira issue requesting for 
the above change. A reference to this message should provide the 
relevant context for the jira issue.

> The alternative of using ExpressionEvaluator is much neater and seems to
> be close to the ScriptEvaluator variant since ExpressionEvaluator
> extends ScriptEvaluator.

AFAIK, only ScriptEvaluator parses java blocks. ExpressionEvaluator does 
not.

> Any suggestions gratefully received.

I would not mind if in addition to the jira issue you could also apply 
and then test the approach suggested by Arno Unkrig, culminating in  git 
pull request. Yay!

> Regards,
> Glyn
> [0] https://bugs.eclipse.org/bugs/show_bug.cgi?id=333920
> [1] mailing list archive link currently broken - if I can get a link to
> it, I'll post this later
> [2]
> https://github.com/ceki/logback/commit/06a5b692f14560636bd92d7bd7cf1f85830f4e55#diff-4
>


-- 
Ceki
http://twitter.com/#!/ceki