Re: Ant JunitTask update to support JDK9 modules

[Tomas Zezula]

Hi Alan,
> On 28 Apr 2016, at 13:22, Alan Bateman  wrote:
> 
> On 27/04/2016 17:17, Tomas Zezula wrote:
>> As seen in the modulepath and classpath mixture thread there are many 
>> possibilities how to execute unit tests.
>> I've sumarised them on the ant-dev mailing list in the context of Apache Ant 
>> in 
>> http://mail-archives.apache.org/mod_mbox/ant-dev/201604.mbox/%3cafe6c849-0622-44d1-9ff7-3a6ca4832...@oracle.com%3E
>> As a result of the discussion I've created a pull request for Apache Ant 
>> JUnitTask to support JDK9 modules:
>> https://github.com/apache/ant/pull/18
> On the qualified export approach then one thing you say is that it requires 
> compiling with a "single multi-module compilation". You are right that 
> compiling MyLibrary with a qualified export to MyLibraryTests will require 
> MyLibraryTests to be on the module source path but you should be able to 
> compile them separately if you want, this should do it:
> 
> javac -modulesourcepath src -d mods $(find src/MyLibrary -name "*.java")
> javac -modulepath mods -d mods/MyLibraryTests $(find src/MyLibraryTests -name 
> "*.java”)
Yes, this works.
The MyLibrary is compiled with -modulesourcepath src to make MyLibraryTests 
module resolvable from MyLibrary’s module-info.
> 
> 
> 
> On the split package issue then ant-junit and ant-junit4.jar does seem 
> unusual. Do you know if they are in the same package because one needs 
> package private access to something in the other?
I believe that they do not have package private dependencies. And the packaging 
comes  historically from the source layout. The sources are in the same package 
and then packed into ant-junit (the optional JUnitTask and Ant’s test runner) 
ant-junit4 (containing just the Adapter from JUnit 4 API to Ant 3.8 API).
> I have no interest in making the options more complicated but if you really 
> want to then -Xpatch can patch automatic modules and so 
> `-Xpatch:ant.junit=ant-junit4.jar` will eventually overlay the contents of 
> ant-junit4.jar over module ant.junit.
Thanks Alan, it’s good to know this. These two jars are Ant implementation 
details, they were always put on classpath by Ant. I will try to keep them in 
the UNNAMED-MODULE. There is no need to
make them automatic modules because no one depends on them, they are only a 
test launcher.
— Tomas
> 
> -Alan

Re: #ReflectiveAccessByInstrumentationAgents

[Paul Benedict]

I prefer the originally-proposed addExports() but allowing the layer to
authorize it -- with a security check or other custom constraint the
implementer chooses to code.

Cheers,
Paul

On Fri, May 6, 2016 at 9:34 AM, Peter Levart  wrote:

> Hi,
>
>
> On 05/06/2016 10:07 AM, Andrew Dinn wrote:
>
>> >Have you looked at injecting code in the victim module that invokes
>>> >Module addExports to export the packages to the Byteman modules (modules
>>> >plural because it sounds like the code is split between the unnamed
>>> >module of the app class loader and the unnamed modulie of the boot
>>> loader).
>>>
>> Not yet but I will do. n.b. for all practical purposes there is only one
>> Byteman module although which one it is depends upon whether the agent
>> jar is hoisted into the bootstrap or remains in the system loader. In
>> the former case the code is split but only in that the Main class gets
>> left in the system loader  while all others sit in bootstrap. Since Main
>> merely acts as a trampoline it does not require any special module
>> linkage.
>> regards,
>>
>>
>> Andrew Dinn
>>
>
> So if Main agent class is always loaded by bootstrap loader / unnamed
> module, what about an API point in Instrumentation like:
>
> void addModuleExportsToBootstrapUnnamed(Module module, String pn);
>
> This would only allow adding exports from arbitrary module / package to
> bootstrap unnamed module, which would contain (part of) agent. This part of
> agent could also contain trampolines with regulated access from
> instrumented / tested classes. Every call to such exposed code would have
> to be done by invocation through Byteman layer then which could regulate
> access by any means including but not limited to exporting the Byteman code
> package to selected set of modules.
>
> Such limited addExports API would not allow to break encapsulation in
> arbitrary ways.
>
> Regards, Peter
>
>

Re: Ant JunitTask update to support JDK9 modules

[Tomas Zezula]

> On 27 Apr 2016, at 23:26, Alex Buckley  wrote:
> 
> On 4/27/2016 1:41 PM, Jan Lahoda wrote:
>> On 27.4.2016 21:30, Alex Buckley wrote:
>>> Your changes skyrocket in complexity because your Ant setup involves
>>> both ant-junit.jar and ant-junit4.jar. The JUnit task documentation
>>> (https://ant.apache.org/manual/Tasks/junit.html) doesn't suggest this is
>>> necessary or desirable. It's either pointless or dangerous to have both
>>> JARs on the classpath, which is why the module system rejects them as
>> 
>> I think it would be useful if you could elaborate on why using both
>> ant-junit.jar and ant-junit4.jar is pointless or dangerous. What I see
>> in the jars, the org.apache.tools.ant.taskdefs.optional.junit package is
>> perfectly partitioned between the jars (there is no class that would be
>> in both jars). It seems ant-junit4.jar cannot work by itself, without
>> ant-junit.jar. I am not 100% sure what ant-junit4.jar does, but it seems
>> it aids in running single test method in a JUnit 4 way. There appears to
>> be some fallback to JUnit 3 style if the JUnit 4 style is not available,
>> but it is hard to say if that is sufficient. So dropping (the content
>> of) ant-junit4.jar would mean at least careful testing, but more
>> probably a loss of a significant feature.
> 
> Perfect partitioning of a split package is uncommon. The common case is some 
> overlap of types, leading to obscure mix-and-match problems, so the module 
> system rejects a split package unconditionally. Ant's redistribution of JUnit 
> JARs (or perhaps its JUnit's own scheme for v3-to-v4 migration) is 
> architected in a way that's at odds with the module system.
> 
> It might be possible to make a case for relaxing the anti-split-package 
> provision for automatic modules, since they're coming from the classpath 
> world where every artifact is "open" to combination with artifacts elsewhere 
> on the path. But that would reduce reliability, since generally it's a 
> _feature_ that the module system warns:
> 
> java.lang.module.ResolutionException: Modules ant.junit4 and ant.junit export 
> package org.apache.tools.ant.taskdefs.optional.junit
> to module hamcrest.core
It’s exactly as Jan described. The ant-junit.jar and ant-junit4.jar are 
perfectly partitioned. Neither the ant-junit.jar nor the ant-junit4.jar is a 
redistribution of junit.
The ant-junit.jar contains an implementation of Ant specific junit runner, the 
Ant does not use org.junit.runner.JUnitCore runner. The ant-junit4.jar contains 
just
an adapter of Junit 4 API to JUnit 3.8 API for execution of tests and listening 
of events.
These two jars were always an Ant implementation detail, there were put on 
classpath of the tested application automatically by an Ant. No change in this.
The junit library needs to be either on classpath or newly on modulepath. I’ve 
described both these scenarios in the junit.html of the Ant manual.
I personally prefer to put the test framework on classpath (unnamed module) 
with -XaddReads:${module.name}=ALL-UNNAMED 
-XaddExports:${module.name}/my.test=ALL-UNNAMED
especially in case of generic generated build scripts. It’s very easy for an 
user to add another test framework just by adding it on classpath. There is no 
need to modify jvm options as the unnamed module
is already covered.
— Tomas
> 
> Alex

Re: #ReflectiveAccessByInstrumentationAgents

[Peter Levart]

Hi,


On 05/06/2016 10:07 AM, Andrew Dinn wrote:

>Have you looked at injecting code in the victim module that invokes
>Module addExports to export the packages to the Byteman modules (modules
>plural because it sounds like the code is split between the unnamed
>module of the app class loader and the unnamed modulie of the boot loader).

Not yet but I will do. n.b. for all practical purposes there is only one
Byteman module although which one it is depends upon whether the agent
jar is hoisted into the bootstrap or remains in the system loader. In
the former case the code is split but only in that the Main class gets
left in the system loader  while all others sit in bootstrap. Since Main
merely acts as a trampoline it does not require any special module linkage.
regards,


Andrew Dinn


So if Main agent class is always loaded by bootstrap loader / unnamed 
module, what about an API point in Instrumentation like:


void addModuleExportsToBootstrapUnnamed(Module module, String pn);

This would only allow adding exports from arbitrary module / package to 
bootstrap unnamed module, which would contain (part of) agent. This part 
of agent could also contain trampolines with regulated access from 
instrumented / tested classes. Every call to such exposed code would 
have to be done by invocation through Byteman layer then which could 
regulate access by any means including but not limited to exporting the 
Byteman code package to selected set of modules.


Such limited addExports API would not allow to break encapsulation in 
arbitrary ways.


Regards, Peter

Re: #ReflectiveAccessByInstrumentationAgents

[Alan Bateman]

On 06/05/2016 09:47, Andrew Dinn wrote:

What I don't want to do is export the API provided by a concealed
class/package that exposes a module check -free setAccessible to
anything other than the Byteman class which uses it. If I follow your
suggestion then I would potentially expose setAccessible to any class in
the system classpath. So, rather than use module import/export checks I
think I need to employ a caller class check.

A caller check seems overkill here but to your point, yes, if you coerce 
the target module to export packages to the unnamed module of the app 
class loader then it means that anything on the class path can access 
public types in those packages or they can use setAccessible(true) to 
hack into non-public types and members. From your earlier mails then it 
might be that you only need to export to the unnamed module of the boot 
loader so maybe this is not a concern.


There are other more sophisticated approaches but I'm not sure if it's 
worth going into them now. For example deploying java agents as explicit 
modules (open question on whether this is needed in the short term) or 
using dynamic modules. I think that is a topic for another thread and 
another day.


-Alan

Re: #ReflectiveAccessByInstrumentationAgents

[Andrew Dinn]

On 06/05/16 09:37, Alan Bateman wrote:
> On 06/05/2016 09:16, Andrew Dinn wrote:
>> :
>> Yes, that's probably the best way to restrict access if you assume the
>> agent is itself in a Jigsaw module.
> Your agent may not be in a named module but it will be in an unnamed
> module. Once you catch up on the concepts then I think it should be much
> clearer.

Oh, no, I grokked that. The way I intend to use it (i.e. compatibly with
what happens in earlier JDKs) my agent it will be loaded by either the
bootstrap or system loader and hence will be in their respective unnamed
module.

What I don't want to do is export the API provided by a concealed
class/package that exposes a module check -free setAccessible to
anything other than the Byteman class which uses it. If I follow your
suggestion then I would potentially expose setAccessible to any class in
the system classpath. So, rather than use module import/export checks I
think I need to employ a caller class check.

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

hg: jigsaw/jake/jdk: 67 new changesets

[alan . bateman]

Changeset: 41f95dde9770
Author:mrkam
Date:  2016-04-25 16:29 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/41f95dde9770

8154594: JFrame.setDefaultCloseOperation is prohibited in jtreg: Missing part 
of the fix
Reviewed-by: alexsch, prr

! 
test/sanity/client/lib/SwingSet3/src/com/sun/swingset3/demos/button/ButtonDemo.java
! 
test/sanity/client/lib/SwingSet3/src/com/sun/swingset3/demos/tabbedpane/TabbedPaneDemo.java
! 
test/sanity/client/lib/SwingSet3/src/com/sun/swingset3/demos/window/WindowDemo.java

Changeset: d24c177b70ac
Author:mrkam
Date:  2016-04-25 16:34 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/d24c177b70ac

8154706: Sanity tests prepareBundle task doesn't produce working bundle
Reviewed-by: alexsch, prr

! test/sanity/client/TEST.ROOT.template

Changeset: 5e70a502b37c
Author:jlaskey
Date:  2016-04-26 11:55 -0300
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/5e70a502b37c

8132994: /modules and /packages should not be parsed by the jimage parser
Reviewed-by: sundar

! src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ImageLocationWriter.java

Changeset: ae10dd067bcd
Author:rriggs
Date:  2016-04-26 17:35 -0400
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/ae10dd067bcd

8066750: Remove HTTP proxy implementation and tests from RMI
Reviewed-by: smarks

! src/java.rmi/share/classes/java/rmi/server/RMISocketFactory.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/CGIHandler.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpAwareServerSocket.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpInputStream.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpOutputStream.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpReceiveSocket.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpSendInputStream.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpSendOutputStream.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpSendSocket.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/RMIDirectSocketFactory.java
- 
src/java.rmi/share/classes/sun/rmi/transport/proxy/RMIHttpToCGISocketFactory.java
- 
src/java.rmi/share/classes/sun/rmi/transport/proxy/RMIHttpToPortSocketFactory.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/RMIMasterSocketFactory.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/RMISocketInfo.java
- src/java.rmi/share/classes/sun/rmi/transport/proxy/WrappedSocket.java
! src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPConnection.java
+ src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPDirectSocketFactory.java
! src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPTransport.java
! test/ProblemList.txt
- test/java/rmi/transport/httpSocket/HttpSocketTest.java
- test/java/rmi/transport/httpSocket/HttpSocketTest_Stub.java
- test/java/rmi/transport/httpSocket/security.policy
- test/sun/rmi/transport/proxy/DisableHttpDefaultValue.java
- test/sun/rmi/transport/proxy/EagerHttpFallback.java
- test/sun/rmi/transport/tcp/blockAccept/BlockAcceptTest.java
- test/sun/rmi/transport/tcp/blockAccept/TestIface.java
- test/sun/rmi/transport/tcp/blockAccept/TestImpl.java
- test/sun/rmi/transport/tcp/blockAccept/TestImpl_Stub.java
- test/sun/rmi/transport/tcp/blockAccept/security.policy

Changeset: 3243b2b2a365
Author:rriggs
Date:  2016-04-26 21:25 -0400
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/3243b2b2a365

8155182: fix to JDK-8066750 broke jdk9 builds
Summary: Restore RMI Http Proxy support for now
Reviewed-by: darcy, lancea, smarks

! src/java.rmi/share/classes/java/rmi/server/RMISocketFactory.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/CGIHandler.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpAwareServerSocket.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpInputStream.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpOutputStream.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpReceiveSocket.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpSendInputStream.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpSendOutputStream.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/HttpSendSocket.java
! 
src/java.rmi/share/classes/sun/rmi/transport/proxy/RMIDirectSocketFactory.java 
< src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPDirectSocketFactory.java
+ 
src/java.rmi/share/classes/sun/rmi/transport/proxy/RMIHttpToCGISocketFactory.java
+ 
src/java.rmi/share/classes/sun/rmi/transport/proxy/RMIHttpToPortSocketFactory.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/RMIMasterSocketFactory.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/RMISocketInfo.java
+ src/java.rmi/share/classes/sun/rmi/transport/proxy/WrappedSocket.java
! src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPConnection.java
! src/java.rmi/share/classes/sun/rmi/transport/tcp/TCPTransport.java
! 

hg: jigsaw/jake/jaxws: 2 new changesets

[alan . bateman]

Changeset: 2d52e0610b05
Author:lana
Date:  2016-05-05 17:35 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxws/rev/2d52e0610b05

Added tag jdk-9+117 for changeset 58265b39fc74

! .hgtags

Changeset: 1d96ce2eaed2
Author:alanb
Date:  2016-05-06 08:08 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxws/rev/1d96ce2eaed2

Merge

hg: jigsaw/jake/jaxp: 5 new changesets

[alan . bateman]

Changeset: af1875980cbf
Author:joehw
Date:  2016-04-28 09:08 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/af1875980cbf

8154220: Semi-colon delimited list of catalog files in System property is 
throwing IllegalArgumentException
Reviewed-by: lancea

! src/java.xml/share/classes/javax/xml/catalog/CatalogFeatures.java
! test/javax/xml/jaxp/unittest/catalog/CatalogTest.java
+ test/javax/xml/jaxp/unittest/catalog/first_cat.xml
+ test/javax/xml/jaxp/unittest/catalog/next_cat.xml
+ test/javax/xml/jaxp/unittest/catalog/second_cat.xml

Changeset: 4b13796fa518
Author:lana
Date:  2016-04-28 09:39 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/4b13796fa518

Merge


Changeset: 46b57560cd06
Author:fyuan
Date:  2016-04-28 19:11 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/46b57560cd06

8155514: jaxp.library.TestPolicy should extend the default security policy
Reviewed-by: mchung, joehw
Contributed-by: Frank Yuan 

! test/javax/xml/jaxp/libs/jaxp/library/TestPolicy.java

Changeset: 6513699ddc34
Author:lana
Date:  2016-05-05 17:35 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/6513699ddc34

Added tag jdk-9+117 for changeset 46b57560cd06

! .hgtags

Changeset: 987a44004fe3
Author:alanb
Date:  2016-05-06 08:08 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/jaxp/rev/987a44004fe3

Merge

hg: jigsaw/jake/nashorn: 5 new changesets

[alan . bateman]

Changeset: bafd733be429
Author:hannesw
Date:  2016-04-27 15:50 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/bafd733be429

8134503: support ES6 parsing in Nashorn
Reviewed-by: jlaskey, sundar, mhaupt
Contributed-by: andreas.wo...@oracle.com

! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CodeGenerator.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/CompilationPhase.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/SplitIntoFunctions.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/codegen/WeighNodes.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/AccessNode.java
! src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/BaseNode.java
! src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/Block.java
+ src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/ClassNode.java
+ 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/ExpressionList.java
! src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/ForNode.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/FunctionNode.java
! src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/IdentNode.java
! src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/IndexNode.java
+ src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/Module.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/PropertyNode.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/visitor/NodeOperatorVisitor.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/ir/visitor/NodeVisitor.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/Parser.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/ParserContext.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/ParserContextFunctionNode.java
+ 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/ParserContextModuleNode.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/parser/TokenType.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/resources/Messages.properties
! src/jdk.scripting.nashorn/share/classes/jdk/nashorn/tools/Shell.java
+ test/script/basic/es6/parser-es6.js
- test/script/basic/yield.js
! test/script/error/NASHORN-154/function_mult_params_in_strict.js.EXPECTED
! test/script/nosecurity/parserapi.js.EXPECTED
! test/script/nosecurity/parserapi_strict.js.EXPECTED
! test/script/nosecurity/treeapi/array_literal.js.EXPECTED
! test/script/nosecurity/treeapi/objectLiteral.js.EXPECTED
! test/script/nosecurity/treeapi/property.js.EXPECTED
! test/script/nosecurity/treeapi/throw.js.EXPECTED
! test/script/nosecurity/treeapi/with.js.EXPECTED
! test/src/jdk/nashorn/internal/test/framework/ScriptRunnable.java

Changeset: fd2296436748
Author:hannesw
Date:  2016-04-28 10:42 +0200
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/fd2296436748

8155025: 0.001.toFixed(2) should return "0.00" not "0"
Reviewed-by: jlaskey, hannesw
Contributed-by: andreas.wo...@oracle.com

! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/doubleconv/DtoaBuffer.java
+ test/script/basic/JDK-8155025.js
+ test/script/basic/JDK-8155025.js.EXPECTED

Changeset: 5267e9181161
Author:lana
Date:  2016-04-28 09:38 -0700
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/5267e9181161

Merge

- test/script/basic/yield.js

Changeset: b0c9a78aee9d
Author:lana
Date:  2016-05-05 17:35 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/b0c9a78aee9d

Added tag jdk-9+117 for changeset 5267e9181161

! .hgtags

Changeset: 878f812e33db
Author:alanb
Date:  2016-05-06 08:09 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/nashorn/rev/878f812e33db

Merge

! .hgtags
! 
src/jdk.scripting.nashorn/share/classes/jdk/nashorn/internal/runtime/Context.java
- test/script/basic/yield.js
! test/src/jdk/nashorn/internal/test/framework/ScriptRunnable.java

hg: jigsaw/jake/corba: 2 new changesets

[alan . bateman]

Changeset: 18695748cc41
Author:lana
Date:  2016-05-05 17:35 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/corba/rev/18695748cc41

Added tag jdk-9+117 for changeset 7a1b36bf2fe5

! .hgtags

Changeset: 285b3e979423
Author:alanb
Date:  2016-05-06 08:08 +0100
URL:   http://hg.openjdk.java.net/jigsaw/jake/corba/rev/285b3e979423

Merge

Re: #ReflectiveAccessByInstrumentationAgents

[Alan Bateman]

On 06/05/2016 09:16, Andrew Dinn wrote:

:
Yes, that's probably the best way to restrict access if you assume the
agent is itself in a Jigsaw module.
Your agent may not be in a named module but it will be in an unnamed 
module. Once you catch up on the concepts then I think it should be much 
clearer.


-Alan.

Re: #ReflectiveAccessByInstrumentationAgents

[Michael Rasmussen]

> Thanks very much for offering me this code but before I can look at it I
> actually need explicit confirmation that I am able to release it under
> LGPL (sorry to be fussy but I don't want any legal issues to cause my
> employer a problem). If you could acknowledge that permission then I'll
> be able to click on the link.
Permission granted.

/Michael

Re: #ReflectiveAccessByInstrumentationAgents

[Andrew Dinn]

On 06/05/16 09:18, Michael Rasmussen wrote:
>> Is your code open source? More importantly, is its license compatible
>> with the LGPL license used by Byteman? If so then can I take a peek?
>> (and maybe steal it :-)
> 
> Nothing fancy about it, just a simple class generated via ASM and then
> defined using unsafe
> 
> Here's a simple main class with it (changed to access override field
> instead, to be compatible with JDK8 as well)
> https://gist.github.com/anonymous/7dacfe87fa9517c7c82d4e910d6f7300
> 
> Feel free to use it as you see fit, consider the above MIT, BSD,
> public domain, or what ever you feel comfortable with :)

Thanks very much for offering me this code but before I can look at it I
actually need explicit confirmation that I am able to release it under
LGPL (sorry to be fussy but I don't want any legal issues to cause my
employer a problem). If you could acknowledge that permission then I'll
be able to click on the link.

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

Re: #ReflectiveAccessByInstrumentationAgents

[Michael Rasmussen]

> Is your code open source? More importantly, is its license compatible
> with the LGPL license used by Byteman? If so then can I take a peek?
> (and maybe steal it :-)

Nothing fancy about it, just a simple class generated via ASM and then
defined using unsafe

Here's a simple main class with it (changed to access override field
instead, to be compatible with JDK8 as well)
https://gist.github.com/anonymous/7dacfe87fa9517c7c82d4e910d6f7300

