RFR 8157310: jdk.dynalink.linker.support.Lookup should have more checks before adding module read link

[Attila Szegedi]

Seconded, this is indeed better. +1

> On 20 May 2016, at 10:38, Hannes Wallnoefer <hannes.wallnoefer at oracle.com> wrote:
> 
> Hi Sundar,
> 
> I agree methods feel more in their place now.
> 
> Hannes
> 
> Am 2016-05-20 um 09:12 schrieb Sundararajan Athijegannathan:
>> Hi,
>> 
>> Thanks for the review. But, I adjusted the change slightly -- moved
>> addModuleRead and caller-sensitive fallback attempt to
>> CallerSensitiveDynamicMethod itself - leaving Lookup as general
>> exception translating version of j.l.invoke.MethodHandle.Lookup.
>> 
>> I think this is better caller sensitivity handling moved to specific
>> place and Lookup remains a generic API.
>> 
>> http://cr.openjdk.java.net/~sundar/8157310/webrev.01/
>> 
>> Thanks,
>> 
>> -Sundar
>> 
>> 
>> On 5/20/2016 12:22 PM, Attila Szegedi wrote:
>>> +1. Nice work!
>>> 
>>>> On 19 May 2016, at 15:28, Sundararajan Athijegannathan <sundararajan.athijegannathan at oracle.com> wrote:
>>>> 
>>>> Please review http://cr.openjdk.java.net/~sundar/8157310/ for
>>>> https://bugs.openjdk.java.net/browse/JDK-8157310
>>>> 
>>>> What is this fix?
>>>> 
>>>> * When unreflecting caller sensitive methods, dynalink uses specific
>>>> Lookup of actual caller (instead of publicLookup) - in nashorn's case
>>>> it's be the Lookup of script class.
>>>> 
>>>> * Script class may not have read link to the module of the class (of
>>>> caller sensitive member). If there is any IllegalAccessError, dynalink
>>>> adds the read link and tries to
>>>> 
>>>> unreflect again. We don't want unnecessary module link read in such
>>>> cases. Check has been added whether the package is exported from
>>>> declaring module. Note that there is no security issue here.  Even
>>>> before the fix, module read edge does not give any capability. unreflect
>>>> call after adding unnecessary read line would still fail for
>>>> non-exported package case. There were unnecessary module read links
>>>> created - which is being avoided now.
>>>> 
>>>> * Additional "API" in Lookup.java slipped from jigsaw code into
>>>> jdk9-dev. Those unreflectCallerSensitive and
>>>> unreflectConstructorCallerSensitive were meant to be 'interim' somehow
>>>> slipped.  Now, only unreflect and unreflectConstructor in Lookup. Caller
>>>> sensitiveness is hidden in the implementation.  These methods were never
>>>> APIs when dynalink API was created.
>>>> 
>>>> Thanks,
>>>> 
>>>> -Sundar
>>>> 
>>>> 
>