setAccessible() alternative with Jigsaw - feedback on Lookup

[Matej Novotny]

So I did some hacking and tried to make Weld use MethodHandles.Lookup and here is a bit of a feedback for you.
But first of all, thanks for your quick advice on how to approach this.

Alright, I should say that I _somehow_ managed to make it work in simple cases (SE container for most part).
I haven't tried my dirty solution in anything EE, which is where problems will definitely pop-up (as they always do when you say "EAR").
ATM it is pretty impossible to try Weld 3 (will support jdk 9, still not final, CDI 2.0 impl) with any AS as none support it ATM (will try to port it to Wildfly 11).


To sum up how well Lookup worked for me, let me shed some light on how Weld works (simplified ofc) and therefore what I needed to achieve:

In short, we scan CP for classes which are to become beans. Such classes are the processed and eventually get a proxy created for them (given they have the right scope).
This proxy is a piece of generated bytecode which we need to register via CL, or now via Lookup, using defineClass method.
We respect the packages - proxy lands in the same package as the original class (with notable exceptions, see below).
All the above has to be done during bootstrap - you cannot dynamically add/remove beans once you got application running.


So, how did Lookup work for us?

1) privateLookupIn + drop private mode
This was the way to go, as the bean classes can be anywhere (in classes not openly accessible to us, especially if we consider modules).
BTW I am not sure about the purpose of the private mode as you always need to drop it to be able to use lookup.

2) Lookup approach carries along the need to pass the reference to the base lookup class at all times.
This is kind of weird  because in some (not-so-rare) cases, we need to create "artificial packages" in which we place proxy classes. For instance when we create a proxy for Interger, Map, Principal,...
Ofc we cannot place in into java.* packages, so we create our own. For this to work with Lookup, we need to have that package created ahead of time and have a reference to some "dummy" useless class inside to be able to do the lookup.
Or is there a way to define a whole new (open by default) package where we could place it using Lookup? Having the "dummy" package/class just to use Lookup is lame and very error prone (I can think of several ways to break the proxy creation even now).

3) All in all the changes carried along a lot of complexity compared to plain old ClassLoader.defineClass
We need to create A LOT of Lookups (might present performance overhead, haven't tested yet).
Extra care needs to be given to the packages for Lookup not to blow up while we in fact don't really care where the proxy lands as long as it is usable.


Another nasty thing is that the code of course needs to work with both, JDK 9 and 8.
While it isn't impossible, it will add a not-so-nice reflection magic layer to the mix.

Regards
Matej


----- Original Message -----
> From: "Alan Bateman" <Alan.Bateman at oracle.com>
> To: "Matej Novotny" <manovotn at redhat.com>, jigsaw-dev at openjdk.java.net
> Cc: "Martin Kouba" <mkouba at redhat.com>
> Sent: Friday, March 31, 2017 4:28:34 PM
> Subject: Re: setAccessible() alternative with Jigsaw
> 
> On 31/03/2017 14:46, Matej Novotny wrote:
> 
> > Hello,
> >
> > I work on Weld, context dependency injection framework.
> > Long story short - we need to generate proxies for classes - bytecode which
> > we then "register" with the class loader using
> > java.lang.ClassLoader#defineClass.
> >
> > Obviously, for this you need reflections - to load java.lang.ClassLoader,
> > then to load the method itself, and most importantly, to make the method
> > accessible cause it's `protected`.
> > In JDK 9, this blows up as soon as you try to make the method accessible
> > (invocation of setAccessible).
> >
> > Fair enough, but what is the "legitimate" alternative?
> > I know I can --add-opens  / --add-opens / --permit-illegal-access
> > But all those just bypass the checks and don't really solve it. I am
> > looking for an intended way to do such stuff.
> > I am pretty sure there are many frameworks which need to do this in one way
> > or the other.
> >
> > So far I have found workarounds which involve using `sun.misc.Unsafe`
> > because there (for some reason) you are allowed to invoke setAccessible().
> > Is this the official intended backdoor? Because it sure does not look any
> > safer/cleaner solution than the original one...
> >
> As Claes pointed out, look at Lookup.defineClass. It shouldn't be too
> hard to imagine a future update of the CDI spec where there a select
> variant that includes a Lookup to the target class with the @Inject
> annotation. When using Weld outside of a container then I think there is
> main code that obtains the WeldContainer and selects the types, that
> might be a suitable place to extend to provide the Lookup to the library.
> 
> MethodHandles.privateLookupIn might also be useful to know about, esp.
> starting out when you don't want to make API changes and where the
> user's code is on the class path or in open modules.
> 
> -Alan
>