Feel free to use it as you see fit, consider the above MIT, BSD,
public domain, or what ever you feel comfortable with :)

/Michael

Re: #ReflectiveAccessByInstrumentationAgents

[Andrew Dinn]

On 06/05/16 08:59, Alan Bateman wrote:
> On 06/05/2016 08:32, Peter Levart wrote:
>> I think that not giving the agent such API just makes life for agent
>> writers harder but does not prevent them to break encapsulation in
>> arbitrary ways anyway. 
> 
> We've been over this a few times, it's on the issues list, but we want
> to see how far we can get without adding this method.

Ok, I'll be that guinea pig.

> On injecting helpers then if the agent can inject an initializer +
> helper method into any public class in the module then it has full
> control. It doesn't matter if it's a concealed package because the
> initializer (which the agent can trigger to execute without access) just
> needs to export the package to the agent.

Yes, that's probably the best way to restrict access if you assume the
agent is itself in a Jigsaw module. But I doubt all (or even most?)
agents will want to use Jigsaw. They certainly won't if they want to
implement functionality that is useful to pre- and post-JDK9 runtimes
since that will force the devs down the path of dual-source and
dual-produuct. That is a very unattractive proposition since it
complicates both development and deployment.

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

Re: #ReflectiveAccessByInstrumentationAgents

