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.athijegannat...@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 >> >>
