Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 26AEF200CCB for ; Wed, 5 Jul 2017 23:02:25 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 246651647BB; Wed, 5 Jul 2017 21:02:25 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id A04DB1647B6 for ; Wed, 5 Jul 2017 23:02:23 +0200 (CEST) Received: (qmail 89952 invoked by uid 500); 5 Jul 2017 21:02:22 -0000 Mailing-List: contact dev-help@groovy.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@groovy.apache.org Delivered-To: mailing list dev@groovy.apache.org Received: (qmail 89938 invoked by uid 99); 5 Jul 2017 21:02:22 -0000 Received: from mail-relay.apache.org (HELO mail-relay.apache.org) (140.211.11.15) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 05 Jul 2017 21:02:22 +0000 Received: from VEGA (p4FC8B290.dip0.t-ipconnect.de [79.200.178.144]) by mail-relay.apache.org (ASF Mail Server at mail-relay.apache.org) with ESMTPSA id E63631A00C5; Wed, 5 Jul 2017 21:02:19 +0000 (UTC) From: "Uwe Schindler" To: , References: <1499272911.21698.16.camel@winder.org.uk> <023c01d2f5b1$2acd9ba0$8068d2e0$@apache.org> <025d01d2f5b4$00eff550$02cfdff0$@apache.org> <027301d2f5b9$e449ea90$acddbfb0$@apache.org> In-Reply-To: Subject: RE: trySetAccessible for Java 9 Date: Wed, 5 Jul 2017 23:02:13 +0200 Message-ID: <029d01d2f5d1$fe4bb4b0$fae31e10$@apache.org> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_NextPart_000_029E_01D2F5E2.C1D80720" X-Mailer: Microsoft Outlook 16.0 Content-Language: de Thread-Index: AQDLwAQokRlBJPELMGQUZlgB5WvpCAKEBN4FARkOuGoBlz6ZQAHgRoQEAVA5yDICOobcmAKaqpnUo+pNWaA= archived-at: Wed, 05 Jul 2017 21:02:25 -0000 This is a multipart message in MIME format. ------=_NextPart_000_029E_01D2F5E2.C1D80720 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi, =20 unfortunately the MethodHandle approach did not work without a small = modification: =20 java.lang.IllegalAccessException: Attempt to lookup caller-sensitive = method using restricted lookup object =20 As the trySetAccessible method is caller sensitive, you cannot get a = MethodHandle from it using a public lookup. By changing the code to use = a normal (private) lookup, it works (as expected). There are no security = implications by that as we only access public methods. Only the lookup = object needs the =E2=80=9Cowner=E2=80=9D class to inject right caller = sensitiveness. The private lookup (private to CachedClass) is allowed to = get the method handle (and it should also be kept private inside = CachedClass, otherwise you violate security!!!) =20 I updated by branch: https://github.com/apache/groovy/compare/master...uschindler:java9/trySet= Accessible =20 This already helps when starting gradle, because as soon as the = compileGroovy tasks are starting, you are using the bootstrapped JAR = file. The CachedClass problem is fixed, no more illegal reflective = acceses, but I got a new one from a bytecode generated class (!?): =20 WARNING: An illegal reflective access operation has occurred WARNING: Illegal reflective access by = org.codehaus.groovy.reflection.InjectedInvoker/1364880320 = (file:/C:/Users/Uwe%20Schindler/Projects/groovy/target/classes/java/main/= ) to method java.lang.Object.finalize() WARNING: Please consider reporting this to the maintainers of = org.codehaus.groovy.reflection.InjectedInvoker/1364880320 WARNING: Use --illegal-access=3Dwarn to enable warnings of further = illegal reflective access operations WARNING: All illegal access operations will be denied in a future = release =20 As you see the whole thing got better, but now we have the same problem = in org.codehaus.groovy.reflection.InjectedInvoker, but this one is = synthetic. It looks like we must change the bytecode of that, too. But = here we are lucky: We can use the detected Java 9 version and just = create different bytecode at runtime depending on Java version? I have = not looked at this, I just verified that my branch works with Java 9 = build 175. =20 I have not looked at VMPlugin stuff, that should be done by somebody = else. But in that case: how about using a multi-release jar in that = case? I know there is no support to create those in Maven/Gradle, but I = am sure one can script it! =20 IMHO: I would on Java 9 never ever use the array setAccessible method. = You can be sure that it throws an exception in most cases, so why even = try and take the cost of =20 And I am not sure if setAccessible with array will not also print = warnings, once Alan Bateman & Co. fixed this bug (it is a bug)! =20 Uwe =20 ----- Uwe Schindler uschindler@apache.org=20 ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/ =20 From: C=C3=A9dric Champeau [mailto:cchampeau@apache.org]=20 Sent: Wednesday, July 5, 2017 8:12 PM To: Uwe Schindler Cc: dev@groovy.apache.org Subject: Re: trySetAccessible for Java 9 =20 Thanks Uwe! To test with JDK 9 you'll need Gradle 4.1-milestone-1. I = know Jochen has some special setup to make it work on previous releases = of Gradle but I didn't try that. =20 2017-07-05 20:09 GMT+02:00 Uwe Schindler >: Here is my quick patch: = = https://github.com/apache/groovy/compare/master...uschindler:java9/trySet= Accessible =20 Sorry for my ignorance, but how to run tests with Java 9? Gradle fails = for me to launch daemon! =20 Uwe =20 ----- Uwe Schindler uschindler@apache.org =20 ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/ =20 From: Uwe Schindler [mailto:uschindler@apache.org = ]=20 Sent: Wednesday, July 5, 2017 7:27 PM To: dev@groovy.apache.org ; = cchampeau@apache.org =20 Subject: RE: trySetAccessible for Java 9 =20 Working on it. =20 I just looked at the code and found out that it already has a = =E2=80=9Efallback=E2=80=9C mechanism: It first tries = setAccessible(array, true) and then falls back to do it one by one. I = think with Java 9, wenn cannot do this. So I=E2=80=99d change that to: =20 * Get methodhandle in static initializer, if not there set to NULL * In the makeAccessible method check for nullness of methodhandle: if = null proceed as before, if not do a for-loop and call trySetAccesible() = on all, ignoring return value. =20 ----- Uwe Schindler uschindler@apache.org =20 ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/ =20 From: C=C3=A9dric Champeau [mailto:cchampeau@apache.org]=20 Sent: Wednesday, July 5, 2017 7:10 PM To: dev@groovy.apache.org =20 Cc: Russel Winder > Subject: Re: trySetAccessible for Java 9 =20 Thanks Uwe, patches/PRs are very welcome :) I did miss your suggestion, = sorry I wasn't able to follow everything on this list lately. =20 The risk I saw was that the MethodHandle class wasn't always available, = but for 2.4+, it's not a problem! =20 =20 2017-07-05 19:07 GMT+02:00 Uwe Schindler >: Hi, =20 I made this suggestion about a month ago! In Lucene/Elasticsearch we do = everything with MethodHandles that requires new Java 9 APIs (currently = Elasticsearch=E2=80=99s Painless Script engine is the first one that = uses indy string concats!). In general I would not use an if/then/else = construct at all. Just try to get a MethodHandle to trySetAccessible(), = if this fails get a MethodHandle to a local/private method with same = signature. =20 Finally you may need to adapt the MethodHandle to the right types and = then call it _always_ with correct casting to make javac use correct = types. Be sure to make the MethodHandle a static final constant = somewhere! This removed the need for a if/then/else on every call. =20 I may provide a patch, if you like. I=E2=80=99d just need some = directions where to look at. Should be a 10 liner. =20 Uwe =20 ----- Uwe Schindler uschindler@apache.org =20 ASF Member, Apache Lucene PMC / Committer Bremen, Germany http://lucene.apache.org/ =20 From: C=C3=A9dric Champeau [mailto:cchampeau@apache.org = ]=20 Sent: Wednesday, July 5, 2017 6:55 PM To: Russel Winder > Cc: dev@groovy.apache.org =20 Subject: Re: trySetAccessible for Java 9 =20 Actually I'm realizing that the `MethodHandle` API came with Java 7. So = we _can_ compile against it. So I guess an option is to have the method = handle redirect to `trySetAccessible` if the detected runtime is Java 9, = and a backport method if < 9. =20 =20 =20 2017-07-05 18:41 GMT+02:00 Russel Winder >: On Wed, 2017-07-05 at 18:28 +0200, C=C3=A9dric Champeau wrote: > [=E2=80=A6] > Any suggestion? How about leave Groovy 2.x as a "can only build on JDK8", and put all = effort for a JDK9 build on Groovy 3.x which, as I understand it requires JDK8 = as a runtime. This would seem to minimise hassle and maximise forward-looking benefit. Unless I am missing something. -- Russel. =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D Dr Russel Winder t:+44 20 7585 2200 = voip:sip: russel.winder@ekiga.net =20 41 Buckmaster Road m:+44 7770 465 077 = xmpp:russel@winder.org.uk =20 London SW11 1EN, UK w: www.russel.org.uk = skype:russel_winder =20 =20 =20 ------=_NextPart_000_029E_01D2F5E2.C1D80720 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