[Andrew Dinn]

On 06/05/16 07:29, Alan Bateman wrote:
> On 05/05/2016 23:26, Peter Levart wrote:
>> What about adding an all-powers addModuleExports(module, pn, other)
>> method to java.lang.instrument.Instrumentation (like it was done with
>> addModuleReads) to simplify the agent's task? An agent could be
>> considered trusted code, couldn't it?
> It was looked into but has the potential to break encapsulation in
> arbitrary ways. I think we have to see how far we can get with injecting
> code into the target module to export the packages to the agent or other
> module that the agent is arranging to access the internals.

Well, actually, I am deliberately trying to break encapsulation in
arbitrary ways :-)

although it is not really quite that arbitrary. I am only doing so for
very strictly limited purposes (testing, monitoring/debugging) and only
so as to grant access to 'trusted' code.

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

Re: #ReflectiveAccessByInstrumentationAgents

[Andrew Dinn]

On 05/05/16 20:50, Alan Bateman wrote:
> 
> On 05/05/2016 17:31, Andrew Dinn wrote:
>> :
>>
>> I looked at several ways of making this work and decided the best thing
>> was to have the agent redefine AccessibleObject.checkCanSetAccessible so
>> that it grants Byteman code (specifically one Byteman class called Rule)
>> free reign when it calls this method. This still retains security
>> manager checks made around this call. Choosing this method targets the
>> code which does module access checks.
>>
> This is very fragile because checkCanSetAccessible is not part of the
> API and may go away or change at any time.

