Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StandardEvaluationContext does not support concurrent variable access [SPR-17448] #21980

Closed
spring-issuemaster opened this Issue Oct 31, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Oct 31, 2018

chenzhiguo opened SPR-17448 and commented

I invoke a method with SpEL. Sometimes, the param used by the method i invoke is null!!

 

@Test
public void testAsyncBuildNull() {
    int i = 0;
    PageRequest param = new PageRequest();
    param.setPin("chenzhiguo");
    for (; i < 5000; i++) {
        defaultSpELParser.setVariable("param", param);
        defaultSpELParser.setVariable("param"+ i,defaultSpELParser.doAsyncParser("@extendDemoService.syncGetJsfData2(#param)", Object.class));
        System.out.println("######Start:" + i);
    }
    for (int x=0; x < 5000; x++) {
        try {
System.out.println("Result:"+defaultSpELParser.doParserByTemplate("#{#param" + x +".get()}"));
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
}
@Async
@Override
public <T> Future<T> doAsyncParser(String expression, Class<T> beanType) {
    if (null == expression) {
        expression = "";
    }
    return new AsyncResult<>(expressionParser.parseExpression(expression)
            .getValue(evaluationContext, beanType));
}
public String syncGetJsfData2(PageRequest param) {
    if (null == param) {
        LOGGER.error("param is null"); // always goto here at the first or second time when i test 5000 times
        return "null";
    } 
    return param.getPin();
}

I think the "StandardEvaluationContext"->variables = new HashMap<>();  Is it thread  unsafe  when the SpEL working?

So, I extend StandardEvaluationContext to MyEvaluationContext and it working properly.

public class MyEvaluationContext extends StandardEvaluationContext {

    private final Map<String, Object> variables = new ConcurrentHashMap<>();

    @Override
    public void setVariable(String name, @Nullable Object value) {
        this.variables.put(name, value);
    }

    @Override
    public void setVariables(Map<String,Object> variables) {
        this.variables.putAll(variables);
    }

    @Override
    public void registerFunction(String name, Method method) {
        this.variables.put(name, method);
    }

    @Override
    @Nullable
    public Object lookupVariable(String name) {
        return this.variables.get(name);
    }
}

But I'm not sure exactly why. I look forward to your reply.

Thanks.


Affects: 5.0.5

Issue Links:

  • #21974 ConcurrentModificationException in DispatcherServlet with asynchronous ApplicationEventMulticaster
  • #21819 Concurrency Exception during bean configuration related to not thread safe getBeanPostProcessor access
  • #22097 SpEL variable evaluation fails with NPE against ConcurrentHashMap

Referenced from: commits 591e7f1, 59fa647

Backported to: 5.0.11

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 31, 2018

Juergen Hoeller commented

StandardEvaluationContext is not designed for mutability in a multi-threaded environment, rather for populating it once and then (in populated form) being shared among multiple threads. Do you intend to expose runtime variable changes to all threads? Are you using the evaluation context as a sort of runtime attribute model?

Looking at your code, the Expression instances are meant to be shared, only calling parseExpression once, whereas the EvaluationContext is usually freshly built for every evaluation attempt if the variables differ.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Oct 31, 2018

chenzhiguo commented

Juergen HoellerIf I need to use StandardEvaluationContext in multithreaded environment, are there any existing implementation classes? What happens now is that my system occasionally encounters a null exception.

In my production environment, parseExpression is called multiple times and the expression template differ.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 1, 2018

Juergen Hoeller commented

The key question is: Why are you sharing a mutable EvaluationContext instance to begin with? Sharing a pre-built EvaluationContext instance (populating its variables before any evaluation happens) is quite common, but sharing an instance and mutating its variables on the fly (with concurrent evaluation happening in other threads) is not originally intended there... We may declare the variable map as concurrent, but I'd also like to understand what specifically you are trying to accomplish with this arrangement.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 2, 2018

chenzhiguo commented

Juergen Hoeller The usage scenario goes like this: I'm going to call N methods to get different data, and I am call these methods with same ExpressionParser. But, the business methods be invoked that are executed that have the same parameters where be stored in  EvaluationContext -> variables.  To improve performance, I've encapsulated them: 

@Async
@Override
public <T> Future<T> doAsyncParser(String expression, Class<T> beanType) {
    if (null == expression) {
        expression = "";
    }
    return new AsyncResult<>(expressionParser.parseExpression(expression)
            .getValue(evaluationContext, beanType));
}

So, the anomaly described above has occurred.

 

 

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Nov 5, 2018

Juergen Hoeller commented

Since StandardEvaluationContext has some basic preparations for concurrent use already, I've simply added concurrent variable access to it through the use of a ConcurrentHashMap plus special null value handling. This will be available in the upcoming 5.1.3.BUILD-SNAPSHOT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.