Hi,

 

unfortunately the MethodHandle = approach did not work without a small = modification:

 

java.lang.IllegalAccessException: = Attempt to lookup caller-sensitive method using restricted lookup = object

 

As the trySetAccessible method is = caller sensitive, you cannot get a MethodHandle from it using a public = lookup. By changing the code to use a normal (private) lookup, it works = (as expected). There are no security implications by that as we only = access public methods. Only the lookup object needs the = =E2=80=9Cowner=E2=80=9D class to inject right caller sensitiveness. The = private lookup (private to CachedClass) is allowed to get the method = handle (and it should also be kept private inside CachedClass, otherwise = you violate security!!!)

 

I updated by = branch:

https://github.com/apache/groovy/compare/master...usc= hindler:java9/trySetAccessible

 

This already helps when starting = gradle, because as soon as the compileGroovy tasks are starting, you are = using the bootstrapped JAR file. The CachedClass problem is fixed, no = more illegal reflective acceses, but I got a new one from a bytecode = generated class (!?):

 

WARNING: An illegal reflective = access operation has occurred

WARNING: Illegal reflective access = by org.codehaus.groovy.reflection.InjectedInvoker/1364880320 = (file:/C:/Users/Uwe%20Schindler/Projects/groovy/target/classes/java/main/= ) to method java.lang.Object.finalize()

