[foreign] RFR 8222337: Binder use of MethodHandles.publicLookup is bogus

[Maurizio Cimadamore]

Fixed and pushed

Thanks!

Maurizio

On 11/04/2019 14:56, Sundararajan Athijegannathan wrote:
> Looks good!
>
> Minor comments:
>
> * Util.getCallbackMH may have a comment explaining why we're doing 
> this. i.e., to support binding of callbacks which may be in 
> implementation detail of a module (and hence may be in non-exported 
> package of the module that uses panama)
>
> * The test could avoid java.desktop. Could use java.base or some such. 
> That way, we can have minimal dependency from panama tests (just in 
> case panama is targeted to educed JDK :) )
>
> -Sundar
>
> On 11/04/19, 6:40 PM, Maurizio Cimadamore wrote:
>> Hi,
>> since JDK 9, the behavior of MethodHandles::lookup vs 
>> MethodHandles::publicLookup has changed slightly:
>>
>> 1) MethodHandles::lookup() does access check using full module graph 
>> (e.g. including checks whether the module of the lookup class 'reads' 
>> the module in which the accessed member is defined)
>>
>> 2) MethodHandles::publicLookup() does a more relaxed check - as long 
>> as the accessed member is in some public type defined in an 
>> _unconditionally_ exported package, access is granted. Module reads 
>> are _not_ taken into account.
>>
>> The lack of checks for module reads is what made use of 
>> MethodHandle::publicLookup appealing for Panama: after all, the 
>> binder lives in java.base and java.base does NOT read user-defined 
>> modules.
>>
>> That said, publicLookup() has issues - in that it only works if the 
>> package in which the member is defined is unconditionally exported. 
>> So, any modular use of the binder will quickly run into issues.
>>
>> The best option is to use a regular lookup, rather than a public one. 
>> But then we have problem with module reads; to address these we have 
>> two options:
>>
>> * add the required module reads dynamically
>> * call setAccessible on the reflective member we want to obtain a MH for
>>
>> Given the simplicity of the latter approach, I decided to go down 
>> that route. I also added a test which checks that we can create 
>> callbacks for interfaces which sit at various level of package 
>> 'exported-ness'.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~mcimadamore/panama/8222337/
>>
>> Cheers
>> Maurizio
>>