[groovy] branch GROOVY_3_0_X updated: GROOVY-9397: Make respondsTo thread safe

[su...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
     new f3d4005  GROOVY-9397: Make respondsTo thread safe
f3d4005 is described below

commit f3d400574e43df7bbdc0e9136f13e756a3596266
Author: Benjamin Roe <be...@cirrus.com>
AuthorDate: Thu Mar 11 11:37:05 2021 +0000

    GROOVY-9397: Make respondsTo thread safe
    
    By changing the initialized state of the Closure's Metaclass without any
    locking, if the Closure is visible to another thread then any call to
    invokeMethod can see an uninitialized closure, causing an exception.
    
    This fix removes the need to change the initialized state of the
    closure.
    
    (cherry picked from commit be55ede40d9ab1b60c4ac9074ef3bce25a6db74e)
---
 src/main/java/groovy/lang/MetaClassImpl.java       | 26 +++++++++++++---------
 .../groovy/runtime/metaclass/ClosureMetaClass.java |  4 +---
 src/test/groovy/ClosureMethodCallTest.groovy       | 19 ++++++++++++++++
 3 files changed, 35 insertions(+), 14 deletions(-)

diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index a01022f..1b71bc0 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -3432,18 +3432,22 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
      */
     public synchronized void initialize() {
         if (!isInitialized()) {
-            fillMethodIndex();
-            try {
-                addProperties();
-            } catch (Throwable e) {
-                if (!AndroidSupport.isRunningAndroid()) {
-                    UncheckedThrow.rethrow(e);
-                }
-                // Introspection failure...
-                // May happen in Android
-            }
-            setInitialized(true);
+          reinitialize();
+        }
+    }
+
+    protected synchronized void reinitialize() {
+      fillMethodIndex();
+      try {
+        addProperties();
+      } catch (Throwable e) {
+        if (!AndroidSupport.isRunningAndroid()) {
+          UncheckedThrow.rethrow(e);
         }
+        // Introspection failure...
+        // May happen in Android
+      }
+      setInitialized(true);
     }
 
     private void addProperties() {
diff --git a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
index af0bf2b..fafb786 100644
--- a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
+++ b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java
@@ -696,9 +696,7 @@ public final class ClosureMetaClass extends MetaClassImpl {
 
     private synchronized void loadMetaInfo() {
         if (metaMethodIndex.isEmpty()) {
-            initialized = false;
-            super.initialize();
-            initialized = true;
+          reinitialize();
         }
     }
 
diff --git a/src/test/groovy/ClosureMethodCallTest.groovy b/src/test/groovy/ClosureMethodCallTest.groovy
index d442373..feac6bf 100644
--- a/src/test/groovy/ClosureMethodCallTest.groovy
+++ b/src/test/groovy/ClosureMethodCallTest.groovy
@@ -20,6 +20,9 @@ package groovy
 
 import groovy.test.GroovyTestCase
 
+import java.util.concurrent.Executor
+import java.util.concurrent.Executors
+
 class ClosureMethodCallTest extends GroovyTestCase {
 
     void testCallingClosureWithMultipleArguments() {
@@ -119,4 +122,20 @@ class ClosureMethodCallTest extends GroovyTestCase {
             assert ref(1) == 1
         '''
     }
+
+    //GROOVY-9397
+    void testRespondsToIsThreadSafe() {
+      final Executor executor = Executors.newCachedThreadPool()
+      try {
+        final Closure action = { -> }
+        // ensure that executing the closure and calling respondsTo
+        // concurrently doesn't throw an exception.
+        for (int i = 0; i < 500; ++i) {
+          executor.execute(action)
+          executor.execute(() -> action.respondsTo('test'))
+        }
+      } finally {
+        executor.shutdownNow();
+      }
+    }
 }