j-easy / easy-rules

The simple, stupid rules engine for Java
https://github.com/j-easy/easy-rules/wiki
MIT License
4.83k stars 1.04k forks source link

A thread safety problem occur when multiple threads share the Rules #418

Open xiaobai6163 opened 8 months ago

xiaobai6163 commented 8 months ago

When accessing through annotations (i.e., generating proxy classes), there is a thread safety problem occur when multiple threads share the Rules

Rule 'rule' performed with error java.util.ConcurrentModificationException
at java.util.TreeMap$PrivateEntryIterator.nextEntry(TreeMap.java:1211)
at java.util.TreeMap$KeyIterator.next(TreeMap.java:1265)
at org.jeasy.rules.core.RuleProxy.executeMethod(RuleProxy.java:140)
at org.jeasy.rules.core.RuleProxy.invoke(RuleProxy.java:109)
at com.sun.proxy.$Proxy142.execute(Unknown Source)
at org.jeasy.rules.support.composite.ActivationRuleGroup.execute(ActivationRuleGroup.java:98)
at org.jeasy.rules.core.DefaultRulesEngine.doFire(DefaultRulesEngine.java:112)
at org.jeasy.rules.core.DefaultRulesEngine.fire(DefaultRulesEngine.java:70)

This is the problematic source code:

package org.jeasy.rules.core; public class RuleProxy implements InvocationHandler { //xxxxx private Set getActionMethodBeans() { if (this.actionMethods == null) { this.actionMethods = new TreeSet<>(); Method[] methods = getMethods(); for (Method method : methods) { if (method.isAnnotationPresent(Action.class)) { Action actionAnnotation = method.getAnnotation(Action.class); int order = actionAnnotation.order(); this.actionMethods.add(new ActionMethodOrderBean(method, order)); } } } return this.actionMethods; } }

I think a simpler solution would be to add double-checked lock when execute "this.actionMethods = new TreeSet<>();" , to check if "this.actionMethods" is still null.

I have been waiting here for your response. Thank you!!!

dvgaba commented 5 months ago

Could you please share a sample ?