Review Request: JDK-8173978: Lookup.in should allow teleporting from a lookup class in a named module without dropping all access

[Alan Bateman]

On 01/07/2019 20:51, Mandy Chung wrote:
> This is an enhancement to |`Lookup::in`| and 
> |`MethodHandles::privateLookupIn`| API
> for cross module teleporting.  A `Lookup` object will record the previous
> lookup class from which this |Lookup| object was teleported such that
> the access check will use both the previous lookup class and the current
> |lookup| context (current lookup class and allowed modes) to determine if
> a type is accessible to this `Lookup` object or not.
>
> In a nutshell, `T` in M2 is accessible to a `Lookup` object on `C`
> (lookup class in M1) and `PLC` (previous lookup class in M0) if and 
> only if
> 1. both M0 and M1 can read M2
> 2. T is in a package that is exported from M2 at least to both M0 and M1
>
> Detailed specification is in Lookup class spec and `accessClass` javadoc.
> The relevant spec about cross-module teleporting is in the Lookup class
> spec and `Lookup::in` and `MethodHandles::privateLookupIn`.
>
> CSR: https://bugs.openjdk.java.net/browse/JDK-8226916
>
> webrev:
> http://cr.openjdk.java.net/~mchung/jdk14/8173978/webrev.00
>
> javadoc:
> http://cr.openjdk.java.net/~mchung/jdk14/8173978/javadoc/java.base/java/lang/invoke/MethodHandles.Lookup.html 
>
>
> http://cr.openjdk.java.net/~mchung/jdk14/8173978/javadoc/java.base/java/lang/invoke/MethodHandles.html#privateLookupIn(java.lang.Class,java.lang.invoke.MethodHandles.Lookup) 
>
This is really good work and fixes several issues left over from JDK 9. 
The compatibility issues (mostly small/advanced) are unfortunate but 
necessary and I hope it won't impact too many things. It will need a 
detail release note once the CSR is done and the changes are in.

I've taken several passes over the changes and I don't see anything 
obviously wrong. One minor nit is that the @throws 
IllegalAccessException in accessClass(Class) needs to be updated to 
allow for it to thrown when not accessible from the previous lookup 
class. Also VerifyAccess::isModuleAccessible shouldn't need to if the 
package is exported unconditionally as it is covered by isExported(pn, 
m1) && isExported(pn, m2).

-Alan