FunctionReference's method field is volatile

Issue: SPR-16255

Author: jhoeller

spring-expression/src/main/java/org/springframework/expression/spel/ast/FunctionReference.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -47,6 +47,7 @@
  * (right now), so the names must be unique.
  *
  * @author Andy Clement
+ * @author Juergen Hoeller
  * @since 3.0
  */
 public class FunctionReference extends SpelNodeImpl {
@@ -56,9 +57,7 @@ public class FunctionReference extends SpelNodeImpl {
 	// Captures the most recently used method for the function invocation *if* the method
 	// can safely be used for compilation (i.e. no argument conversion is going on)
 	@Nullable
-	private Method method;
-	
-	private boolean argumentConversionOccurred;
+	private volatile Method method;
 
 
 	public FunctionReference(String functionName, int pos, SpelNodeImpl... arguments) {
@@ -90,14 +89,13 @@ public TypedValue getValueInternal(ExpressionState state) throws EvaluationExcep
 	}
 
 	/**
-	 * Execute a function represented as a java.lang.reflect.Method.
+	 * Execute a function represented as a {@code java.lang.reflect.Method}.
 	 * @param state the expression evaluation state
 	 * @param method the method to invoke
 	 * @return the return value of the invoked Java method
 	 * @throws EvaluationException if there is any problem invoking the method
 	 */
 	private TypedValue executeFunctionJLRMethod(ExpressionState state, Method method) throws EvaluationException {
-		this.method = null;
 		Object[] functionArgs = getArguments(state);
 
 		if (!method.isVarArgs() && method.getParameterCount() != functionArgs.length) {
@@ -112,25 +110,33 @@ private TypedValue executeFunctionJLRMethod(ExpressionState state, Method method
 
 		// Convert arguments if necessary and remap them for varargs if required
 		TypeConverter converter = state.getEvaluationContext().getTypeConverter();
-		argumentConversionOccurred = ReflectionHelper.convertAllArguments(converter, functionArgs, method);
+		boolean argumentConversionOccurred = ReflectionHelper.convertAllArguments(converter, functionArgs, method);
 		if (method.isVarArgs()) {
 			functionArgs = ReflectionHelper.setupArgumentsForVarargsInvocation(
 					method.getParameterTypes(), functionArgs);
 		}
+		boolean compilable = false;
 
 		try {
 			ReflectionUtils.makeAccessible(method);
 			Object result = method.invoke(method.getClass(), functionArgs);
-			if (!argumentConversionOccurred) {
-				this.method = method;
-				this.exitTypeDescriptor = CodeFlow.toDescriptor(method.getReturnType());
-			}
+			compilable = !argumentConversionOccurred;
 			return new TypedValue(result, new TypeDescriptor(new MethodParameter(method, -1)).narrow(result));
 		}
 		catch (Exception ex) {
 			throw new SpelEvaluationException(getStartPosition(), ex, SpelMessage.EXCEPTION_DURING_FUNCTION_CALL,
 					this.name, ex.getMessage());
 		}
+		finally {
+			if (compilable) {
+				this.exitTypeDescriptor = CodeFlow.toDescriptor(method.getReturnType());
+				this.method = method;
+			}
+			else {
+				this.exitTypeDescriptor = null;
+				this.method = null;
+			}
+		}
 	}
 
 	@Override
@@ -162,12 +168,13 @@ private Object[] getArguments(ExpressionState state) throws EvaluationException
 
 	@Override
 	public boolean isCompilable() {
-		if (this.method == null || this.argumentConversionOccurred) {
+		Method method = this.method;
+		if (method == null) {
 			return false;
 		}
-		int methodModifiers = this.method.getModifiers();
+		int methodModifiers = method.getModifiers();
 		if (!Modifier.isStatic(methodModifiers) || !Modifier.isPublic(methodModifiers) ||
-				!Modifier.isPublic(this.method.getDeclaringClass().getModifiers())) {
+				!Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
 			return false;
 		}
 		for (SpelNodeImpl child : this.children) {
@@ -179,12 +186,13 @@ public boolean isCompilable() {
 	}
 
 	@Override 
-	public void generateCode(MethodVisitor mv,CodeFlow cf) {
-		Assert.state(this.method != null, "No method handle");
-		String classDesc = this.method.getDeclaringClass().getName().replace('.', '/');
-		generateCodeForArguments(mv, cf, this.method, this.children);
-		mv.visitMethodInsn(INVOKESTATIC, classDesc, this.method.getName(),
-				CodeFlow.createSignatureDescriptor(this.method), false);
+	public void generateCode(MethodVisitor mv, CodeFlow cf) {
+		Method method = this.method;
+		Assert.state(method != null, "No method handle");
+		String classDesc = method.getDeclaringClass().getName().replace('.', '/');
+		generateCodeForArguments(mv, cf, method, this.children);
+		mv.visitMethodInsn(INVOKESTATIC, classDesc, method.getName(),
+				CodeFlow.createSignatureDescriptor(method), false);
 		cf.pushDescriptor(this.exitTypeDescriptor);
 	}