Skip to content

Commit c508a70

Browse files
jhoellerjhoeller
committed
MethodReference accesses cached executor in a thread-safe manner
Issue: SPR-12269
1 parent bb45fb4 commit c508a70

File tree

2 files changed

+115
-92
lines changed

2 files changed

+115
-92
lines changed

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

Lines changed: 89 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,7 @@ public TypedValue getValueInternal(ExpressionState state) throws EvaluationExcep
8383
TypeDescriptor targetType = state.getActiveContextObject().getTypeDescriptor();
8484
Object[] arguments = getArguments(state);
8585
TypedValue result = getValueInternal(evaluationContext, value, targetType, arguments);
86-
if (cachedExecutor.get() instanceof ReflectiveMethodExecutor) {
87-
ReflectiveMethodExecutor executor = (ReflectiveMethodExecutor) cachedExecutor.get();
88-
Method method = executor.getMethod();
89-
exitTypeDescriptor = CodeFlow.toDescriptor(method.getReturnType());
90-
}
86+
updateExitTypeDescriptor();
9187
return result;
9288
}
9389

@@ -172,7 +168,9 @@ private List<TypeDescriptor> getArgumentTypes(Object... arguments) {
172168
return Collections.unmodifiableList(descriptors);
173169
}
174170

175-
private MethodExecutor getCachedExecutor(EvaluationContext evaluationContext, Object value, TypeDescriptor target, List<TypeDescriptor> argumentTypes) {
171+
private MethodExecutor getCachedExecutor(EvaluationContext evaluationContext, Object value,
172+
TypeDescriptor target, List<TypeDescriptor> argumentTypes) {
173+
176174
List<MethodResolver> methodResolvers = evaluationContext.getMethodResolvers();
177175
if (methodResolvers == null || methodResolvers.size() != 1 ||
178176
!(methodResolvers.get(0) instanceof ReflectiveMethodResolver)) {
@@ -230,6 +228,14 @@ private void throwSimpleExceptionIfPossible(Object value, AccessException ae) {
230228
}
231229
}
232230

231+
private void updateExitTypeDescriptor() {
232+
CachedMethodExecutor executorToCheck = this.cachedExecutor;
233+
if (executorToCheck.get() instanceof ReflectiveMethodExecutor) {
234+
Method method = ((ReflectiveMethodExecutor) executorToCheck.get()).getMethod();
235+
this.exitTypeDescriptor = CodeFlow.toDescriptor(method.getReturnType());
236+
}
237+
}
238+
233239
@Override
234240
public String toStringAST() {
235241
StringBuilder sb = new StringBuilder();
@@ -244,6 +250,80 @@ public String toStringAST() {
244250
return sb.toString();
245251
}
246252

253+
/**
254+
* A method reference is compilable if it has been resolved to a reflectively accessible method
255+
* and the child nodes (arguments to the method) are also compilable.
256+
*/
257+
@Override
258+
public boolean isCompilable() {
259+
CachedMethodExecutor executorToCheck = this.cachedExecutor;
260+
if (executorToCheck == null || !(executorToCheck.get() instanceof ReflectiveMethodExecutor)) {
261+
return false;
262+
}
263+
264+
for (SpelNodeImpl child : this.children) {
265+
if (!child.isCompilable()) {
266+
return false;
267+
}
268+
}
269+
270+
ReflectiveMethodExecutor executor = (ReflectiveMethodExecutor) executorToCheck.get();
271+
Method method = executor.getMethod();
272+
if (!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
273+
return false;
274+
}
275+
if (method.isVarArgs()) {
276+
return false;
277+
}
278+
if (executor.didArgumentConversionOccur()) {
279+
return false;
280+
}
281+
282+
return true;
283+
}
284+
285+
@Override
286+
public void generateCode(MethodVisitor mv, CodeFlow codeflow) {
287+
CachedMethodExecutor executorToCheck = this.cachedExecutor;
288+
if (executorToCheck == null || !(executorToCheck.get() instanceof ReflectiveMethodExecutor)) {
289+
throw new IllegalStateException("No applicable cached executor found: " + executorToCheck);
290+
}
291+
292+
Method method = ((ReflectiveMethodExecutor) executorToCheck.get()).getMethod();
293+
boolean isStaticMethod = Modifier.isStatic(method.getModifiers());
294+
String descriptor = codeflow.lastDescriptor();
295+
296+
if (descriptor == null && !isStaticMethod) {
297+
codeflow.loadTarget(mv);
298+
}
299+
300+
boolean itf = method.getDeclaringClass().isInterface();
301+
String methodDeclaringClassSlashedDescriptor = method.getDeclaringClass().getName().replace('.', '/');
302+
if (!isStaticMethod) {
303+
if (descriptor == null || !descriptor.equals(methodDeclaringClassSlashedDescriptor)) {
304+
mv.visitTypeInsn(CHECKCAST, methodDeclaringClassSlashedDescriptor);
305+
}
306+
}
307+
String[] paramDescriptors = CodeFlow.toParamDescriptors(method);
308+
for (int i = 0; i < this.children.length;i++) {
309+
SpelNodeImpl child = this.children[i];
310+
codeflow.enterCompilationScope();
311+
child.generateCode(mv, codeflow);
312+
// Check if need to box it for the method reference?
313+
if (CodeFlow.isPrimitive(codeflow.lastDescriptor()) && paramDescriptors[i].charAt(0) == 'L') {
314+
CodeFlow.insertBoxIfNecessary(mv, codeflow.lastDescriptor().charAt(0));
315+
}
316+
else if (!codeflow.lastDescriptor().equals(paramDescriptors[i])) {
317+
// This would be unnecessary in the case of subtyping (e.g. method takes Number but Integer passed in)
318+
CodeFlow.insertCheckCast(mv, paramDescriptors[i]);
319+
}
320+
codeflow.exitCompilationScope();
321+
}
322+
mv.visitMethodInsn(isStaticMethod ? INVOKESTATIC : INVOKEVIRTUAL,
323+
methodDeclaringClassSlashedDescriptor, method.getName(), CodeFlow.createSignatureDescriptor(method), itf);
324+
codeflow.pushDescriptor(this.exitTypeDescriptor);
325+
}
326+
247327

248328
private class MethodValueRef implements ValueRef {
249329

@@ -264,12 +344,9 @@ public MethodValueRef(ExpressionState state, Object[] arguments) {
264344

265345
@Override
266346
public TypedValue getValue() {
267-
TypedValue result = MethodReference.this.getValueInternal(this.evaluationContext, this.value, this.targetType, this.arguments);
268-
if (MethodReference.this.cachedExecutor.get() instanceof ReflectiveMethodExecutor) {
269-
ReflectiveMethodExecutor executor = (ReflectiveMethodExecutor) MethodReference.this.cachedExecutor.get();
270-
Method method = executor.getMethod();
271-
MethodReference.this.exitTypeDescriptor = CodeFlow.toDescriptor(method.getReturnType());
272-
}
347+
TypedValue result = MethodReference.this.getValueInternal(
348+
this.evaluationContext, this.value, this.targetType, this.arguments);
349+
updateExitTypeDescriptor();
273350
return result;
274351
}
275352

@@ -313,70 +390,4 @@ public MethodExecutor get() {
313390
}
314391
}
315392

316-
/**
317-
* A method reference is compilable if it has been resolved to a reflectively accessible method
318-
* and the child nodes (arguments to the method) are also compilable.
319-
*/
320-
@Override
321-
public boolean isCompilable() {
322-
if (this.cachedExecutor == null || !(this.cachedExecutor.get() instanceof ReflectiveMethodExecutor)) {
323-
return false;
324-
}
325-
for (SpelNodeImpl child: children) {
326-
if (!child.isCompilable()) {
327-
return false;
328-
}
329-
}
330-
ReflectiveMethodExecutor executor = (ReflectiveMethodExecutor) this.cachedExecutor.get();
331-
Method method = executor.getMethod();
332-
if (!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers())) {
333-
return false;
334-
}
335-
if (method.isVarArgs()) {
336-
return false;
337-
}
338-
if (executor.didArgumentConversionOccur()) {
339-
return false;
340-
}
341-
return true;
342-
}
343-
344-
@Override
345-
public void generateCode(MethodVisitor mv,CodeFlow codeflow) {
346-
ReflectiveMethodExecutor executor = (ReflectiveMethodExecutor) this.cachedExecutor.get();
347-
Method method = executor.getMethod();
348-
boolean isStaticMethod = Modifier.isStatic(method.getModifiers());
349-
String descriptor = codeflow.lastDescriptor();
350-
351-
if (descriptor == null && !isStaticMethod) {
352-
codeflow.loadTarget(mv);
353-
}
354-
355-
boolean itf = method.getDeclaringClass().isInterface();
356-
String methodDeclaringClassSlashedDescriptor = method.getDeclaringClass().getName().replace('.','/');
357-
if (!isStaticMethod) {
358-
if (descriptor == null || !descriptor.equals(methodDeclaringClassSlashedDescriptor)) {
359-
mv.visitTypeInsn(CHECKCAST, methodDeclaringClassSlashedDescriptor);
360-
}
361-
}
362-
String[] paramDescriptors = CodeFlow.toParamDescriptors(method);
363-
for (int c = 0; c < this.children.length;c++) {
364-
SpelNodeImpl child = this.children[c];
365-
codeflow.enterCompilationScope();
366-
child.generateCode(mv, codeflow);
367-
// Check if need to box it for the method reference?
368-
if (CodeFlow.isPrimitive(codeflow.lastDescriptor()) && (paramDescriptors[c].charAt(0)=='L')) {
369-
CodeFlow.insertBoxIfNecessary(mv, codeflow.lastDescriptor().charAt(0));
370-
}
371-
else if (!codeflow.lastDescriptor().equals(paramDescriptors[c])) {
372-
// This would be unnecessary in the case of subtyping (e.g. method takes a Number but passed in is an Integer)
373-
CodeFlow.insertCheckCast(mv, paramDescriptors[c]);
374-
}
375-
codeflow.exitCompilationScope();
376-
}
377-
mv.visitMethodInsn(isStaticMethod ? INVOKESTATIC : INVOKEVIRTUAL,
378-
methodDeclaringClassSlashedDescriptor, method.getName(), CodeFlow.createSignatureDescriptor(method), itf);
379-
codeflow.pushDescriptor(this.exitTypeDescriptor);
380-
}
381-
382393
}

‎spring-expression/src/main/java/org/springframework/expression/spel/standard/SpelCompiler.java

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -103,17 +103,18 @@ public CompiledExpression compile(SpelNodeImpl expression) {
103103
logger.debug("SpEL: compiling " + expression.toStringAST());
104104
}
105105
Class<? extends CompiledExpression> clazz = createExpressionClass(expression);
106-
try {
107-
return clazz.newInstance();
108-
}
109-
catch (Exception ex) {
110-
throw new IllegalStateException("Failed to instantiate CompiledExpression", ex);
106+
if (clazz != null) {
107+
try {
108+
return clazz.newInstance();
109+
}
110+
catch (Throwable ex) {
111+
throw new IllegalStateException("Failed to instantiate CompiledExpression", ex);
112+
}
111113
}
112114
}
113-
else {
114-
if (logger.isDebugEnabled()) {
115-
logger.debug("SpEL: unable to compile " + expression.toStringAST());
116-
}
115+
116+
if (logger.isDebugEnabled()) {
117+
logger.debug("SpEL: unable to compile " + expression.toStringAST());
117118
}
118119
return null;
119120
}
@@ -123,9 +124,11 @@ private int getNextSuffix() {
123124
}
124125

125126
/**
126-
* Generate the class that encapsulates the compiled expression and define it. The
127-
* generated class will be a subtype of CompiledExpression.
127+
* Generate the class that encapsulates the compiled expression and define it.
128+
* The generated class will be a subtype of CompiledExpression.
128129
* @param expressionToCompile the expression to be compiled
130+
* @return the expression call, or {@code null} if the decision was to opt out of
131+
* compilation during code generation
129132
*/
130133
@SuppressWarnings("unchecked")
131134
private Class<? extends CompiledExpression> createExpressionClass(SpelNodeImpl expressionToCompile) {
@@ -150,10 +153,19 @@ private Class<? extends CompiledExpression> createExpressionClass(SpelNodeImpl e
150153

151154
CodeFlow codeflow = new CodeFlow();
152155

153-
// Ask the expression Ast to generate the body of the method
154-
expressionToCompile.generateCode(mv,codeflow);
156+
// Ask the expression AST to generate the body of the method
157+
try {
158+
expressionToCompile.generateCode(mv, codeflow);
159+
}
160+
catch (IllegalStateException ex) {
161+
if (logger.isDebugEnabled()) {
162+
logger.debug(expressionToCompile.getClass().getSimpleName() +
163+
".generateCode opted out of compilation: " + ex.getMessage());
164+
}
165+
return null;
166+
}
155167

156-
CodeFlow.insertBoxIfNecessary(mv,codeflow.lastDescriptor());
168+
CodeFlow.insertBoxIfNecessary(mv, codeflow.lastDescriptor());
157169
if ("V".equals(codeflow.lastDescriptor())) {
158170
mv.visitInsn(ACONST_NULL);
159171
}

0 commit comments

Comments
 (0)