SpEL fails to recover from error during MIXED mode compilation

[happier233]

Affects: 5.2.13.RELEASE

---

I am testing spel performance, when I share a SpelExpression instance between multiple threads.

I tried set SpelCompilerMode to MIXED or IMMEDIATE and put a variable with different type in the context, then I triggered an exception.

If I set SpelCompilerMode to OFF, it works correctly. So I am confused, is spel thread safe?

I didn't find any description of spel's thread safety in the spring project document. I want to ask it's a spel thread safe bug or spel is not thread safe when compile is enable.

Here is my test code:

import java.util.concurrent.atomic.AtomicInteger;

import org.springframework.expression.EvaluationContext;
import org.springframework.expression.Expression;
import org.springframework.expression.ExpressionParser;
import org.springframework.expression.spel.SpelCompilerMode;
import org.springframework.expression.spel.SpelParserConfiguration;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.DataBindingMethodResolver;
import org.springframework.expression.spel.support.SimpleEvaluationContext;

import lombok.AllArgsConstructor;
import lombok.Data;

/**
 * @author happier233
 * @version Main.java, v 0.1 2021年08月18日 10:02 上午 happier233
 */
public class Main {

    public static String                  name                    = "default";

    public static Context                 root                    = new Context();

    public static SpelParserConfiguration spelParserConfiguration = new SpelParserConfiguration(
        SpelCompilerMode.MIXED, Main.class.getClassLoader());
    public static ExpressionParser        parser                  = new SpelExpressionParser(
        spelParserConfiguration);

    public static Expression              expression              = parser
        .parseExpression("#bean.name + '234'");

    public static void main(String[] args) {
        calc(() -> {
            for (int i = 0; i < 10; i++) {
                calc(() -> run(true));
            }
        });
        new Thread(Main::run).start();
        new Thread(Main::run).start();
    }

    public static void run() {
        calc(() -> {
            for (int i = 0; i < 1000000; i++) {
                run(false);
            }
        });
    }

    private static final AtomicInteger cc = new AtomicInteger();

    private static void run(boolean flag) {
        EvaluationContext context = SimpleEvaluationContext.forReadOnlyDataBinding()
            .withRootObject(root)
            .withMethodResolvers(DataBindingMethodResolver.forInstanceMethodInvocation()).build();
        if ((cc.incrementAndGet() & 1) == 0) {
            context.setVariable("bean", new Bean("test"));
        } else {
            context.setVariable("bean", new Bean2(123));
        }
        Object value = expression.getValue(context);
        if (flag) {
            System.out.println(value);
        }
    }

    public static void calc(Runnable runnable) {
        long start = System.currentTimeMillis();
        runnable.run();
        long end = System.currentTimeMillis();
        System.out.println("time[" + name + "]: " + (end - start));
    }

    @Data
    public static class Context {
        private String        title = "233";
        private AtomicInteger count = new AtomicInteger(0);
    }

    @Data
    @AllArgsConstructor
    public static class Bean {
        private String name;
    }

    @Data
    @AllArgsConstructor
    public static class Bean2 {
        private Integer name;
    }
}

Here is the excpetion stack:

Exception in thread "Thread-1" java.lang.IllegalStateException: Failed to instantiate CompiledExpression
	at org.springframework.expression.spel.standard.SpelCompiler.compile(SpelCompiler.java:113)
	at org.springframework.expression.spel.standard.SpelExpression.compileExpression(SpelExpression.java:526)
	at org.springframework.expression.spel.standard.SpelExpression.checkCompile(SpelExpression.java:497)
	at org.springframework.expression.spel.standard.SpelExpression.getValue(SpelExpression.java:273)
	at org.example.acm.Main.run(Main.java:65)
	at org.example.acm.Main.lambda$run$2(Main.java:49)
	at org.example.acm.Main.calc(Main.java:73)
	at org.example.acm.Main.run(Main.java:47)
	at java.lang.Thread.run(Thread.java:750)
Caused by: java.lang.VerifyError: (class: spel/Ex27, method: getValue signature: (Ljava/lang/Object;Lorg/springframework/expression/EvaluationContext;)Ljava/lang/Object;) Incompatible object argument for function call
	at java.lang.Class.getDeclaredConstructors0(Native Method)
	at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
	at java.lang.Class.getConstructor0(Class.java:3075)
	at java.lang.Class.getDeclaredConstructor(Class.java:2178)
	at org.springframework.util.ReflectionUtils.accessibleConstructor(ReflectionUtils.java:185)
	at org.springframework.expression.spel.standard.SpelCompiler.compile(SpelCompiler.java:110)
	... 8 more

[happier233]

I made a mistake, this problem is not happened between multiple threads. I remove the multi thread code, I still happened.
Just When I put a variable with different type in the context, the expcetion will happen.

[sbrannen]

Hi @happier233,

Thanks for reporting your first issue for the Spring Framework. 👍

This appears to be a bug in SpEL with regard to state tracking for the SpelExpression.interpretedCount field.

Specifically, interpretedCount is always incremented in checkCompile(ExpressionState) even if the call to compileExpression() fails with an exception.

We'll fix it.

[sbrannen]

I made a mistake, this problem is not happened between multiple threads. I remove the multi thread code, I still happened.

Are you positive that the same error occurs without concurrent use of the shared expression instance?

