Merge pull request #106 from rolandhe/master

resolve race condition problem

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<String,Method> methodsByPropertyName = this.cache.get(clazz);
             if (methodsByPropertyName == null)
             {
-                methodsByPropertyName = new ConcurrentHashMap<String, Method>();
-                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<String, Method>();
-                this.cache.put(clazz, methodsByPropertyName);
+                ConcurrentHashMap<String,Method> 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<String,Method> methodsByPropertyName = this.cache.get(clazz);
-            if (methodsByPropertyName == null)
-                return false;
-
-            return methodsByPropertyName.containsKey(propertyName);
-        }
 
         /**
          * Allow clearing for the underlying cache of the ClassPropertyMethodCache.

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;
+    }
+}

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;
+    }
+}

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();
+        }
+    }
+
+}