Yes, that is true. I was assuming that

  once baked into JDK9 this was very unlikely to change

  if it did I could upgrade Byteman to implement another such hack

However, I think you are right that what I need to do is implement
something that depends on a guaranteed property e.g. inject a class
which makes the call into the module of java.lang.Object (java .base).
I'll still need to make sure it can only be called from my Rule class so
I can guarantee that it does not leak access.

> Have you looked at injecting code in the victim module that invokes
> Module addExports to export the packages to the Byteman modules (modules
> plural because it sounds like the code is split between the unnamed
> module of the app class loader and the unnamed modulie of the boot loader).

Not yet but I will do. n.b. for all practical purposes there is only one
Byteman module although which one it is depends upon whether the agent
jar is hoisted into the bootstrap or remains in the system loader. In
the former case the code is split but only in that the Main class gets
left in the system loader  while all others sit in bootstrap. Since Main
merely acts as a trampoline it does not require any special module linkage.
regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

Re: #ReflectiveAccessByInstrumentationAgents

[Alan Bateman]

On 06/05/2016 08:32, Peter Levart wrote:

:

The easiest thing for agent to do then is to observe class loading and 
when 1st class of a particular module located in an exported package 
is about to be loaded, instruments it and adds a static method to it - 
a trampoline that invokes Module.addExports(pn, other). The added 
method can then be invoked via reflection.