If so, could you please share an example without concurrent access which demonstrates that?

[sbrannen]

Update

Using a simplified version of the original example, I can reliably reproduce a stack trace similar to the following.

	at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
	at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
	at java.util.concurrent.ForkJoinTask.getThrowableException(ForkJoinTask.java:593)
	at java.util.concurrent.ForkJoinTask.reportException(ForkJoinTask.java:677)
	at java.util.concurrent.ForkJoinTask.invoke(ForkJoinTask.java:735)
	at java.util.stream.ForEachOps$ForEachOp.evaluateParallel(ForEachOps.java:159)
	at java.util.stream.ForEachOps$ForEachOp$OfInt.evaluateParallel(ForEachOps.java:188)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:233)
	at java.util.stream.IntPipeline.forEach(IntPipeline.java:427)
	at java.util.stream.IntPipeline$Head.forEach(IntPipeline.java:584)
	at example.Main.main(Main.java:37)
Caused by: java.lang.IllegalStateException: Failed to instantiate CompiledExpression
	at org.springframework.expression.spel.standard.SpelCompiler.compile(SpelCompiler.java:114)
	at org.springframework.expression.spel.standard.SpelExpression.compileExpression(SpelExpression.java:527)
	at org.springframework.expression.spel.standard.SpelExpression.checkCompile(SpelExpression.java:498)
	at org.springframework.expression.spel.standard.SpelExpression.getValue(SpelExpression.java:274)
	at example.Main.evaluateExpression(Main.java:67)
	at java.util.stream.ForEachOps$ForEachOp$OfInt.accept(ForEachOps.java:204)
	at java.util.stream.Streams$RangeIntSpliterator.forEachRemaining(Streams.java:110)
	at java.util.Spliterator$OfInt.forEachRemaining(Spliterator.java:693)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
	at java.util.stream.ForEachOps$ForEachTask.compute(ForEachOps.java:290)
	at java.util.concurrent.CountedCompleter.exec(CountedCompleter.java:731)
	at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
	at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
	at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
	at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:175)
Caused by: java.lang.VerifyError: Bad type on operand stack
Exception Details:
  Location:
    spel/Ex10.getValue(Ljava/lang/Object;Lorg/springframework/expression/EvaluationContext;)Ljava/lang/Object; @11: invokevirtual
  Reason:
    Type 'example/Main$Bean2' (current frame, stack[0]) is not assignable to 'example/Main$Bean1'
  Current Frame:
    bci: @11
    flags: { }
    locals: { 'spel/Ex10', 'java/lang/Object', 'org/springframework/expression/EvaluationContext' }
    stack: { 'example/Main$Bean2' }
  Bytecode:
    0x0000000: 2c12 0eb9 0014 0200 c000 16b6 001c b0  

	at java.lang.Class.getDeclaredConstructors0(Native Method)
	at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
	at java.lang.Class.getConstructor0(Class.java:3075)
	at java.lang.Class.getDeclaredConstructor(Class.java:2178)
	at org.springframework.util.ReflectionUtils.accessibleConstructor(ReflectionUtils.java:185)
	at org.springframework.expression.spel.standard.SpelCompiler.compile(SpelCompiler.java:111)
	... 14 more

The key thing to note is:

Type 'example/Main$Bean2' (current frame, stack[0]) is not assignable to 'example/Main$Bean1'

Based on my current understanding of the issue, this means that the compiled expression Class (spel/Ex10) generated by the SpelCompiler contains references to types from custom registered variables from two different evaluations (in this case Bean1 and Bean2) of the shared SpelExpression. Thus, there appears to be a concurrency issue with regard to the generation of the byte code for the compiled class.

[sbrannen]

Related issue:

• Unsafe double-checked locking in SpelExpression#compileExpression #24265

[happier233]

Thus, there appears to be a concurrency issue with regard to the generation of the byte code for the compiled class.

Yes, I agree with that. In my project, I have an interface "Order" and there are multiple implementations. So I may invoke one SpelExpression with different implementations.

[happier233]

In the implementation org.springframework.expression.spel.standard.SpelExpression#getValue, all exception will be caught, include compile error. When the compile model is MIXED, it will be failover to normal ast to get value. But in SpelExpression#checkCompile, there is no exception catahing.

[sbrannen]

In my project, I have an interface "Order" and there are multiple implementations. So I may invoke one SpelExpression with different implementations.

Do the evaluated/compiled expressions all invoke only methods defined in your Order interface, and if so are there any generics involved in the method signatures?

Or does your expression invoke any method that is unique to a particular concrete implementation?

[sbrannen]

This has been fixed in 5.3.x (for inclusion in 5.3.17) and main. See commit 94af2ca for details.

Although we recommend that you not use the MIXED compiler mode in SpEL when frequently changing types used in the expression (such as a custom variable), this fix should allow you to do so. However, keep in mind that the compiler will continually try to compile expressions (generating classes for them on the fly) only to throw them away later, and at some point the compiler will determine that it does not make sense to continue trying (due to the failedAttempts threshold).

In other words, if you used MIXED mode in such scenarios you will not actually benefit from compiled expressions.

In the particular example provided, you could introduce a common interface that each of the types used as the bean variable can implement. If the method invoked on the bean variable comes from the common interface, SpEL should be able to reliably compile and reuse an expression based on that interface.