Re: RFR 8167614: Avoid module dependency from jdk.dynalink to jdk.internal.module of java.base module

[Attila Szegedi]

Looks good to me, +1.

It seems correct that it’s the responsibility of the users of Dynalink to add 
the read edges from their own modules.

Attila.

> On 12 Oct 2016, at 18:31, Sundararajan Athijegannathan 
>  wrote:
> 
> Updated nashorn webrev:
> http://cr.openjdk.java.net/~sundar/8167614/nashorn/webrev.01/
> 
> Changed to use Layer.boot().findModule.
> 
> Thanks
> 
> -Sundar
> 
> 
> On 10/12/2016 9:42 PM, Alan Bateman wrote:
>> On 12/10/2016 16:33, Sundararajan Athijegannathan wrote:
>> 
>>> :
>>> 
>>> Dynalink used to automatically add those necessary add edges. With the
>>> current change, nashorn adds necessary read edges. CallerSensitive
>>> methods are found only in java.base, java.logging, java.sql and
>>> java.sql.rowset modules - the first two are always present [in nashorn's
>>> compact1 dependency world]. The later two are checked for presence and
>>> read-edges are added conditionally (see ScriptLoader.java changes).
>> JDK-8154346 tracks fixing java.sql.DriverManager, there are
>> compatibility concerns to changing it but it is being looked at.
>> 
>> I'm not familiar with the issue in the java.sql.rowset module but it
>> may be that the security checks in SerialJavaObject::getFields can be
>> re-visited (I don't know all the history on that).
>> 
>> 
>>> 
>>> Yes, I need boot layer modules only and I'll change that.
>>> 
>> Thanks.
>> 
>> -Alan.
> 

Re: RFR 8167614: Avoid module dependency from jdk.dynalink to jdk.internal.module of java.base module

[Alan Bateman]

On 12/10/2016 17:31, Sundararajan Athijegannathan wrote:


Updated nashorn webrev:
http://cr.openjdk.java.net/~sundar/8167614/nashorn/webrev.01/

Changed to use Layer.boot().findModule.

That loos okay to me. I assume that once we fix the issues in java.sql 
that this code can be removed.


-Alan

Re: RFR 8167614: Avoid module dependency from jdk.dynalink to jdk.internal.module of java.base module

[Sundararajan Athijegannathan]

Dynalink normally uses unreflect with publicLookup to get method handles
for j.l.reflect.Method objects (found reflectively). But, publicLookup
can not be used to unreflect caller sensitive methods. So, dynalink uses
specific Lookup object from the callsite - for example, lookup of the
Nashorn script (generated) class. But, script module has to have
read-edge the module of the CallerSensitive method's class - or else
that unreflect will fail.

Dynalink used to automatically add those necessary add edges. With the
current change, nashorn adds necessary read edges. CallerSensitive
methods are found only in java.base, java.logging, java.sql and
java.sql.rowset modules - the first two are always present [in nashorn's
compact1 dependency world]. The later two are checked for presence and
read-edges are added conditionally (see ScriptLoader.java changes).

Yes, I need boot layer modules only and I'll change that.

-Sundar

On 10/12/2016 8:51 PM, Alan Bateman wrote:
> On 12/10/2016 16:11, Sundararajan Athijegannathan wrote:
>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8167614
>>
>> jdk webrev: http://cr.openjdk.java.net/~sundar/8167614/jdk/webrev.00/
>>
>> nashorn webrev:
>> http://cr.openjdk.java.net/~sundar/8167614/nashorn/webrev.00/
>>
> In jdk/nashorn/internal/runtime/Context.java then it checks if the
> java.sql and java.sql.rowset are observable. This does not mean they
> are in the boot layer, I think you want
> Layer.boot().findModule("java.sql").ifPresent(...). However, the this
> code in Nashorn makes me wonder if this is actually needed or why
> these two are special cased.
>
> -Alan

Re: RFR 8167614: Avoid module dependency from jdk.dynalink to jdk.internal.module of java.base module

[Alan Bateman]

On 12/10/2016 16:11, Sundararajan Athijegannathan wrote:


Bug: https://bugs.openjdk.java.net/browse/JDK-8167614

jdk webrev: http://cr.openjdk.java.net/~sundar/8167614/jdk/webrev.00/

nashorn webrev:
http://cr.openjdk.java.net/~sundar/8167614/nashorn/webrev.00/

In jdk/nashorn/internal/runtime/Context.java then it checks if the 
java.sql and java.sql.rowset are observable. This does not mean they are 
in the boot layer, I think you want 
Layer.boot().findModule("java.sql").ifPresent(...). However, the this 
code in Nashorn makes me wonder if this is actually needed or why these 
two are special cased.


-Alan

Re: RFR 8167614: Avoid module dependency from jdk.dynalink to jdk.internal.module of java.base module

[Jim Laskey (Oracle)]

+1

> On Oct 12, 2016, at 12:11 PM, Sundararajan Athijegannathan 
>  wrote:
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8167614
> 
> jdk webrev: http://cr.openjdk.java.net/~sundar/8167614/jdk/webrev.00/
> 
> nashorn webrev:
> http://cr.openjdk.java.net/~sundar/8167614/nashorn/webrev.00/
> 
> Thanks,
> 
> -Sundar
>