You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2022/08/28 15:07:49 UTC
[groovy] branch master updated (69b88f0bfd -> 0628b2dd46)
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
from 69b88f0bfd GROOVY-10730: Bump Gradle to 7.5.1
new f391325399 GROOVY-10730: Remove the jdk16+ add-opens jvm args
new 0628b2dd46 GROOVY-10730: Fix accessing `clone` illegally
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../main/groovy/org.apache.groovy-tested.gradle | 6 --
src/main/java/groovy/lang/MetaClassImpl.java | 22 +-----
.../org/codehaus/groovy/runtime/ArrayUtil.java | 30 ++++++++
.../org/codehaus/groovy/runtime/InvokerHelper.java | 4 ++
.../org/codehaus/groovy/vmplugin/v8/Selector.java | 11 +++
src/test/groovy/inspect/InspectorTest.java | 14 +++-
.../groovy/transform/ImmutableTransformTest.groovy | 8 +--
.../groovy/text/StreamingTemplateEngineTest.groovy | 80 ++++++++++++----------
8 files changed, 103 insertions(+), 72 deletions(-)
[groovy] 02/02: GROOVY-10730: Fix accessing `clone` illegally
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 0628b2dd46e6ceebfc735b1767d01b29fccf5818
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sun Aug 28 22:24:38 2022 +0800
GROOVY-10730: Fix accessing `clone` illegally
---
src/main/java/groovy/lang/MetaClassImpl.java | 22 +-----
.../org/codehaus/groovy/runtime/ArrayUtil.java | 30 ++++++++
.../org/codehaus/groovy/runtime/InvokerHelper.java | 4 ++
.../org/codehaus/groovy/vmplugin/v8/Selector.java | 11 +++
src/test/groovy/inspect/InspectorTest.java | 14 +++-
.../groovy/transform/ImmutableTransformTest.groovy | 8 +--
.../groovy/text/StreamingTemplateEngineTest.groovy | 80 ++++++++++++----------
7 files changed, 103 insertions(+), 66 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index c32102989d..14ca8081fb 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -1131,7 +1131,7 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
if (receiverClass.isArray()) {
if (methodName.equals("clone") && arguments.length == 0) {
try {
- return (Object[]) MethodHandleHolder.CLONE_ARRAY_METHOD_HANDLE.invokeExact((Object[]) object);
+ return (Object[]) ArrayUtil.getCloneArrayMethodHandle().invokeExact((Object[]) object);
} catch (Throwable t) {
throw new GroovyRuntimeException(t);
}
@@ -1164,26 +1164,6 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
}
- private static class MethodHandleHolder {
- private static final MethodHandle CLONE_ARRAY_METHOD_HANDLE;
- static {
- final Class<ArrayUtil> arrayUtilClass = ArrayUtil.class;
- Method cloneArrayMethod;
- try {
- cloneArrayMethod = arrayUtilClass.getDeclaredMethod("cloneArray", Object[].class);
- } catch (NoSuchMethodException e) {
- throw new GroovyBugError("Failed to find `cloneArray` method in class `" + arrayUtilClass.getName() + "`", e);
- }
-
- try {
- CLONE_ARRAY_METHOD_HANDLE = MethodHandles.lookup().in(arrayUtilClass).unreflect(cloneArrayMethod);
- } catch (IllegalAccessException e) {
- throw new GroovyBugError("Failed to create method handle for " + cloneArrayMethod);
- }
- }
- private MethodHandleHolder() {}
- }
-
private static final ClassValue<Map<String, Set<Method>>> SPECIAL_METHODS_MAP = new ClassValue<Map<String, Set<Method>>>() {
@Override
protected Map<String, Set<Method>> computeValue(final Class<?> type) {
diff --git a/src/main/java/org/codehaus/groovy/runtime/ArrayUtil.java b/src/main/java/org/codehaus/groovy/runtime/ArrayUtil.java
index 72bcd83c51..d35a4d80ea 100644
--- a/src/main/java/org/codehaus/groovy/runtime/ArrayUtil.java
+++ b/src/main/java/org/codehaus/groovy/runtime/ArrayUtil.java
@@ -18,6 +18,12 @@
*/
package org.codehaus.groovy.runtime;
+import org.codehaus.groovy.GroovyBugError;
+
+import java.lang.invoke.MethodHandle;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Method;
+
/**
* This is a generated class used internally during the writing of bytecode within the CallSiteWriter logic.
* This is not a class exposed to users, as is the case with almost all classes in the org.codehaus.groovy packages.
@@ -55,6 +61,30 @@ package org.codehaus.groovy.runtime;
public class ArrayUtil {
private static final Object[] EMPTY = new Object[0];
+ public static MethodHandle getCloneArrayMethodHandle() {
+ return MethodHandleHolder.CLONE_ARRAY_METHOD_HANDLE;
+ }
+
+ private static class MethodHandleHolder {
+ private static final MethodHandle CLONE_ARRAY_METHOD_HANDLE;
+ static {
+ final Class<ArrayUtil> arrayUtilClass = ArrayUtil.class;
+ Method cloneArrayMethod;
+ try {
+ cloneArrayMethod = arrayUtilClass.getDeclaredMethod("cloneArray", Object[].class);
+ } catch (NoSuchMethodException e) {
+ throw new GroovyBugError("Failed to find `cloneArray` method in class `" + arrayUtilClass.getName() + "`", e);
+ }
+
+ try {
+ CLONE_ARRAY_METHOD_HANDLE = MethodHandles.lookup().in(arrayUtilClass).unreflect(cloneArrayMethod);
+ } catch (IllegalAccessException e) {
+ throw new GroovyBugError("Failed to create method handle for " + cloneArrayMethod);
+ }
+ }
+ private MethodHandleHolder() {}
+ }
+
public static <T> T[] cloneArray(T[] array) {
return array.clone();
}
diff --git a/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java b/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
index 66926668b3..e39de8d48c 100644
--- a/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
+++ b/src/main/java/org/codehaus/groovy/runtime/InvokerHelper.java
@@ -586,6 +586,10 @@ public class InvokerHelper {
// it's an instance; check if it's a Java one
if (!(object instanceof GroovyObject)) {
+ if (null != object && object.getClass().isArray() && "clone".equals(methodName) && (null == arguments || arguments.getClass().isArray() && 0 == ((Object[]) arguments).length)) {
+ return ArrayUtil.cloneArray((Object[]) object);
+ }
+
return invokePojoMethod(object, methodName, arguments);
}
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
index 2352a1c21b..c5d75a4968 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
@@ -38,6 +38,7 @@ import org.codehaus.groovy.reflection.CachedMethod;
import org.codehaus.groovy.reflection.ClassInfo;
import org.codehaus.groovy.reflection.GeneratedMetaMethod;
import org.codehaus.groovy.reflection.stdclasses.CachedSAMClass;
+import org.codehaus.groovy.runtime.ArrayUtil;
import org.codehaus.groovy.runtime.GeneratedClosure;
import org.codehaus.groovy.runtime.GroovyCategorySupport;
import org.codehaus.groovy.runtime.GroovyCategorySupport.CategoryMethod;
@@ -673,9 +674,19 @@ public abstract class Selector {
}
}
+ private static final Method OBJECT_CLONE_METHOD;
+ static {
+ try {
+ OBJECT_CLONE_METHOD = Object.class.getDeclaredMethod("clone");
+ } catch (NoSuchMethodException e) {
+ throw new GroovyBugError(e); // should never happen
+ }
+ }
private MethodHandle correctClassForNameAndUnReflectOtherwise(Method m) throws IllegalAccessException {
if (m.getDeclaringClass() == Class.class && m.getName().equals("forName") && m.getParameterTypes().length == 1) {
return MethodHandles.insertArguments(CLASS_FOR_NAME, 1, true, sender.getClassLoader());
+ } else if (null != args && args.getClass().isArray() && OBJECT_CLONE_METHOD.equals(m)) {
+ return ArrayUtil.getCloneArrayMethodHandle();
} else {
return LOOKUP.unreflect(m);
}
diff --git a/src/test/groovy/inspect/InspectorTest.java b/src/test/groovy/inspect/InspectorTest.java
index 0bcc38530e..cc789e2117 100644
--- a/src/test/groovy/inspect/InspectorTest.java
+++ b/src/test/groovy/inspect/InspectorTest.java
@@ -21,7 +21,6 @@ package groovy.inspect;
import groovy.lang.GroovyShell;
import groovy.lang.MetaProperty;
import groovy.lang.PropertyValue;
-import org.codehaus.groovy.runtime.FormatHelper;
import org.jmock.Mock;
import org.jmock.cglib.MockObjectTestCase;
@@ -29,10 +28,19 @@ import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.io.Serializable;
import java.lang.reflect.Field;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import static groovy.test.GroovyAssert.isAtLeastJdk;
+
public class InspectorTest extends MockObjectTestCase implements Serializable {
public String someField = "only for testing";
public static final String SOME_CONST = "only for testing";
@@ -139,6 +147,8 @@ public class InspectorTest extends MockObjectTestCase implements Serializable {
// TODO: if our code can never access inspect in this way, it would be better
// to move this to a boundary class and then we wouldn't need this test
public void testInspectUninspectableProperty() {
+ if (isAtLeastJdk("16.0")) return;
+
Object dummyInstance = new Object();
Inspector inspector = getTestableInspector(dummyInstance);
Class[] paramTypes = {Object.class, MetaProperty.class};
diff --git a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
index f108715511..2095952132 100644
--- a/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/ImmutableTransformTest.groovy
@@ -139,8 +139,7 @@ class ImmutableTransformTest extends GroovyShellTestCase {
@Test
void testCloneableFieldNotCloneableObject() {
- def cls = shouldFail(CloneNotSupportedException) {
- def objects = evaluate("""
+ shouldFail(isAtLeastJdk('16.0') ? IllegalAccessException : CloneNotSupportedException, '''
import groovy.transform.Immutable
class Dolly {
@@ -154,10 +153,7 @@ class ImmutableTransformTest extends GroovyShellTestCase {
def dolly = new Dolly(name: "The Sheep")
[dolly, new Lab(name: "Area 51", clone: dolly)]
- """)
- }
-
- assert cls == 'Dolly'
+ ''')
}
@Test
diff --git a/subprojects/groovy-templates/src/test/groovy/groovy/text/StreamingTemplateEngineTest.groovy b/subprojects/groovy-templates/src/test/groovy/groovy/text/StreamingTemplateEngineTest.groovy
index 78b0ed0b47..94e1273ac4 100644
--- a/subprojects/groovy-templates/src/test/groovy/groovy/text/StreamingTemplateEngineTest.groovy
+++ b/subprojects/groovy-templates/src/test/groovy/groovy/text/StreamingTemplateEngineTest.groovy
@@ -23,6 +23,8 @@ import org.junit.Test
import java.util.concurrent.ConcurrentHashMap
+import static groovy.test.GroovyAssert.assertScript
+
class StreamingTemplateEngineTest {
TemplateEngine engine
Map binding
@@ -463,48 +465,52 @@ class StreamingTemplateEngineTest {
@Test
void reuseClassLoader1() {
- final reuseOption = 'groovy.StreamingTemplateEngine.reuseClassLoader'
- System.setProperty(reuseOption, 'true')
+ assertScript '''
+ final reuseOption = 'groovy.StreamingTemplateEngine.reuseClassLoader'
+ System.setProperty(reuseOption, 'true')
- try {
- // reload class to initialize static field from the beginning
- def steClass = reloadClass('groovy.text.StreamingTemplateEngine')
-
- GroovyClassLoader gcl = new GroovyClassLoader()
- def engine = steClass.newInstance(gcl)
- assert 'Hello, Daniel' == engine.createTemplate('Hello, ${name}').make([name: 'Daniel']).toString()
- assert gcl.loadedClasses.length > 0
- def cloned = gcl.loadedClasses.clone()
- assert 'Hello, Paul' == engine.createTemplate('Hello, ${name}').make([name: 'Paul']).toString()
- assert cloned == gcl.loadedClasses
- } finally {
- System.clearProperty(reuseOption)
- }
+ try {
+ // reload class to initialize static field from the beginning
+ def steClass = groovy.text.StreamingTemplateEngineTest.reloadClass('groovy.text.StreamingTemplateEngine')
+
+ GroovyClassLoader gcl = new GroovyClassLoader()
+ def engine = steClass.newInstance(gcl)
+ assert 'Hello, Daniel' == engine.createTemplate('Hello, ${name}').make([name: 'Daniel']).toString()
+ assert gcl.loadedClasses.length > 0
+ def cloned = gcl.loadedClasses.clone()
+ assert 'Hello, Paul' == engine.createTemplate('Hello, ${name}').make([name: 'Paul']).toString()
+ assert cloned == gcl.loadedClasses
+ } finally {
+ System.clearProperty(reuseOption)
+ }
+ '''
}
@Test
void reuseClassLoader2() {
- final reuseOption = 'groovy.StreamingTemplateEngine.reuseClassLoader'
- System.setProperty(reuseOption, 'true')
-
- try {
- // reload class to initialize static field from the beginning
- def steClass = reloadClass('groovy.text.StreamingTemplateEngine')
-
- GroovyClassLoader gcl = new GroovyClassLoader()
- def engine = steClass.newInstance(gcl)
- assert 'Hello, Daniel' == engine.createTemplate('Hello, ${name}').make([name: 'Daniel']).toString()
- assert gcl.loadedClasses.length > 0
- def cloned = gcl.loadedClasses.clone()
- engine = steClass.newInstance(gcl)
- assert 'Hello, Paul' == engine.createTemplate('Hello, ${name}').make([name: 'Paul']).toString()
- assert cloned == gcl.loadedClasses
- } finally {
- System.clearProperty(reuseOption)
- }
- }
-
- private static Class reloadClass(String className) {
+ assertScript '''
+ final reuseOption = 'groovy.StreamingTemplateEngine.reuseClassLoader'
+ System.setProperty(reuseOption, 'true')
+
+ try {
+ // reload class to initialize static field from the beginning
+ def steClass = groovy.text.StreamingTemplateEngineTest.reloadClass('groovy.text.StreamingTemplateEngine')
+
+ GroovyClassLoader gcl = new GroovyClassLoader()
+ def engine = steClass.newInstance(gcl)
+ assert 'Hello, Daniel' == engine.createTemplate('Hello, ${name}').make([name: 'Daniel']).toString()
+ assert gcl.loadedClasses.length > 0
+ def cloned = gcl.loadedClasses.clone()
+ engine = steClass.newInstance(gcl)
+ assert 'Hello, Paul' == engine.createTemplate('Hello, ${name}').make([name: 'Paul']).toString()
+ assert cloned == gcl.loadedClasses
+ } finally {
+ System.clearProperty(reuseOption)
+ }
+ '''
+ }
+
+ static Class reloadClass(String className) {
def clazz =
new GroovyClassLoader() {
private final Map<String, Class> loadedClasses = new ConcurrentHashMap<String, Class>()
[groovy] 01/02: GROOVY-10730: Remove the jdk16+ add-opens jvm args
Posted by su...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit f3913253998953532c2e6cc99886ca12446496ad
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sun Aug 28 22:24:10 2022 +0800
GROOVY-10730: Remove the jdk16+ add-opens jvm args
---
buildSrc/src/main/groovy/org.apache.groovy-tested.gradle | 6 ------
1 file changed, 6 deletions(-)
diff --git a/buildSrc/src/main/groovy/org.apache.groovy-tested.gradle b/buildSrc/src/main/groovy/org.apache.groovy-tested.gradle
index dfa7479e60..19ac5e7492 100644
--- a/buildSrc/src/main/groovy/org.apache.groovy-tested.gradle
+++ b/buildSrc/src/main/groovy/org.apache.groovy-tested.gradle
@@ -32,18 +32,12 @@ sourceSets {
}
}
-// TODO remove the jdk16+ add-opens jvm args once offending code/tests are fixed
-// https://docs.gradle.org/7.5/userguide/upgrading_version_7.html#removes_implicit_add_opens_for_test_workers
tasks.withType(Test).configureEach {
def fs = objects.newInstance(TestServices).fileSystemOperations
def grapeDirectory = new File(temporaryDir, "grape")
def jdk8 = ['-XX:+UseConcMarkSweepGC']
def jdk9 = ['-Djava.locale.providers=COMPAT,SPI']
// def jdk9 = ['-Djava.locale.providers=COMPAT,SPI', '--illegal-access=debug']
- if (JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_16)) {
- jdk9 += ["--add-opens=java.base/java.lang=ALL-UNNAMED",
- "--add-opens=java.base/java.util=ALL-UNNAMED"]
- }
def common = ['-ea', "-Xms${groovyJUnit_ms}", "-Xmx${groovyJUnit_mx}", "-Duser.language=en"]
if (JavaVersion.current().isJava9Compatible()) {
jvmArgs(*common, *jdk9)