But what if agent needs to augment exports of some module before the 
1st exported class of the module comes around? Suppose that classes in 
concealed packages are loaded 1st and that they already start 
executing code in them before the 1st exported class is loaded. Agent 
would have to piggy-back the exports augmentation code on the 
instrumented methods themselves to ensure exports are added soon 
enough. What if instrumentation has the goal to introduce as little 
overhead as possible? Adding redundant Module.addExports() invocations 
all over the place on hot paths would not help to achieve that goal.


I think that not giving the agent such API just makes life for agent 
writers harder but does not prevent them to break encapsulation in 
arbitrary ways anyway. 


We've been over this a few times, it's on the issues list, but we want 
to see how far we can get without adding this method.


On injecting helpers then if the agent can inject an initializer + 
helper method into any public class in the module then it has full 
control. It doesn't matter if it's a concealed package because the 
initializer (which the agent can trigger to execute without access) just 
needs to export the package to the agent.


-Alan

Re: #ReflectiveAccessByInstrumentationAgents

[Andrew Dinn]

On 05/05/16 23:26, Peter Levart wrote:
> What about adding an all-powers addModuleExports(module, pn, other)
> method to java.lang.instrument.Instrumentation (like it was done with
> addModuleReads) to simplify the agent's task? An agent could be
> considered trusted code, couldn't it?

