Re: RFR(L) 8136930: Simplify use of module-system options by custom launchers

[Alan Bateman]

On 18/07/2016 00:05, harold seigel wrote:


Hi,

Please review these Hotspot VM only changes to process the seven 
module-specific options that have been renamed to have gnu-like 
names.  JDK changes for this bug will be reviewed separately.


Descriptions of these options are here 
.  For these six options, 
--module-path, --upgrade-module-path, --add-modules, --limit-modules, 
--add-reads, and --add-exports, the JVM just sets a system property.  
For the --patch-module option, the JVM sets a system property and then 
processes the option in the same way as when it was named -Xpatch.


Additionally, the JVM now checks properties specified on the command 
line.  If a property matches one of the properties used by one of the 
above options then the JVM ignores the property.  This forces users to 
use the explicit option when wanting to do things like add a module or 
a package export.


The RFR contains two new tests.  Also, many existing tests were 
changed to use the new option names.


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

Webrev: http://cr.openjdk.java.net/~hseigel/bug_8136930.hs/

I assume the module-info.java for jdk.hotspot.agent should be ignored.

Also, just to say that we'll probably aim to bring all the CLI changes 
into jdk9/dev in a few weeks, maybe the week of Aug 15-19. This requires 
coordinated changes in all repos. This initial push will have both the 
old and new module options, then we'll remove the old module options 
after a transition period. Not important for reviewing the hotspot 
changes, but might be useful in case there are questions.


-Alan

Re: RFR(L) 8136930: Simplify use of module-system options by custom launchers

[Alan Bateman]

On 18/07/2016 00:05, harold seigel wrote:


Hi,

Please review these Hotspot VM only changes to process the seven 
module-specific options that have been renamed to have gnu-like 
names.  JDK changes for this bug will be reviewed separately.


Descriptions of these options are here 
.  For these six options, 
--module-path, --upgrade-module-path, --add-modules, --limit-modules, 
--add-reads, and --add-exports, the JVM just sets a system property.  
For the --patch-module option, the JVM sets a system property and then 
processes the option in the same way as when it was named -Xpatch.


Additionally, the JVM now checks properties specified on the command 
line.  If a property matches one of the properties used by one of the 
above options then the JVM ignores the property.  This forces users to 
use the explicit option when wanting to do things like add a module or 
a package export.


The RFR contains two new tests.  Also, many existing tests were 
changed to use the new option names.


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

Webrev: http://cr.openjdk.java.net/~hseigel/bug_8136930.hs/
A passing comment on unsupported_options (options not allowed when using 
-Xshare:dump) then it has -m in the list. I assume that should not be 
there. That is java launcher option, the VM don't see or unrecognize it, 
also it's long form is --module.


-Alan

Re: It's not too late for access control

[Andrew Dinn]

On 16/07/16 11:34, dalibor topic wrote:
> 
> 
> On 15.07.2016 22:25, Jason T. Greene wrote:
>> The assumption you seem to make is that the use case of reflective
>> access to internal packages  is wrong, poor programming practice, or
>> an error.
>>
>> That couldn't be further from the truth.
> 
> As with many things, it kind of depends on who you ask:
> https://www.securecoding.cert.org/confluence/display/java/SEC05-J.+Do+not+use+reflection+to+increase+accessibility+of+classes,+methods,+or+fields
> 
> ...
> 
> In short, let's not argue about absolute statements one way or the other
> if we can avoid it.

If you reread Jason's statement above I think you will notice that this
is the point of his statement, to reject one such extreme. He did not
thereby recommend careless use of a potentially insecure capability.
Indeed he has taken great care to emphasise that what he wants (and what
I want) is a module system which provides a safe, controlled way of
opening up access to non-public members, retaining the opportunity to
implement the rich software tools that we currently have.

