From 33dc3b0e50d570633fc0df7f9f8234f6a5c542e1 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 10 Jan 2020 14:47:29 +0100 Subject: [PATCH] Thread-safe compiled expression evaluation in SpelExpression Closes gh-24265 --- .../spel/standard/SpelExpression.java | 120 ++++++++++-------- 1 file changed, 70 insertions(+), 50 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java index 6321dd3a9d2a..3329322fc90d 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelExpression.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ package org.springframework.expression.spel.standard; +import java.util.concurrent.atomic.AtomicInteger; + import org.springframework.core.convert.TypeDescriptor; import org.springframework.expression.EvaluationContext; import org.springframework.expression.EvaluationException; @@ -62,15 +64,15 @@ public class SpelExpression implements Expression { private EvaluationContext evaluationContext; // Holds the compiled form of the expression (if it has been compiled) - private CompiledExpression compiledAst; + private volatile CompiledExpression compiledAst; // Count of many times as the expression been interpreted - can trigger compilation // when certain limit reached - private volatile int interpretedCount = 0; + private final AtomicInteger interpretedCount = new AtomicInteger(0); // The number of times compilation was attempted and failed - enables us to eventually // give up trying to compile it when it just doesn't seem to be possible. - private volatile int failedAttempts = 0; + private final AtomicInteger failedAttempts = new AtomicInteger(0); /** @@ -112,16 +114,17 @@ public String getExpressionString() { @Override public Object getValue() throws EvaluationException { - if (this.compiledAst != null) { + CompiledExpression compiledAst = this.compiledAst; + if (compiledAst != null) { try { EvaluationContext context = getEvaluationContext(); - return this.compiledAst.getValue(context.getRootObject().getValue(), context); + return compiledAst.getValue(context.getRootObject().getValue(), context); } catch (Throwable ex) { // If running in mixed mode, revert to interpreted if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { - this.interpretedCount = 0; this.compiledAst = null; + this.interpretedCount.set(0); } else { // Running in SpelCompilerMode.immediate mode - propagate exception to caller @@ -139,10 +142,11 @@ public Object getValue() throws EvaluationException { @SuppressWarnings("unchecked") @Override public T getValue(Class expectedResultType) throws EvaluationException { - if (this.compiledAst != null) { + CompiledExpression compiledAst = this.compiledAst; + if (compiledAst != null) { try { EvaluationContext context = getEvaluationContext(); - Object result = this.compiledAst.getValue(context.getRootObject().getValue(), context); + Object result = compiledAst.getValue(context.getRootObject().getValue(), context); if (expectedResultType == null) { return (T) result; } @@ -154,8 +158,8 @@ public T getValue(Class expectedResultType) throws EvaluationException { catch (Throwable ex) { // If running in mixed mode, revert to interpreted if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { - this.interpretedCount = 0; this.compiledAst = null; + this.interpretedCount.set(0); } else { // Running in SpelCompilerMode.immediate mode - propagate exception to caller @@ -173,15 +177,16 @@ public T getValue(Class expectedResultType) throws EvaluationException { @Override public Object getValue(Object rootObject) throws EvaluationException { - if (this.compiledAst != null) { + CompiledExpression compiledAst = this.compiledAst; + if (compiledAst != null) { try { - return this.compiledAst.getValue(rootObject, getEvaluationContext()); + return compiledAst.getValue(rootObject, getEvaluationContext()); } catch (Throwable ex) { // If running in mixed mode, revert to interpreted if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { - this.interpretedCount = 0; this.compiledAst = null; + this.interpretedCount.set(0); } else { // Running in SpelCompilerMode.immediate mode - propagate exception to caller @@ -200,9 +205,10 @@ public Object getValue(Object rootObject) throws EvaluationException { @SuppressWarnings("unchecked") @Override public T getValue(Object rootObject, Class expectedResultType) throws EvaluationException { - if (this.compiledAst != null) { + CompiledExpression compiledAst = this.compiledAst; + if (compiledAst != null) { try { - Object result = this.compiledAst.getValue(rootObject, getEvaluationContext()); + Object result = compiledAst.getValue(rootObject, getEvaluationContext()); if (expectedResultType == null) { return (T)result; } @@ -214,8 +220,8 @@ public T getValue(Object rootObject, Class expectedResultType) throws Eva catch (Throwable ex) { // If running in mixed mode, revert to interpreted if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { - this.interpretedCount = 0; this.compiledAst = null; + this.interpretedCount.set(0); } else { // Running in SpelCompilerMode.immediate mode - propagate exception to caller @@ -236,15 +242,16 @@ public T getValue(Object rootObject, Class expectedResultType) throws Eva public Object getValue(EvaluationContext context) throws EvaluationException { Assert.notNull(context, "EvaluationContext is required"); - if (this.compiledAst != null) { + CompiledExpression compiledAst = this.compiledAst; + if (compiledAst != null) { try { - return this.compiledAst.getValue(context.getRootObject().getValue(), context); + return compiledAst.getValue(context.getRootObject().getValue(), context); } catch (Throwable ex) { // If running in mixed mode, revert to interpreted if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { - this.interpretedCount = 0; this.compiledAst = null; + this.interpretedCount.set(0); } else { // Running in SpelCompilerMode.immediate mode - propagate exception to caller @@ -264,9 +271,10 @@ public Object getValue(EvaluationContext context) throws EvaluationException { public T getValue(EvaluationContext context, Class expectedResultType) throws EvaluationException { Assert.notNull(context, "EvaluationContext is required"); - if (this.compiledAst != null) { + CompiledExpression compiledAst = this.compiledAst; + if (compiledAst != null) { try { - Object result = this.compiledAst.getValue(context.getRootObject().getValue(), context); + Object result = compiledAst.getValue(context.getRootObject().getValue(), context); if (expectedResultType != null) { return ExpressionUtils.convertTypedValue(context, new TypedValue(result), expectedResultType); } @@ -277,8 +285,8 @@ public T getValue(EvaluationContext context, Class expectedResultType) th catch (Throwable ex) { // If running in mixed mode, revert to interpreted if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { - this.interpretedCount = 0; this.compiledAst = null; + this.interpretedCount.set(0); } else { // Running in SpelCompilerMode.immediate mode - propagate exception to caller @@ -297,15 +305,16 @@ public T getValue(EvaluationContext context, Class expectedResultType) th public Object getValue(EvaluationContext context, Object rootObject) throws EvaluationException { Assert.notNull(context, "EvaluationContext is required"); - if (this.compiledAst != null) { + CompiledExpression compiledAst = this.compiledAst; + if (compiledAst != null) { try { - return this.compiledAst.getValue(rootObject, context); + return compiledAst.getValue(rootObject, context); } catch (Throwable ex) { // If running in mixed mode, revert to interpreted if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { - this.interpretedCount = 0; this.compiledAst = null; + this.interpretedCount.set(0); } else { // Running in SpelCompilerMode.immediate mode - propagate exception to caller @@ -327,9 +336,10 @@ public T getValue(EvaluationContext context, Object rootObject, Class exp Assert.notNull(context, "EvaluationContext is required"); - if (this.compiledAst != null) { + CompiledExpression compiledAst = this.compiledAst; + if (compiledAst != null) { try { - Object result = this.compiledAst.getValue(rootObject, context); + Object result = compiledAst.getValue(rootObject, context); if (expectedResultType != null) { return ExpressionUtils.convertTypedValue(context, new TypedValue(result), expectedResultType); } @@ -340,8 +350,8 @@ public T getValue(EvaluationContext context, Object rootObject, Class exp catch (Throwable ex) { // If running in mixed mode, revert to interpreted if (this.configuration.getCompilerMode() == SpelCompilerMode.MIXED) { - this.interpretedCount = 0; this.compiledAst = null; + this.interpretedCount.set(0); } else { // Running in SpelCompilerMode.immediate mode - propagate exception to caller @@ -454,48 +464,58 @@ public void setValue(EvaluationContext context, Object rootObject, Object value) * @param expressionState the expression state used to determine compilation mode */ private void checkCompile(ExpressionState expressionState) { - this.interpretedCount++; + this.interpretedCount.incrementAndGet(); SpelCompilerMode compilerMode = expressionState.getConfiguration().getCompilerMode(); if (compilerMode != SpelCompilerMode.OFF) { if (compilerMode == SpelCompilerMode.IMMEDIATE) { - if (this.interpretedCount > 1) { + if (this.interpretedCount.get() > 1) { compileExpression(); } } else { // compilerMode = SpelCompilerMode.MIXED - if (this.interpretedCount > INTERPRETED_COUNT_THRESHOLD) { + if (this.interpretedCount.get() > INTERPRETED_COUNT_THRESHOLD) { compileExpression(); } } } } - /** - * Perform expression compilation. This will only succeed once exit descriptors for all nodes have - * been determined. If the compilation fails and has failed more than 100 times the expression is - * no longer considered suitable for compilation. + * Perform expression compilation. This will only succeed once exit descriptors for + * all nodes have been determined. If the compilation fails and has failed more than + * 100 times the expression is no longer considered suitable for compilation. + * @return whether this expression has been successfully compiled */ public boolean compileExpression() { - if (this.failedAttempts > FAILED_ATTEMPTS_THRESHOLD) { + CompiledExpression compiledAst = this.compiledAst; + if (compiledAst != null) { + // Previously compiled + return true; + } + if (this.failedAttempts.get() > FAILED_ATTEMPTS_THRESHOLD) { // Don't try again return false; } - if (this.compiledAst == null) { - synchronized (this.expression) { - // Possibly compiled by another thread before this thread got into the sync block - if (this.compiledAst != null) { - return true; - } - SpelCompiler compiler = SpelCompiler.getCompiler(this.configuration.getCompilerClassLoader()); - this.compiledAst = compiler.compile(this.ast); - if (this.compiledAst == null) { - this.failedAttempts++; - } + + synchronized (this) { + if (this.compiledAst != null) { + // Compiled by another thread before this thread got into the sync block + return true; + } + SpelCompiler compiler = SpelCompiler.getCompiler(this.configuration.getCompilerClassLoader()); + compiledAst = compiler.compile(this.ast); + if (compiledAst != null) { + // Successfully compiled + this.compiledAst = compiledAst; + return true; + } + else { + // Failed to compile + this.failedAttempts.incrementAndGet(); + return false; } } - return (this.compiledAst != null); } /** @@ -505,8 +525,8 @@ public boolean compileExpression() { */ public void revertToInterpreted() { this.compiledAst = null; - this.interpretedCount = 0; - this.failedAttempts = 0; + this.interpretedCount.set(0); + this.failedAttempts.set(0); } /**