[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-nashorn-dev
Subject:    Re: RFR 8157310: jdk.dynalink.linker.support.Lookup should have more checks before adding module rea
From:       Attila Szegedi <szegedia () gmail ! com>
Date:       2016-05-20 7:41:31
Message-ID: 058DA9A2-C853-4BAF-BD41-7F13CA141ED7 () gmail ! com
[Download RAW message or body]

Seconded, this is indeed better. +1

> On 20 May 2016, at 10:38, Hannes Wallnoefer <hannes.wallnoefer@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@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
> > > > 
> > > > 
> 


[prev in list] [next in list] [prev in thread] [next in thread]