Spring SpEL fails to recover from error during MIXED mode compilation

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

Comment From: 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.

Comment From: 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.

Comment From: 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?

Comment From: sbrannen

Update

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

```Exception in thread "main" java.lang.IllegalStateException: java.lang.IllegalStateException: Failed to instantiate CompiledExpression 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.

Comment From: sbrannen

Related issue:

24265

Comment From: 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.

Comment From: 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.

Comment From: 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?

Comment From: sbrannen

This has been fixed in 5.3.x (for inclusion in 5.3.17) and main. See commit 94af2ca06bf711de03286999b1c64a703d6de552 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.

Spring SpEL fails to recover from error during MIXED mode compilation

Update

24265