WARNING: Please consider reporting = this to the maintainers of = org.codehaus.groovy.reflection.InjectedInvoker/1364880320

WARNING: Use = --illegal-access=3Dwarn to enable warnings of further illegal reflective = access operations

WARNING: All illegal = access operations will be denied in a future = release

 

As you see the whole thing got = better, but now we have the same problem in = org.codehaus.groovy.reflection.InjectedInvoker, but this one is = synthetic. It looks like we must change the bytecode of that, too. But = here we are lucky: We can use the detected Java 9 version and just = create different bytecode at runtime depending on Java version? I have = not looked at this, I just verified that my branch works with Java 9 = build 175.

 

I have not looked at VMPlugin = stuff, that should be done by somebody else. But in that case: how about = using a multi-release jar in that case? I know there is no support to = create those in Maven/Gradle, but I am sure one can script = it!

 

IMHO: I would on Java 9 never ever = use the array setAccessible method. You can be sure that it throws an = exception in most cases, so why even try and take the cost = of

 

And I am not sure if setAccessible = with array will not also print warnings, once Alan Bateman & Co. = fixed this bug (it is a bug)!

 

Uwe

 

-----

Uwe = Schindler

uschindler@apache.org

ASF Member, Apache Lucene PMC / = Committer

Bremen, = Germany

http://lucene.apache.org/

 

From: = C=C3=A9dric Champeau [mailto:cchampeau@apache.org]
Sent: = Wednesday, July 5, 2017 8:12 PM
To: Uwe Schindler = <uschindler@apache.org>
Cc: = dev@groovy.apache.org
Subject: Re: trySetAccessible for Java = 9

 

Thanks = Uwe! To test with JDK 9 you'll need Gradle 4.1-milestone-1. I know = Jochen has some special setup to make it work on previous releases of = Gradle but I didn't try that.

 

2017-07-05 20:09 GMT+02:00 Uwe Schindler <uschindler@apache.org>:

Here is my quick patch:

https://github.com/apache/groovy/compare/master...uschindler= :java9/trySetAccessible

 

Sorry for my ignorance, but how to run tests with Java 9? = Gradle fails for me to launch daemon!

 

Uwe

 

-----

Uwe Schindler

uschindler@apache.org

ASF Member, Apache Lucene PMC / = Committer

Bremen, = Germany

http://lucene.apache.org/

 <= /o:p>

