[GitHub] groovy pull request #811: GROOVY-8843: Fix illegal reflective access within ...

2018-10-15 Thread paulk-asert 
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 
LOOKUP_Constructor;
+static {
+Constructor 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 ...

2018-10-15 Thread blackdrag 
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 
LOOKUP_Constructor;
+static {
+Constructor 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


---

[GitHub] groovy pull request #811: GROOVY-8843: Fix illegal reflective access within ...

2018-10-15 Thread blackdrag 
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 
LOOKUP_Constructor;
+static {
+Constructor 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 
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 ...

2018-10-15 Thread paulk-asert 
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 
Date:   2018-10-15T08:12:45Z

GROOVY-8843: Fix illegal reflective access within o.c.g.vmplugin.v7.Java7




---