[logback-user] JaninoEventEvaluator and class loader

[Glyn Normington]

Arno kindly fixed the Janino mailing list archive, as promised, so here's the link to my thread there:

[1] http://old.nabble.com/Janino-and-OSGi--to33030424.html

Regards,
Glyn

On 8 Jan 2012, at 18:50, Arno Unkrig wrote:

> Hi Glyn and everybody else,
> 
> one quick note here: There is no difference between "ScriptEvaluator" and "ExpressionEvaluator" (other than that the former compiles a method body and the latter an expression). Initially, both came with a set of constructors with varying parameter counts and types, but because the configuration possibilities grew over time, the recommended usage pattern is now to call the parameterless constructor, then a series of setters, and then (one of) the "cook()" methods.
> 
> I also wonder why LOGBACK switched from ExpressionEvaluator
> 
> https://github.com/ceki/logback/blob/6c5ba501831d19879e6865f795a1c294ad25bf7d/logback-core/src/main/java/ch/qos/logback/core/boolex/JaninoEventEvaluatorBase.java
> 
> to ScriptEvaluator
> 
> https://github.com/ceki/logback/blob/06a5b692f14560636bd92d7bd7cf1f85830f4e55/logback-core/src/main/java/ch/qos/logback/core/boolex/JaninoEventEvaluatorBase.java
> 
> anyway!? That's a huge semantical change...
> 
> Am 05.01.2012 13:09, schrieb Glyn Normington:
>> 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.
>> 
>> 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.
>> 
>> 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.
>> 
>> I found the relevant commit ([2]), but I can't tell from that why this
>> specific change was made.
>> 
>> 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 alternative of using ExpressionEvaluator is much neater and seems to
>> be close to the ScriptEvaluator variant since ExpressionEvaluator
>> extends ScriptEvaluator.
>> 
>> Any suggestions gratefully received.
>> 
>> 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
>> 
>