Well, that was actually something I also thought of and it is a /very/
neat solution. Indeed the thought behind your rhetorical question is
what motivated me to raise #ReflectiveAccessByInstrumentationAgents as a
missing requirement. An agent really should be considered trusted code,
shouldn't it.

Much as I think this would be the best general solution it doesn't quite
work for me, for practical rather than technical reasons. The downside
of using an API extension is that it requires compiling the agent as
JDK9 code. I need my agent to run on 1.6+ and currently build it using
target 1.6.

I could finesse this issue by releasing both a JDK8- agent and a JDK9+
agent. That path is unattractive because it requires me to maintain dual
source & product. That requires users to reconfigure their build and
runtime paths when they switch between JDK8 and JDK9. This doesn't sound
like much but you would be astonished how many users fail at configuring
this sort of thing (no, /really/).

Anyway, if it came to a beauty contest I'd definitely vote for extending
Instrumentation with yet another God-like power i.e. its own
setAccessible API without any module checks. Of course, I'd prefer that
we adopt this on its merits rather than by resort to mob rule^H^H^H^H
democracy.

regards,


Andrew Dinn
---
Senior Principal Software Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander

Re: #ReflectiveAccessByInstrumentationAgents

[Andrew Dinn]

On 05/05/16 18:38, Michael Rasmussen wrote:
> While waiting for some proper way of doing this, in my current prototype
> for supporting jigsaw I'm simply injecting a class into the
> java.lang.reflect package that can call setAccessible0, in order to
> circumvent these checks.
> Probably not what was envisioned either :)