From:= Uwe Schindler [mailto:uschindler@apache.org]
Sent: Wednesday, = July 5, 2017 7:27 PM
To: dev@groovy.apache.org; cchampeau@apache.org
Subject: RE: = trySetAccessible for Java 9

 <= /o:p>

Working on = it.

 <= /o:p>

I just looked at the code and found out that it already has = a =E2=80=9Efallback=E2=80=9C mechanism: It first tries = setAccessible(array, true) and then falls back to do it one by one. I = think with Java 9, wenn cannot do this. So I=E2=80=99d change that = to:

 

  • Get methodhandle in = static initializer, if not there set to NULL
  • In the makeAccessible = method check for nullness of methodhandle: if null proceed as before, if = not do a for-loop and call trySetAccesible() on all, ignoring return = value.

 

-----

Uwe Schindler

uschindler@apache.org

ASF Member, Apache Lucene PMC / = Committer

Bremen, = Germany

http://lucene.apache.org/

 <= /o:p>

From:= C=C3=A9dric Champeau [mailto:cchampeau@apache.org]
Sent: = Wednesday, July 5, 2017 7:10 PM
To: dev@groovy.apache.org
Cc: Russel Winder = <russel@winder.org.uk>
Subject: Re: = trySetAccessible for Java 9

 <= /o:p>

Thanks Uwe, = patches/PRs are very welcome :) I did miss your suggestion, sorry I = wasn't able to follow everything on this list = lately.

 <= /o:p>

The risk I = saw was that the MethodHandle class wasn't always available, but for = 2.4+, it's not a problem!

 <= /o:p>

 <= /o:p>

2017-07-05 = 19:07 GMT+02:00 Uwe Schindler <uschindler@apache.org>:

Hi,

 <= /o:p>

I made this suggestion about a month ago! In = Lucene/Elasticsearch we do everything with MethodHandles that requires = new Java 9 APIs (currently Elasticsearch=E2=80=99s Painless Script = engine is the first one that uses indy string concats!). In general I = would not use an if/then/else construct at all. Just try to get a = MethodHandle to trySetAccessible(), if this fails get a MethodHandle to = a local/private method with same signature.

 

Finally you may need to adapt the MethodHandle to the right = types and then call it _always_ with correct casting to make = javac use correct types. Be sure to make the MethodHandle a static final = constant somewhere! This removed the need for a if/then/else on every = call.

 

I may provide a patch, if you like. I=E2=80=99d just need = some directions where to look at. Should be a 10 = liner.

 

Uwe

 

-----

Uwe Schindler

uschindler@apache.org

ASF Member, Apache Lucene PMC / = Committer

Bremen, = Germany

http://lucene.apache.org/

 <= /o:p>

From:= C=C3=A9dric Champeau [mailto:cchampeau@apache.org]
Sent: Wednesday, = July 5, 2017 6:55 PM
To: Russel Winder <russel@winder.org.uk>
Cc: dev@groovy.apache.org
Subject: Re: = trySetAccessible for Java 9

 <= /o:p>

Actually = I'm realizing that the `MethodHandle` API came with Java 7. So we _can_ = compile against it. So I guess an option is to have the method handle = redirect to `trySetAccessible` if the detected runtime is Java 9, and a = backport method if < 9.

 <= /o:p>

 <= /o:p>

 <= /o:p>

2017-07-05 = 18:41 GMT+02:00 Russel Winder <russel@winder.org.uk>:

On Wed, = 2017-07-05 at 18:28 +0200, C=C3=A9dric Champeau = wrote:
>
[=E2=80=A6]
> Any suggestion?

How about = leave Groovy 2.x as a "can only build on JDK8", and put all = effort
for a JDK9 build on Groovy 3.x which, as I understand it = requires JDK8 as a
runtime. This would seem to minimise hassle and = maximise forward-looking
benefit. Unless I am missing = something.

--
Russel.
<= span = class=3Dm2818162978178696580m338477240417035116hoenzb>=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
Dr Russel = Winder     t:+44 20 7585 2200  =  voip:sip:
russel.winder@ekiga.net
41 Buckmaster = Road   m:+44 7770 465 077   xmpp:russel@winder.org.uk
London SW11 1EN, = UK  w: www.russel.org.uk = skype:russel_winder

 <= /o:p>

 <= /o:p>

 

------=_NextPart_000_029E_01D2F5E2.C1D80720--