From 3fbb901100649fe4a1dcde6f20f528f3ba87b726 Mon Sep 17 00:00:00 2001
From: Iwao AVE! <harawata@gmail.com>
Date: Mon, 13 Jan 2020 00:30:55 +0900
Subject: [PATCH] Avoid flipping accessible flag in OgnlMemberAccess

Once a field is made accessible, we keep it accessible.
Flipping accessible flag causes a race condition.
Possibly related to #1648 .
---
 .../scripting/xmltags/OgnlMemberAccess.java   |  6 +--
 .../scripting/xmltags/OgnlCacheTest.java      | 54 +++++++++++++++++++
 2 files changed, 56 insertions(+), 4 deletions(-)
 create mode 100644 src/test/java/org/apache/ibatis/scripting/xmltags/OgnlCacheTest.java

diff --git a/src/main/java/org/apache/ibatis/scripting/xmltags/OgnlMemberAccess.java b/src/main/java/org/apache/ibatis/scripting/xmltags/OgnlMemberAccess.java
index 3c54612dbba..2631e3a23b0 100644
--- a/src/main/java/org/apache/ibatis/scripting/xmltags/OgnlMemberAccess.java
+++ b/src/main/java/org/apache/ibatis/scripting/xmltags/OgnlMemberAccess.java
@@ -1,5 +1,5 @@
 /**
- *    Copyright 2009-2019 the original author or authors.
+ *    Copyright 2009-2020 the original author or authors.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -58,9 +58,7 @@ public Object setup(Map context, Object target, Member member, String propertyNa
   @Override
   public void restore(Map context, Object target, Member member, String propertyName,
       Object state) {
-    if (state != null) {
-      ((AccessibleObject) member).setAccessible((Boolean) state);
-    }
+    // Flipping accessible flag is not thread safe. See #1648
   }
 
   @Override
diff --git a/src/test/java/org/apache/ibatis/scripting/xmltags/OgnlCacheTest.java b/src/test/java/org/apache/ibatis/scripting/xmltags/OgnlCacheTest.java
new file mode 100644
index 00000000000..538a7e6cdcb
--- /dev/null
+++ b/src/test/java/org/apache/ibatis/scripting/xmltags/OgnlCacheTest.java
@@ -0,0 +1,54 @@
+/**
+ *    Copyright 2009-2020 the original author or authors.
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *       http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+
+package org.apache.ibatis.scripting.xmltags;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.stream.IntStream;
+
+import org.junit.jupiter.api.Test;
+
+public class OgnlCacheTest {
+  @Test
+  void concurrentAccess() throws Exception {
+    class DataClass {
+      @SuppressWarnings("unused")
+      private int id;
+    }
+    int run = 1000;
+    Map<String, Object> context = new HashMap<>();
+    List<Future<Object>> futures = new ArrayList<>();
+    context.put("data", new DataClass());
+    ExecutorService executor = Executors.newCachedThreadPool();
+    IntStream.range(0, run).forEach(i -> {
+      futures.add(executor.submit(() -> {
+        return OgnlCache.getValue("data.id", context);
+      }));
+    });
+    for (int i = 0; i < run; i++) {
+      assertNotNull(futures.get(i).get());
+    }
+    executor.shutdown();
+  }
+}