On 09.07.2017 13:13, Uwe Schindler wrote:
Hi,
thanks for the insight. I was just not sure where this synthetic class
name came from.
I think we should ask on the Jigsaw mailing list if the warning was
intended to be also printed by the trySetAccessible call. IMHO,
trySetAccessible is a new API, so it should **not** grant access nor
print a warning. The Jigsaw kill switch should have no effect on
trySetAccessible! In contrast, setAccessible should behave as it does
with the Jigsaw kill switch enabled.
maybe they did it wrong, since they changed the kill switch suddenly.
Nevertheless, I’d change the code in the following way: Instead of
trySetAccessible method handle, use a methodhandle to get the Module
instance of the Groovy runtime (fetch it once in the static initializer
-- should be the unnamed module, but we should be flexible here) and
when trying to make everything accessible in a class, use the same
methodhandle to get the module, too. If the module if different, don’t
try to make anything accessible at all and just return with empty array.
In the future we may try improve the whole thing my looking into the
module properties (“open” packages), but as a quick fix this is fine.
well... let us assume you have a class
class IAmInJavaBase {
final protected void callMe()
}
in java.base and this
class GroovyWithModules extends IAmInJavaBase {
void foo(){callMe()}
}
The Groovy runtime will no longer be allowed to call "callMe", because
while the unnamed module reads java.base, it does not make java.base
automatically an open module. And even if IAmInJavaBase is in an
exported package, that does not include "callMe"
The two ways to make this call work is by using the Lookup object
provided by GroovyWithModules and a direct method call. Even if you get
that right, you still have to handle inner classes and Closures.
(Technically you can do this with reflection as well, but then the
invokeMethod has to come from GroovyWithModules directly)
Now if you stop trying to make things accessible and just return an
empty array, then "callMe" will not exist anymore. That means, that even
in indy, where you have this lookup object, you will get a
MissingMethodException.
If you go with the approach of reacting to AccessibleObject.canAccess
returning true... I would be very surprised to see canAccess returning
true for that method.
the major pain with the central invoker simply is, that if X can access
foo and Y cannot access foo, you have trouble to make a system in which
both are allowed to make their call... especially if you do not know by
100% who the original caller actually is.
[...]
Finally I have the question: Is it really needed to keep trying to make
everything accessible by default in Groovy 3 ???
The big problem is giving up the central invoker pattern completely. It
also means dragging Lookup instances through the whole MOP to finally
reach the place of invocation... or to change the whole MOP to return
something you can invoke.
My original plan for MOP2 was actually trying to change the MOP to
always return something you can then invoke. But...
class GroovyWithModules extends IAmInJavaBase {
void foo(){
def cl = new Helper().build()
cl.setDelegate(this)
cl.call()
}
}
class Helper {
def build() {
return { callMe () }
}
}
new GroovyWithModules().foo()
This code works in Groovy today, but with modules... So even though I do
let build return something that can call callMe() I still have to give
it the rights to do so. So either I transport the Lookup object through
the whole MOP and let it for example return a MethodHandle, or I create
another intermediate layer here that I can control from the caller and
that then will produce the MethodHandle with the appropriate Lookup
object. So I do have to at least combine the two approaches now.
And frankly, in the code above I use Helper just as an demonstration of
what the runtime would have to do. If I actually want to get the above
working I am pretty screwed. In { callMe () } the best lookup object I
can automatically use is the one from Helper, and that one is not having
access.
I could introduce something like:
class Helper {
def build(x) {
return { inTheNameOf(x) {callMe ()} }
}
}
A where x is actually a GroovyWithModules instance with very special
logic for inTheNameOf, which would basically try to somehow get the
Lookup object with enough rights from x to be able to do the call. This
looks majorly awkward and you actually rip open all the holes, the
module system is supposed to keep under the covers. In the end the whole
concept of setting a delegate is now broken for the module system.
Because in the end, each of our Closures are specialized central
invoker. And something like x."$foo"() takes the specialization away
completely. Since central invoker do not work properly anymore in JDK9,
Closures will also produce trouble in some cases.
Maybe you should break
backwards compatibility and no longer do this by default.
You know, if breaking the API would be our only problem here, it
wouldn't be a big deal. Having to overthrow basic concepts of the
language is the kind of backwards compatibility break we are talking
about here.
This was IMHO
a mis-design of the Groovy language from the beginning.
It was the way to do it back in 2004 with a security manager. And the
security manager mechanism almost forced this structure upon you.
This is also a
security issue, as every groovy module can corrupt the whole JVM. This
is also one reason why Elasticsearch moved to their own scripting
language, as the cowboy “setAccessible” everywhere is a serious security
issue!
Yes, they do not know how to use a security manager or simply do not
want to bother with one.
bye Jochen