Hmm, I considered that but had not investigated modules in enough detail
to determine how to do it. So, I took the easy route and knobbled the
access check code.

Is your code open source? More importantly, is its license compatible
with the LGPL license used by Byteman? If so then can I take a peek?
(and maybe steal it :-)

regards,


Andrew Dinn
---

Re: #ReflectiveAccessByInstrumentationAgents

[Peter Levart]

On 05/06/2016 08:29 AM, Alan Bateman wrote:

On 05/05/2016 23:26, Peter Levart wrote:


What about adding an all-powers addModuleExports(module, pn, other) 
method to java.lang.instrument.Instrumentation (like it was done with 
addModuleReads) to simplify the agent's task? An agent could be 
considered trusted code, couldn't it?
It was looked into but has the potential to break encapsulation in 
arbitrary ways. I think we have to see how far we can get with 
injecting code into the target module to export the packages to the 
agent or other module that the agent is arranging to access the internals.


-Alan


The easiest thing for agent to do then is to observe class loading and 
when 1st class of a particular module located in an exported package is 
about to be loaded, instruments it and adds a static method to it - a 
trampoline that invokes Module.addExports(pn, other). The added method 
can then be invoked via reflection.


But what if agent needs to augment exports of some module before the 1st 
exported class of the module comes around? Suppose that classes in 
concealed packages are loaded 1st and that they already start executing 
code in them before the 1st exported class is loaded. Agent would have 
to piggy-back the exports augmentation code on the instrumented methods 
themselves to ensure exports are added soon enough. What if 
instrumentation has the goal to introduce as little overhead as 
possible? Adding redundant Module.addExports() invocations all over the 
place on hot paths would not help to achieve that goal.


