diff --git a/src/main/java/ognl/OgnlRuntime.java b/src/main/java/ognl/OgnlRuntime.java index f2a6813d..b81ebdfd 100644 --- a/src/main/java/ognl/OgnlRuntime.java +++ b/src/main/java/ognl/OgnlRuntime.java @@ -2764,13 +2764,12 @@ public static Method getGetMethod(OgnlContext context, Class targetClass, String { // Cache is a map in two levels, so we provide two keys (see comments in ClassPropertyMethodCache below) Method method = cacheGetMethod.get(targetClass, propertyName); + if (method == ClassPropertyMethodCache.NULL_REPLACEMENT) { + return null; + } if (method != null) return method; - // By checking key existence now and not before calling 'get', we will save a map resolution 90% of the times - if (cacheGetMethod.containsKey(targetClass, propertyName)) - return null; - method = _getGetMethod(context, targetClass, propertyName); // will be null if not found - will cache it anyway cacheGetMethod.put(targetClass, propertyName, method); @@ -2865,12 +2864,15 @@ public static Method getSetMethod(OgnlContext context, Class targetClass, String { // Cache is a map in two levels, so we provide two keys (see comments in ClassPropertyMethodCache below) Method method = cacheSetMethod.get(targetClass, propertyName); + if (method == ClassPropertyMethodCache.NULL_REPLACEMENT) { + return null; + } if (method != null) return method; // By checking key existence now and not before calling 'get', we will save a map resolution 90% of the times - if (cacheSetMethod.containsKey(targetClass, propertyName)) - return null; +// if (cacheSetMethod.containsKey(targetClass, propertyName)) +// return null; method = _getSetMethod(context, targetClass, propertyName); // will be null if not found - will cache it anyway cacheSetMethod.put(targetClass, propertyName, method); @@ -3945,12 +3947,9 @@ Method get(Class clazz, String propertyName) ConcurrentHashMap methodsByPropertyName = this.cache.get(clazz); if (methodsByPropertyName == null) { - methodsByPropertyName = new ConcurrentHashMap(); - this.cache.put(clazz, methodsByPropertyName); + return null; } Method method = methodsByPropertyName.get(propertyName); - if (method == NULL_REPLACEMENT) - return null; return method; } @@ -3960,19 +3959,14 @@ void put(Class clazz, String propertyName, Method method) if (methodsByPropertyName == null) { methodsByPropertyName = new ConcurrentHashMap(); - this.cache.put(clazz, methodsByPropertyName); + ConcurrentHashMap old = this.cache.putIfAbsent(clazz, methodsByPropertyName); + if (null != old) { + methodsByPropertyName = old; + } } - methodsByPropertyName.put(propertyName, (method == null? NULL_REPLACEMENT : method)); + methodsByPropertyName.putIfAbsent(propertyName, (method == null? NULL_REPLACEMENT : method)); } - boolean containsKey(Class clazz, String propertyName) - { - ConcurrentHashMap methodsByPropertyName = this.cache.get(clazz); - if (methodsByPropertyName == null) - return false; - - return methodsByPropertyName.containsKey(propertyName); - } /** * Allow clearing for the underlying cache of the ClassPropertyMethodCache. diff --git a/src/test/java/ognl/race/Base.java b/src/test/java/ognl/race/Base.java new file mode 100644 index 00000000..b8b09db4 --- /dev/null +++ b/src/test/java/ognl/race/Base.java @@ -0,0 +1,15 @@ +package ognl.race; + +/** + * + */ +public class Base { + private Boolean yn = true; + public Boolean getYn() { + return yn; + } + + public void setYn(Boolean yn) { + this.yn = yn; + } +} diff --git a/src/test/java/ognl/race/Persion.java b/src/test/java/ognl/race/Persion.java new file mode 100644 index 00000000..d96572c5 --- /dev/null +++ b/src/test/java/ognl/race/Persion.java @@ -0,0 +1,14 @@ +package ognl.race; + +public class Persion extends Base{ + private String name = "abc"; + + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} diff --git a/src/test/java/ognl/race/RaceTestCase.java b/src/test/java/ognl/race/RaceTestCase.java new file mode 100644 index 00000000..2ac790c5 --- /dev/null +++ b/src/test/java/ognl/race/RaceTestCase.java @@ -0,0 +1,74 @@ +package ognl.race; + + +import ognl.DefaultMemberAccess; +import ognl.Ognl; +import ognl.OgnlContext; +import ognl.OgnlException; +import org.junit.Assert; +import org.junit.Test; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicInteger; + + +public class RaceTestCase { + + @Test + public void testOgnlRace(){ + int concurrent = 128; + final int batchCount = 2000; + final CountDownLatch start = new CountDownLatch(1); + final CountDownLatch wait = new CountDownLatch(concurrent); + final AtomicInteger errCount = new AtomicInteger(0); + + final Persion persion = new Persion(); + for (int i = 0; i < concurrent;i++){ + Thread thread = new Thread(new Runnable() { + @Override + public void run() { + try { + start.await(); + } catch (InterruptedException e) { + e.printStackTrace(); + } + for(int j = 0; j < batchCount;j++){ + if(j % 2 == 0) { + runValue(persion, "yn", errCount); + } else { + runValue(persion, "name", errCount); + } + } + wait.countDown(); + } + }); + thread.setName("work-"+i); + thread.start(); + } + start.countDown(); + try { + wait.await(); + } catch (InterruptedException e) { + e.printStackTrace(); + } +// System.out.println("error:" + errCount.get()); + Assert.assertTrue(errCount.get() == 0); + } + + + + private void runValue(Persion persion,String name,AtomicInteger errCount) { + OgnlContext context = new OgnlContext(null,null,new DefaultMemberAccess(false)); + context.setRoot(persion); + try { + Object value = Ognl.getValue(name, context, context.getRoot()); +// System.out.println(value); + + } catch (OgnlException e) { + errCount.incrementAndGet(); + // TODO Auto-generated catch block + e.printStackTrace(); + } + } + +}