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

[Hannes Wallnoefer]

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
>>>
>>>