You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by paulk-asert <gi...@git.apache.org> on 2018/10/15 08:15:56 UTC
[GitHub] groovy pull request #811: GROOVY-8843: Fix illegal reflective access within ...
GitHub user paulk-asert opened a pull request:
https://github.com/apache/groovy/pull/811
GROOVY-8843: Fix illegal reflective access within o.c.g.vmplugin.v7.J…
…ava7
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/paulk-asert/groovy groovy8843
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/groovy/pull/811.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #811
----
commit 521532204f1f2f8997cf7811f509d86afeff6f7d
Author: Paul King <pa...@...>
Date: 2018-10-15T08:12:45Z
GROOVY-8843: Fix illegal reflective access within o.c.g.vmplugin.v7.Java7
----
---
[GitHub] groovy pull request #811: GROOVY-8843: Fix illegal reflective access within ...
Posted by paulk-asert <gi...@git.apache.org>.
Github user paulk-asert commented on a diff in the pull request:
https://github.com/apache/groovy/pull/811#discussion_r225328503
--- Diff: src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java ---
@@ -18,14 +18,78 @@
*/
package org.codehaus.groovy.vmplugin.v9;
+import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.vmplugin.v8.Java8;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
/**
- * Java 9 based functions will be added here if needed.
+ * Additional Java 9 based functions will be added here as needed.
*/
public class Java9 extends Java8 {
+
+ private static class LookupHolder {
+ private static final Method PRIVATE_LOOKUP;
+ private static final Constructor<MethodHandles.Lookup> LOOKUP_Constructor;
+ static {
+ Constructor<MethodHandles.Lookup> lookup = null;
+ Method privateLookup = null;
+ try { // java 9
+ privateLookup = MethodHandles.class.getMethod("privateLookupIn", Class.class, MethodHandles.Lookup.class);
--- End diff --
We don't eventually, I was just letting the build be built from Java 8 (with current warnings) until we get more things in place.
---
[GitHub] groovy pull request #811: GROOVY-8843: Fix illegal reflective access within ...
Posted by blackdrag <gi...@git.apache.org>.
Github user blackdrag commented on a diff in the pull request:
https://github.com/apache/groovy/pull/811#discussion_r225298000
--- Diff: src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java ---
@@ -18,14 +18,78 @@
*/
package org.codehaus.groovy.vmplugin.v9;
+import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.vmplugin.v8.Java8;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
/**
- * Java 9 based functions will be added here if needed.
+ * Additional Java 9 based functions will be added here as needed.
*/
public class Java9 extends Java8 {
+
+ private static class LookupHolder {
+ private static final Method PRIVATE_LOOKUP;
+ private static final Constructor<MethodHandles.Lookup> LOOKUP_Constructor;
+ static {
+ Constructor<MethodHandles.Lookup> lookup = null;
+ Method privateLookup = null;
+ try { // java 9
+ privateLookup = MethodHandles.class.getMethod("privateLookupIn", Class.class, MethodHandles.Lookup.class);
+ } catch (final NoSuchMethodException e) { // java 8
+ try {
+ lookup = MethodHandles.Lookup.class.getDeclaredConstructor(Class.class, Integer.TYPE);
+ if (!lookup.isAccessible()) {
--- End diff --
we can actually not rely on this being accessible... I am missing the IllegalAccessException and InaccessibleObjectException handling here...
---
[GitHub] groovy pull request #811: GROOVY-8843: Fix illegal reflective access within ...
Posted by blackdrag <gi...@git.apache.org>.
Github user blackdrag commented on a diff in the pull request:
https://github.com/apache/groovy/pull/811#discussion_r225299922
--- Diff: src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java ---
@@ -18,14 +18,78 @@
*/
package org.codehaus.groovy.vmplugin.v9;
+import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.vmplugin.v8.Java8;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
/**
- * Java 9 based functions will be added here if needed.
+ * Additional Java 9 based functions will be added here as needed.
*/
public class Java9 extends Java8 {
+
+ private static class LookupHolder {
+ private static final Method PRIVATE_LOOKUP;
+ private static final Constructor<MethodHandles.Lookup> LOOKUP_Constructor;
+ static {
+ Constructor<MethodHandles.Lookup> lookup = null;
+ Method privateLookup = null;
+ try { // java 9
+ privateLookup = MethodHandles.class.getMethod("privateLookupIn", Class.class, MethodHandles.Lookup.class);
+ } catch (final NoSuchMethodException e) { // java 8
+ try {
+ lookup = MethodHandles.Lookup.class.getDeclaredConstructor(Class.class, Integer.TYPE);
+ if (!lookup.isAccessible()) {
+ lookup.setAccessible(true);
+ }
+ } catch (final NoSuchMethodException ex) {
+ throw new IllegalStateException("Incompatible JVM", e);
+ }
+ }
+ PRIVATE_LOOKUP = privateLookup;
+ LOOKUP_Constructor = lookup;
+ }
+ }
+
+ private static Constructor<MethodHandles.Lookup> getLookupConstructor() {
+ return LookupHolder.LOOKUP_Constructor;
+ }
+
+ private static Method getPrivateLookup() {
+ return LookupHolder.PRIVATE_LOOKUP;
+ }
+
+ public static MethodHandles.Lookup of(final Class<?> declaringClass) {
+ try {
+ if (getPrivateLookup() != null) {
+ return MethodHandles.Lookup.class.cast(getPrivateLookup().invoke(null, declaringClass, MethodHandles.lookup()));
+ }
+ return getLookupConstructor().newInstance(declaringClass, MethodHandles.Lookup.PRIVATE).in(declaringClass);
--- End diff --
If PRIVATE_LOOKUP lookup never fails as I think, then LOOKUP_Constructor will be always the same. Not sure if "in" (https://docs.oracle.com/javase/9/docs/api/java/lang/invoke/MethodHandles.Lookup.html#in-java.lang.Class-) then can still do something useful.
---
[GitHub] groovy pull request #811: GROOVY-8843: Fix illegal reflective access within ...
Posted by blackdrag <gi...@git.apache.org>.
Github user blackdrag commented on a diff in the pull request:
https://github.com/apache/groovy/pull/811#discussion_r225293219
--- Diff: src/main/java/org/codehaus/groovy/vmplugin/v9/Java9.java ---
@@ -18,14 +18,78 @@
*/
package org.codehaus.groovy.vmplugin.v9;
+import org.codehaus.groovy.GroovyBugError;
import org.codehaus.groovy.vmplugin.v8.Java8;
+import java.lang.invoke.MethodHandles;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
/**
- * Java 9 based functions will be added here if needed.
+ * Additional Java 9 based functions will be added here as needed.
*/
public class Java9 extends Java8 {
+
+ private static class LookupHolder {
+ private static final Method PRIVATE_LOOKUP;
+ private static final Constructor<MethodHandles.Lookup> LOOKUP_Constructor;
+ static {
+ Constructor<MethodHandles.Lookup> lookup = null;
+ Method privateLookup = null;
+ try { // java 9
+ privateLookup = MethodHandles.class.getMethod("privateLookupIn", Class.class, MethodHandles.Lookup.class);
--- End diff --
why do we do a reflective lookup here for a method that is public static? This is already the Java 9 module, so pre Java 9 code is of no concern... especially the NoSuchMethodException is supposed to never happen here
---