So, in sum, straw man, Dalibor.

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: Should setAccessible be part of Java or not? (was Re: It's not too late for access control)

[Andrew Haley]

On 15/07/16 19:29, Gregg Wonderly wrote:
> 
>> On Jul 14, 2016, at 6:51 AM, Andrew Haley  wrote:
>>
>>> This is #ReflectiveAccessToNonExportedTypes on the JSR 376 issues list. 
>>> The problem is reasonably well understood and there are several 
>>> proposals and approaches being discussed and considered.
>>
>> Forgive me if I've missed something, but
>> #ReflectiveAccessToNonExportedTypes does not deal with the need to
>> make fields or methods accessible to the framework.  That's what
>> setAccessible is used for.  It would certainly be nice for a
>> framework to be able to say "make it accessible, but only to me.”
> 
> That is the question though.  Why does it seem like a good idea to
> limit accessibility when there are so many other ways that the
> software can be exploited without this single limit being able to
> control all the others?

It's no more than a practical way of reducing software complexity.  (I
don't intend to explain why complexity is the enemy of reliability.)

System complexity is related to the number of connections between
components in a system.  By opening up access with setAccessible() you
add an interface to a module.  If that interface is only accessible to
one client, it's a 1:1 connection.  Making it globally accessible is
(potentially) a 1:N connection.  The wider the access, the more scope
for complex interfaces.

Also, making a method or field globally accessible may mean that the
author of the code can no longer guarantee its correctness.

This is commonplace computer science: it should be familiar to all of
us.

> Do you think that people would not decide to decompile your module
> definition and change all the details for visibility so that they
> can then do with it exactly as they need?  In this day and age, and
> especially in the public software realm, control of anything does
> not exist in practicality.  No matter what you believe might limit
> any aspect of your software, there is no way to categorically
> enforce that.

No, but we can reduce risk and the size of attack surfaces.  And, all
other things being equal, we should.

But we must do so in full awareness of the techniques that real-world
Java software uses.  The plasticity of Java has, to a very large
extent, led to its pre-eminence.  We must not lose that.  There are
bad uses of setAccessible() and good ones.

Andrew.

Re: #ModuleAnnotations - some thoughts and an alternative implementation

[Peter Levart]

Hi Alan,


On 07/18/2016 08:42 AM, Alan Bateman wrote:

On 17/07/2016 23:36, Peter Levart wrote:


:

Here's an alternative implementation that does not need to define the 
class in the VM:


http://cr.openjdk.java.net/~plevart/jdk9-jake/Module.annotations/webrev.02/ 



It instead modifies AnnotationParser to use a special 
AnnotationParser.ConstantPool interface which is implemented by two 
classes: a classical reflection ConstantPool backed by VM and a 
lightweight Java-only implementation which is used to parse 
module-info.class. So while parsing module-info.class, the 
RuntimeVisibleAnnotations attribute is extracted and memorized in 
ModuleDescriptor. Together with the lightweight ConstantPool 
implementation, they are used in Module to parse the annotations when 
1st requiested.


It is more involved than current implementation, but it is also 
lighter and might be extended to support - let's call them 
annotation-descriptors - on ModuleDescriptor.


What do you think?
One of the initial prototypes that we did was to have 
jdk.internal.reflect.ConstantPool backed by the constant pool in the 
module-info.class so that the raw annotations + constant pool could be 
used with the existing parser. So not too dissimilar to what you have 
in this patch.


With the proposal to keep things simple (in particular, not impacting 
ModuleDescriptor, Builder, ...) then it opened up the door for a much 
simpler implementations, hence the current implementation.


It is simpler, yes, but feels like overkill to create new class loader 
and transform and load a class in order to just parse annotations from it...




BTW: I see you've changed Module to implement GenericDeclaration 
rather than AnnotatedElement. I'm curious about.


This was just to simplify changes in 
sun.reflect.generics.factory.CoreReflectionFactory (used by 
AnnotationParser) which takes a GenericDeclaration. But it is odd to 
have a useless method that always returns an empty array, so a variant 
is to implement just AnnotatedElement and have an explicit Module field 
in CoreReflectionFactory:


http://cr.openjdk.java.net/~plevart/jdk9-jake/Module.annotations/webrev.03/

Regards, Peter



-Alan

Re: #ModuleAnnotations - some thoughts and an alternative implementation

[Alan Bateman]

On 18/07/2016 13:10, Peter Levart wrote:



It is simpler, yes, but feels like overkill to create new class loader 
and transform and load a class in order to just parse annotations from 
it...
Sure but I think it would be better things very simple for now because 
so many things are in flux. Also as I said in the fist reply, the 
approach is what the initial/prototype implementation was but moved to 
the simpler approach for that reason. No objection to look at this again 
in the future of course but for now then I think we have to keep the 
implementation really easy to spin.



This was just to simplify changes in 
sun.reflect.generics.factory.CoreReflectionFactory (used by 
AnnotationParser) which takes a GenericDeclaration. But it is odd to 
have a useless method that always returns an empty array, so a variant 
is to implement just AnnotatedElement and have an explicit Module 
field in CoreReflectionFactory:

Okay, I just curious about that as it seems unnecessary.

-Alan

Re: [9] RFR: 8159214: jlink --include-locales problems

[Naoto Sato]

Ping.

On 7/14/16 5:42 AM, Naoto Sato wrote:

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8159214

The fix is located at:

http://cr.openjdk.java.net/~naoto/8159214/webrev.04/

Naoto

Re: [9] RFR: 8159214: jlink --include-locales problems

[Jim Laskey (Oracle)]

+1

> On Jul 18, 2016, at 12:54 PM, Naoto Sato  wrote:
> 
> Ping.
> 
> On 7/14/16 5:42 AM, Naoto Sato wrote:
>> Hello,
>> 
>> Please review the fix to the following issue:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8159214
>> 
>> The fix is located at:
>> 
>> http://cr.openjdk.java.net/~naoto/8159214/webrev.04/
>> 
>> Naoto

Re: RFR(L) 8136930: Simplify use of module-system options by custom launchers

[harold seigel]

Hi Alan,

The JVM does not look for the -m (or --module) option.  Instead, it 
checks if property  "jdk.module.main" is non-null.  If it is non-null 
then the JVM uses the option name in its "Cannot use the following 
option when dumping the shared archive" message, as in:


> $JAVA_TEST/bin/java -Xshare:dump -m my_mod
Error occurred during initialization of VM
Cannot use the following option when dumping the shared archive: -m

Unfortunately, it prints "-m" regardless of whether -m or --module was 
specified.


Harold


On 7/18/2016 3:13 AM, Alan Bateman wrote:

On 18/07/2016 00:05, harold seigel wrote:


Hi,

Please review these Hotspot VM only changes to process the seven 
module-specific options that have been renamed to have gnu-like 
names.  JDK changes for this bug will be reviewed separately.


Descriptions of these options are here 
.  For these six options, 
--module-path, --upgrade-module-path, --add-modules, --limit-modules, 
--add-reads, and --add-exports, the JVM just sets a system property.  
For the --patch-module option, the JVM sets a system property and 
then processes the option in the same way as when it was named -Xpatch.


Additionally, the JVM now checks properties specified on the command 
line.  If a property matches one of the properties used by one of the 
above options then the JVM ignores the property. This forces users to 
use the explicit option when wanting to do things like add a module 
or a package export.


The RFR contains two new tests.  Also, many existing tests were 
changed to use the new option names.


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

Webrev: http://cr.openjdk.java.net/~hseigel/bug_8136930.hs/
A passing comment on unsupported_options (options not allowed when 
using -Xshare:dump) then it has -m in the list. I assume that should 
not be there. That is java launcher option, the VM don't see or 
unrecognize it, also it's long form is --module.


-Alan

8072988: Update javax.annotation.processing for modules

[Jan Lahoda]

Hi,

I'd like to discuss enhacements to the Annotation Processing API for 
modules. My current sketch is here (against jigsaw/jake):

http://cr.openjdk.java.net/~jlahoda/8072988/webrev.00/

This patch augments Filer methods with variants that take a "module" 
parameter, allowing to generate classes/resources to a particular module 
when javac is compiling multiple modules. The existing variants of the 
methods try to infer the target module from the given package and 
originating elements (see JavacFiler.checkOrInferModule).


This patch should also include module-infos in 
RoundEnvironment.getRootElements.


This patch does not allow creating new module-infos. My question is if 
that should be supported, and how that should work. There are several 
possible scenarios which we could possibly support:
-creating the module-info while compiling for the unnamed module, 
converting the compilation to a module (single-module) compilation


-creating the module-info while in multi-module mode (possibly compiling 
multiple modules at once), for a directory that exists on the 
modulesourcepath, possibly containing some source files, but no 
module-info.java. The tricky part is how to work with the sources until 
the module-info.java.


-creating whole modules from scratch in multi-module mode (i.e. no part 
of the module would exist, and would be completely created by the 
annotation processor). Could be tricky with incremental compilation.


Any comments on these or on the patch above would be wholeheartedly welcome.

Thanks,
   Jan

Re: Feedback on proposal for #ReflectiveAccessToNonExportedTypes

[mark . reinhold]

2016/7/13 20:27:45 -0700, jason.gre...@redhat.com:
> Thanks for you reply! My thoughts are inline. I apologize in advance for
> the length/verbosity. Also, as a general disclaimer, I realize that you
> are all experts; in many of my arguments, I occaisionally restate certain
> concepts that I know you are all intimately aware of to frame the
> argument. Corrections are, as always, welcome.

No worries -- we'll try, as always, to be gentle!

> On Jul 13, 2016, at 4:47 PM, mark.reinh...@oracle.com wrote:
>> To put what Alex wrote in a somewhat different way, I'd say that the
>> tension here is between explicit configuration (as one finds today in,
>> e.g., the Maven world) and implicit configuration (IoC).
> 
> Just a small nit: IoC can also be explicit, its just that the
> explicitness is decoupled from the module, and controlled by another
> party, allowing for more flexibility in the assembled system.

Sure -- I was just trying to characterize the situation from the
standpoint of the module developer rather than that of the assembler
or deployer or container implementor (or any other role).

>> Both approaches
>> are important.  The former is typical of standalone Java SE applications
>> while the latter is typical of Java EE applications, though the two
>> approaches are often intermixed.
> 
> I agree they are certainly intermixed elements of a system, but I’d also
> argue IoC is pervasive in SE applications as well (e.g. inclusion of 330
> and 250 in SE are examples of a desire for SE usage). I can’t refute that
> it has greater usage in EE, since its part of the spec, and thus
> effectively every EE application.

FYI, JSR 330 (DI annotations) is not in Java SE, though it's certainly
used in Java SE applications in combination with various DI frameworks.

JSR 250 ("common" annotations) specifies 14 annotations, but just five
of them are in Java SE.  They're really only there to support JAX-WS, a
component shared with Java EE.  So far as I know they're not used much
in SE applications except in conjunction with JAX-WS.

> I think a better use case categorization of this problem is static
> linkage vs dynamic invocation. In static linkage an explicit symbol
> mapping resolved by the language itself is ideal as it avoids ambiguity,
> and by definition is static. On the other hand with dynamic invocation
> it’s common for the caller to utilize introspection and discovery as part
> of the natural flow of executing a dynamic call. Resolving ambiguity is
> not an issue in this case, since it is already handled by the caller as
> part of introspection.

This dichotomy of implementation techniques corresponds well to the
developer-point-of-view dichotomy of explicit vs. implicit configuration
which I described earlier.

>> ...
>> 
>> If I understand correctly, your view of the present proposal is that:
>> 
>> (1) It induces too much boilerplate, requiring developers to write
>>`exports dynamic P` for every single package `P` that's subject
>>to reflection by a framework, and
> 
> That’s an accurate summary of this point. ...
> 
>> (2) It weakens encapsulation too much, by making the types in such a
>>package available for reflection at run time by any module in the
>>system.
> 
> Sorry for the confusion, what I was trying to say on this point was a bit
> different. What I was trying to say was:
> 
> (2') It weakens encapsulation by forcing the introduction of exports
>  introducing potential conflicts that break applications.
> 
> As an example, assume I have three modules with classloader-per-module
> isolation (A, B, and Victim)
> 
> - A exports foo, and has a non-exported package “bar"
> 
> - B exports bar
> 
> - Victim has a module-info with requires A; requires B
> 
> Now A decides to use IoC on some of its classes in bar, so it’s
> definition is changed to:
> 
> { exports foo; exports dynamic bar; }
> 
> Since exports dynamic is internally a normal export at runtime, module
> resolution fails when loading Victim, because its now including a
> duplicate package, even though A had no intention of publishing its
> internal bar package for linkage.

Got it.  Thanks for clarifying this -- I agree that it's a problem.

Fortunately I think we can address it simply by revising the semantics of
`exports dynamic p` to omit the package-conflict constraint.  This would
allow split packages to occur more readily at run time, though still
really only in fairly obscure situations involving poorly-written class
loaders.

> Qualified exports could in theory address this problem, but they are
> problematic in a dynamic environment since the module is simply not in a
> position to know all of the various modules which would/could enhance
> it. ...

I completely agree.  In general I think it's inappropriate for the author
of a module to write qualified exports except in some very special cases,
and this is most definitely not one of them.

>> These observations lead to your suggestion to allow 

Re: Should setAccessible be part of Java or not? (was Re: It's not too late for access control)

[John Rose]

Thanks, Andrew, for a good summary of the practicalities behind
access overrides.

On Jul 18, 2016, at 2:34 AM, Andrew Haley  wrote:
> 
> The wider the access, the more scope for complex interfaces.
> 
> Also, making a method or field globally accessible may mean that the
> author of the code can no longer guarantee its correctness.
> 
> … we can reduce risk and the size of attack surfaces

I especially agree with these parts, from bitter experience.  When you
widen an interface, you add attack surface.  If you widen an interface
with clear documentation and buy-in from the author of the interface,
and can add both negative and positive tests for the new API surface,
then you can sleep at night.  Meanwhile, the best attack surfaces are
implicitly defined (e.g., privates which aren't really private after all)
and are activated by remote control (e.g., setA), without the buy-in
of the library author.

I'm not saying this to condemn frameworks which perform polymorphic,
pattern-driven access overrides, but to explain their special risks.  And
I think in the future we will have a much more explicit and robust set
of rules for opt-in and opt-out into such frameworks, including optioning
which is delegated to third parties, yet is still limited to a small subset
of what is available to root privileges (aka. setA).

Finally, as JIT guy, I'd like to note that one way to crisply evaluate
the "weight" of an API surface given away by some "fancy feature"
(ubiquitous override-ables, dynamic loading, monitor-per-object, AOP,
access overrides, …) is to see how many optimizations it makes difficult
or impossible.  Yes, Java is wonderfully plastic, and yes, we need to
supply hardening transformations so the plastic can be made durable.

— John