I think that not giving the agent such API just makes life for agent 
writers harder but does not prevent them to break encapsulation in 
arbitrary ways anyway.


Regards, Peter

Re: Modules with packages duplication

[Alan Bateman]

On 05/05/2016 21:49, Michael Rasmussen wrote:

:

If that is the case, what if in another layer, comprised of a single
classloader, I have a package split between multiple modules, how will
the current Instrumentation implementation work, since as you mention,
it maps loader+package to Module, which in that case won't be a 1-to-1
mapping (as implemented in JVM_GetModuleByPackageName from
JDK-8147465)

If all modules in the layer are defined to the same class loader then 
there can't be any overlapping packages. So I don't think there is an 
issue here.


-Alan

Re: #ReflectiveAccessByInstrumentationAgents

[Alan Bateman]

On 05/05/2016 23:26, Peter Levart wrote:


What about adding an all-powers addModuleExports(module, pn, other) 
method to java.lang.instrument.Instrumentation (like it was done with 
addModuleReads) to simplify the agent's task? An agent could be 
considered trusted code, couldn't it?
It was looked into but has the potential to break encapsulation in 
arbitrary ways. I think we have to see how far we can get with injecting 
code into the target module to export the packages to the agent or other 
module that the agent is arranging to access the internals.


-Alan