Re: Review Request JDK-8169925: Organize licenses by module in source, JMOD file, and run-time image

2016-12-09 Thread Mandy Chung
> On Dec 9, 2016, at 11:51 PM, Erik Joelsson  wrote:
> 
> Hello Mandy,
> 
> In Copy-java.base.gmk. Modules.gmk is no longer needed. JMOD_* varaibles are 
> no longer used.
> 

Good catch.

> In Main.gmk, it would be good with a comment explaining why the jmods need 
> java.base-copy. Something like:
> 
> # All modules include the main license files from java.base.
> 
> Otherwise I'm ok with the build changes now.

Updated per your comment:

$ hg diff make/copy/Copy-java.base.gmk 
diff --git a/make/copy/Copy-java.base.gmk b/make/copy/Copy-java.base.gmk
--- a/make/copy/Copy-java.base.gmk
+++ b/make/copy/Copy-java.base.gmk
@@ -233,3 +233,17 @@
 endif
 
 

+
+# JDK license and assembly exception files to be packaged in JMOD
+
+JDK_LICENSE ?= $(JDK_TOPDIR)/LICENSE
+JDK_NOTICE  ?= $(JDK_TOPDIR)/ASSEMBLY_EXCEPTION
+
+$(eval $(call SetupCopyFiles, COPY_JDK_NOTICES, \
+FILES := $(JDK_LICENSE) $(JDK_NOTICE), \
+DEST := $(LEGAL_DST_DIR), \
+FLATTEN := true, \
+))
+
+TARGETS += $(COPY_JDK_NOTICES)
+


$ hg diff make/Main.gmk 
diff --git a/make/Main.gmk b/make/Main.gmk
--- a/make/Main.gmk
+++ b/make/Main.gmk
@@ -659,6 +659,9 @@
 exploded-image-optimize
   endif
 
+  # All modules include the main license files from java.base.
+  $(JMOD_TARGETS): java.base-copy
+
   zip-security: java.base-java java.security.jgss-java java.security.jgss-libs 
\
   $(filter jdk.crypto%, $(JAVA_TARGETS))
 



Re: Review Request JDK-8169925: Organize licenses by module in source, JMOD file, and run-time image

2016-12-09 Thread Erik Joelsson

Hello Mandy,

In Copy-java.base.gmk. Modules.gmk is no longer needed. JMOD_* varaibles 
are no longer used.


In Main.gmk, it would be good with a comment explaining why the jmods 
need java.base-copy. Something like:


# All modules include the main license files from java.base.

Otherwise I'm ok with the build changes now.

/Erik

On 2016-12-10 00:19, Mandy Chung wrote:

Erik, Magnus,

I have made the change per your suggestion:
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8169925/webrev.01/

Jeannette, Jeff, and I went through all .md files and I think asm.md is the 
only one got the characters messed up.  These files are contributed by 
Jeannette.

Also, per Naoto, cldr.md are now in both java.base and jdk.localedata.

thanks
Mandy




Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Eric Johnson

On 12/9/16 3:21 PM, Uwe Schindler wrote:
> The -Dsun.reflect.debugModuleAccessChecks=true options help to debug, indeed, 
> but it does not solve the underlying issue. Apache Solr/Lucene and 
> Elasticsearch will no longer work with Java 9 unless you require users to add 
> those strange options. Elasticsearch already runs with a SecurityManager by 
> default, so the question is: why is this not handled by a security manager 
> and a new permission like "crossModuleAccess/module/package"? Why must it be 
> done on command line? This makes it impossible to ship something like Lucene 
> that it work out of box together with correct policy files?
I've asked the question multiple times on this list, about what the
threat model analysis is behind this new level of runtime "security".

Alas, no answers so far.

Eric.



Re: Java module graph png settings?

2016-12-09 Thread Mandy Chung
dot -Tpng

Mandy

> On Dec 9, 2016, at 4:58 PM, Patrick Reinhart  wrote:
> 
> Hi Mandy,
> 
> Can you tell me what options with the Graphviz framework are to get the 
> actual PNG file out of the jdk.dot file? I tried to find this, but did not 
> find the actual place.
> 
> -Patrick
> 
> 
>> Am 08.12.2016 um 07:18 schrieb Mandy Chung :
>> 
>> Hi Patrick
>> 
>> Are you looking for:
>> 
>> http://hg.openjdk.java.net/jdk9/dev/jdk/file/c9785b0f04fd/make/src/classes/build/tools/jigsaw/GenGraphs.java
>> 
>> Mandy
>> 
>>> On Dec 7, 2016, at 1:17 PM, Patrick Reinhart  wrote:
>>> 
>>> Can anyone point me to the place, where the java module dependencies are 
>>> created? I would like to do something similar and I curious how it’s being 
>>> done exactly…
>>> 
>>> Thanks very much
>>> 
>>> -Patrick
>> 
> 



Re: RFR 8168925: MODULES property should be topologically ordered and space-separated list

2016-12-09 Thread Mandy Chung

> On Dec 9, 2016, at 7:32 PM, Sundararajan Athijegannathan 
>  wrote:
> 
> Hi,
> 
> Thanks for your review. Comments below..
> 
> On 10/12/16, 2:18 AM, Mandy Chung wrote:
>>> On Dec 9, 2016, at 12:49 AM, Sundararajan 
>>> Athijegannathan  wrote:
>>> 
>>> Please review 
>>> http://cr.openjdk.java.net/~sundar/8168925/webrev.01/index.html for 
>>> https://bugs.openjdk.java.net/browse/JDK-8168925
>>> 
>> Is the order of Plugin.Category enums significant?  You moved COMPRESSOR 
>> down - is it necessary?
> 
> Yes. Without that ReleaseInfoPlugin will receive compressed resources and 
> compressed module-info.class won't be parsed okay by ModuleDescriptor.read 
> (called by ModuleSorter). Note that auto-decompression is done only by 
> LastResourcePool - which is created after all plugins operate. [I kept 
> getting test failures and debugged to find this is the cause of failures!]
> 

OK.  If the order of the enums determines the plugin ordering, it’d be good 
adding a comment.

>> 
>> Can you look at DefaultImageBuilder::releaseProperties which I think this 
>> should be moved to ReleaseInfoPlugin?  The content of `release` should be 
>> written when the new entry "/java.base/release” is added to the resource 
>> pool.  DefaultImageBuilder does not need to add any more properties to this 
>> `release` file as all properties in `release` are known once the graph is 
>> resolved.
> Okay, I'll check if I can move all stuff there. From initial communication 
> [private email], I thought the suggestion was only about MODULES.

This has beeen my suggestion - move the whole thing out from 
DefaultImageBuilder.  I don’t see the reason why it has to be done in two 
separate places.  That’ll be a good clean up.

>> 
>> 
>> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ResourcePoolManager.java
>> - is this change needed?
> 
> Yes. Helped me debugging to find out which module's module-info.class was not 
> parsed fine. Exception translation puts the name of the module in the message 
> - which will be missing in the exception thrown by 
> ModuleDescriptor.read(ByteBuffer).

OK.

> 
>> 
>> test/tools/jlink/CustomPluginTest.java
>>It’s one option to disable the releaseinfo plugin.  An alternative
>>way to fix this test is [2].
> 
> I could. But that test checks for the module dependency checks that happen 
> after all plugins are exercised. That should happen regardless of 
> release-info plugin is enabled. i.e., even when release-info plugin is 
> disabled, that module missing error should be thrown and the test checks for 
> that.
> 

OK.  If I happen to push JDK-8169925 before this, you can revert my change to 
this test.

Thanks
Mandy

hg: jigsaw/jake/jdk: Updates for #VersionedDependences

2016-12-09 Thread alan . bateman
Changeset: 6743bd89af1f
Author:alanb
Date:  2016-12-10 05:22 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/6743bd89af1f

Updates for #VersionedDependences
DEBUG_ADD_OPENS for debugging
Misc. cleanups

! src/java.base/share/classes/com/sun/java/util/jar/pack/PackerImpl.java
! src/java.base/share/classes/com/sun/java/util/jar/pack/intrinsic.properties
! src/java.base/share/classes/java/lang/StackTraceElement.java
! src/java.base/share/classes/java/lang/module/ModuleDescriptor.java
! src/java.base/share/classes/java/lang/module/ModuleFinder.java
! src/java.base/share/classes/java/lang/module/ModuleInfo.java
! src/java.base/share/classes/java/lang/module/ModulePath.java
! src/java.base/share/classes/java/lang/module/ModuleReference.java
! src/java.base/share/classes/java/lang/module/ModuleReferences.java
! src/java.base/share/classes/java/lang/module/Resolver.java
! src/java.base/share/classes/java/lang/module/SystemModuleFinder.java
! src/java.base/share/classes/java/lang/reflect/AccessibleObject.java
! src/java.base/share/classes/java/lang/reflect/Layer.java
! src/java.base/share/classes/java/lang/reflect/Module.java
! 
src/java.base/share/classes/jdk/internal/jimage/decompressor/StringSharingDecompressor.java
! src/java.base/share/classes/jdk/internal/misc/JavaLangModuleAccess.java
! src/java.base/share/classes/jdk/internal/misc/JavaLangReflectModuleAccess.java
! src/java.base/share/classes/jdk/internal/module/Builder.java
! src/java.base/share/classes/jdk/internal/module/Checks.java
! src/java.base/share/classes/jdk/internal/module/ClassFileAttributes.java
! src/java.base/share/classes/jdk/internal/module/ClassFileConstants.java
! src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java
! src/java.base/share/classes/jdk/internal/module/ModuleInfoExtender.java
! src/java.base/share/classes/jdk/internal/module/ModuleInfoWriter.java
! src/java.base/share/classes/jdk/internal/org/objectweb/asm/ClassReader.java
! src/java.base/share/classes/jdk/internal/org/objectweb/asm/ClassWriter.java
! src/java.base/share/classes/jdk/internal/org/objectweb/asm/Item.java
! src/java.base/share/classes/jdk/internal/reflect/Reflection.java
! src/jdk.jartool/share/classes/sun/tools/jar/Main.java
! 
src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SystemModuleDescriptorPlugin.java
! src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java
! test/java/lang/module/ModuleDescriptorTest.java
! test/java/lang/reflect/Module/AnnotationsTest.java
! test/tools/jlink/plugins/SystemModuleDescriptors/SystemModulesTest.java
! test/tools/pack200/pack200-verifier/src/xmlkit/ClassReader.java



hg: jigsaw/jake/langtools: Updates for #VersionedDependences

2016-12-09 Thread alan . bateman
Changeset: 91ebc06d4220
Author:alanb
Date:  2016-12-10 05:20 +
URL:   http://hg.openjdk.java.net/jigsaw/jake/langtools/rev/91ebc06d4220

Updates for #VersionedDependences
Contributed-by: jonathan.gibb...@oracle.com, jan.lah...@oracle.com

! make/tools/crules/MutableFieldsAnalyzer.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Modules.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassFile.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ModuleNameReader.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/main/Option.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties
! src/jdk.compiler/share/classes/com/sun/tools/javac/util/JDK9Wrappers.java
! src/jdk.compiler/share/classes/com/sun/tools/javac/util/Names.java
! src/jdk.jdeps/share/classes/com/sun/tools/classfile/Attribute.java
! src/jdk.jdeps/share/classes/com/sun/tools/classfile/ClassTranslator.java
! src/jdk.jdeps/share/classes/com/sun/tools/classfile/ClassWriter.java
! src/jdk.jdeps/share/classes/com/sun/tools/classfile/ConstantPool.java
! src/jdk.jdeps/share/classes/com/sun/tools/classfile/Dependencies.java
! 
src/jdk.jdeps/share/classes/com/sun/tools/classfile/ModulePackages_attribute.java
- 
src/jdk.jdeps/share/classes/com/sun/tools/classfile/ModuleVersion_attribute.java
! src/jdk.jdeps/share/classes/com/sun/tools/classfile/Module_attribute.java
! src/jdk.jdeps/share/classes/com/sun/tools/classfile/ReferenceFinder.java
! src/jdk.jdeps/share/classes/com/sun/tools/javap/AttributeWriter.java
! src/jdk.jdeps/share/classes/com/sun/tools/javap/ClassWriter.java
! src/jdk.jdeps/share/classes/com/sun/tools/javap/ConstantWriter.java
! src/jdk.jdeps/share/classes/com/sun/tools/jdeprscan/scan/CPSelector.java
! test/lib/annotations/annotations/classfile/ClassfileInspector.java
! test/tools/javac/MethodParameters/AttributeVisitor.java
! test/tools/javac/T8003967/DetectMutableStaticFields.java
! test/tools/javac/classfiles/attributes/Module/ModuleTestBase.java
! test/tools/javac/lambda/ByteCodeTest.java
+ test/tools/javac/modules/ModuleVersion.java
! test/tools/javac/modules/OpenModulesTest.java



Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Alan Bateman

On 09/12/2016 23:06, Stephen Felts wrote:


I would highly recommend running with 
_JAVA_OPTIONS=-Dsun.reflect.debugModuleAccessChecks=true
It will tell you what add-options are required.
One minor downside is that it will produce the warning in cases where the 
software is already correctly handling the exception from setAccessible, so 
there can be noise.
Yes, this is a useful property to debug cases where setAccessible fails 
because the package is not exported or opened to the caller. There is 
more on this in JEP 261.


-Alan.


Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Alan Bateman

On 09/12/2016 22:32, Uwe Schindler wrote:


Hi,

I updated our Jenkins server for the JDK 9 preview testing to use build 148. 
Previously we had build 140 and build 147, which both worked without any 
issues. But after the update the following stuff goes wrong:

(1) Unmapping of direct buffers no longer works, although this API was marked 
as critical because there is no replacement up to now, so code can unmap memory 
mapped files, which is one of the most important things Apache Lucene needs to 
use to access huge random access files while reading the index. Without memory 
mapping, the slowdown for Lucene users will be huge
sun.misc.Cleaner was indeed on the original list of APIs for JEP 260 to 
identify as a "critical internal API". It turned out not to be useful 
because it would have required some way to get the Cleaner in the first 
place. That lead to the "new" hack that is reading the private "cleaner" 
field from DBB and treating it as a Runnable. That hack now breaks 
because setAccessible has changed in jdk-9+148 to align with the JSR 376 
proposal tracked as #AwkwardStrongEncapsulation.


No need to panic though, there is an update to JEP 260 coming soon for 
this specific need. Details TDB but it will probably be a method in 
jdk.unsupported module. It does mean that libraries using the old (or 
"new") hacks will need to change. I hope it will be seen as a reasonable 
compromise for this generally awkward issue.


-Alan


Re: RFR 8168925: MODULES property should be topologically ordered and space-separated list

2016-12-09 Thread Sundararajan Athijegannathan

Hi,

Thanks for your review. Comments below..

On 10/12/16, 2:18 AM, Mandy Chung wrote:

On Dec 9, 2016, at 12:49 AM, Sundararajan 
Athijegannathan  wrote:

Please review http://cr.openjdk.java.net/~sundar/8168925/webrev.01/index.html 
for https://bugs.openjdk.java.net/browse/JDK-8168925


Is the order of Plugin.Category enums significant?  You moved COMPRESSOR down - 
is it necessary?


Yes. Without that ReleaseInfoPlugin will receive compressed resources 
and compressed module-info.class won't be parsed okay by 
ModuleDescriptor.read (called by ModuleSorter). Note that 
auto-decompression is done only by LastResourcePool - which is created 
after all plugins operate. [I kept getting test failures and debugged to 
find this is the cause of failures!]




Can you look at DefaultImageBuilder::releaseProperties which I think this should be 
moved to ReleaseInfoPlugin?  The content of `release` should be written when the new 
entry "/java.base/release” is added to the resource pool.  DefaultImageBuilder 
does not need to add any more properties to this `release` file as all properties in 
`release` are known once the graph is resolved.
Okay, I'll check if I can move all stuff there. From initial 
communication [private email], I thought the suggestion was only about 
MODULES.




  550 String[] arr = mods.split(" ");
I think you will need to remove the double-quotes before doing the split.

Will fix that.


src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ResourcePoolManager.java
- is this change needed?


Yes. Helped me debugging to find out which module's module-info.class 
was not parsed fine. Exception translation puts the name of the module 
in the message - which will be missing in the exception thrown by 
ModuleDescriptor.read(ByteBuffer).




src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ReleaseInfoPlugin.java
  137 release.put("MODULES", "\"" + new ModuleSorter(in.moduleView())
  138 
.sorted().map(ResourcePoolModule::name).collect(Collectors.joining(" "))
  139 + "\"”);

Since there is always at least one module, you can use Collectors.joining(“ “, "\””, 
"\””) instead of the prepending and appending.


Will fix that.



test/tools/jlink/CustomPluginTest.java
It’s one option to disable the releaseinfo plugin.  An alternative
way to fix this test is [2].


I could. But that test checks for the module dependency checks that 
happen after all plugins are exercised. That should happen regardless of 
release-info plugin is enabled. i.e., even when release-info plugin is 
disabled, that module missing error should be thrown and the test checks 
for that.



test/tools/jlink/ModuleNamesOrderTest.java

  57 String moduleName = "bug8168925”;

Is it the output dir name?  This is not a module name.  @bug has the bug id.  I 
suggest to rename this to “image” or some other name.


Will fix it.


One suggestion to the validation is to remove the double quotes from MODULES 
property value and split it into an array.  That may be a better way to verify 
the dependences.

Hmm.. I'll look into that.

Thanks,
-Sundar



PS. Mandy Chung wrote ModuleSorter for another fix (yet to be pushed).  I'm 
using it for this fix after discussion with her (private email).

ModuleSorter is part of the patch for JDK-8169925[1].  I will take it out if 
Sundar pushes this changeset before JDK-8169925.

Mandy
[1]
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2016-December/010416.html
[2] 
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8169925/webrev.00/jdk/test/tools/jlink/CustomPluginTest.java.udiff.html


Re: Java module graph png settings?

2016-12-09 Thread Patrick Reinhart
Hi Mandy,

Can you tell me what options with the Graphviz framework are to get the actual 
PNG file out of the jdk.dot file? I tried to find this, but did not find the 
actual place.

-Patrick


> Am 08.12.2016 um 07:18 schrieb Mandy Chung :
> 
> Hi Patrick
> 
> Are you looking for:
> 
> http://hg.openjdk.java.net/jdk9/dev/jdk/file/c9785b0f04fd/make/src/classes/build/tools/jigsaw/GenGraphs.java
> 
> Mandy
> 
>> On Dec 7, 2016, at 1:17 PM, Patrick Reinhart  wrote:
>> 
>> Can anyone point me to the place, where the java module dependencies are 
>> created? I would like to do something similar and I curious how it’s being 
>> done exactly…
>> 
>> Thanks very much
>> 
>> -Patrick
> 



RE: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Stephen Felts
A related problem is that opening these packages to satisfy gradle/grooy can 
mask other non-gradle problems with the same packages.  
I isolated these options in our shell that invokes Gradle, although that still 
isn't perfect since we run lots of Java programs during the build.


-Original Message-
From: Kevin Rushforth 
Sent: Friday, December 09, 2016 6:33 PM
To: Stephen Felts
Cc: Uwe Schindler; jigsaw-dev@openjdk.java.net; Core-Libs-Dev
Subject: Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

I second the recommendation of using
"-Dsun.reflect.debugModuleAccessChecks=true". We use gradle to build JavaFX and 
I ended up needing the following to allow our build to run with jdk-9+148:

export _JAVA_OPTIONS="-Dsun.reflect.debugModuleAccessChecks=true
--add-opens=java.base/java.lang=ALL-UNNAMED
--add-opens=java.base/java.util=ALL-UNNAMED
--add-opens=java.base/java.lang.invoke=ALL-UNNAMED
--add-opens=java.base/java.io=ALL-UNNAMED
--add-opens=java.base/java.util.concurrent=ALL-UNNAMED
--add-opens=java.base/java.text=ALL-UNNAMED"

-- Kevin


Stephen Felts wrote:
> I would highly recommend running with 
> _JAVA_OPTIONS=-Dsun.reflect.debugModuleAccessChecks=true
> It will tell you what add-options are required.
> One minor downside is that it will produce the warning in cases where the 
> software is already correctly handling the exception from setAccessible, so 
> there can be noise.
>   


RE: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Stephen Felts
Unsafe is visible in JDK9.  See the list at http://openjdk.java.net/jeps/260

I agree that requiring command line options is a problem.
The experts don't want to merge module access into the security manager.

The link above says "Suggested additions to this list, justified by real-world 
use cases and estimates of developer and end-user impact, are welcome".  So you 
should make clear exactly API's that you want exposed.


-Original Message-
From: Uwe Schindler [mailto:uschind...@apache.org] 
Sent: Friday, December 09, 2016 6:22 PM
To: Stephen Felts; jigsaw-dev@openjdk.java.net; Core-Libs-Dev
Subject: RE: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

Hi,

Thanks for the hints to fix Groovy, although this is hard to do with ANT (which 
is our build system).

The -Dsun.reflect.debugModuleAccessChecks=true options help to debug, indeed, 
but it does not solve the underlying issue. Apache Solr/Lucene and 
Elasticsearch will no longer work with Java 9 unless you require users to add 
those strange options. Elasticsearch already runs with a SecurityManager by 
default, so the question is: why is this not handled by a security manager and 
a new permission like "crossModuleAccess/module/package"? Why must it be done 
on command line? This makes it impossible to ship something like Lucene that it 
work out of box together with correct policy files?

And as said in my previous mail: The direct bytebuffer unmapping has still no 
"official" way to do it, but it is critical to large scale database systems 
like Lucene/Solr/Elasticsearch. You have replacements in Java 9 for Unsafe 
(VarHandles,...), but still no way to allow unmapping of byte buffers that sit 
on huge resources or disallow deleting of files on windows. It was discussed on 
last FOSDEM to do something in Java 10 (I would like to get information how to 
propose the required change as Java 10 dev started now!), and in the meantime 
it was confirmed that some APIs in the JDK are "critical" and will be 
supported. But this is now 

So please re-add the special critical APIs back to the whitelist, so code like 
getting (legacy) Unsafe or unmapping direct buffers works without command line 
parameters that confuse people.

Uwe

-
Uwe Schindler
uschind...@apache.org
ASF Member, Apache Lucene PMC / Committer Bremen, Germany 
http://lucene.apache.org/
> -Original Message-
> From: Stephen Felts [mailto:stephen.fe...@oracle.com]
> Sent: Saturday, December 10, 2016 12:07 AM
> To: Uwe Schindler ; 
> jigsaw-dev@openjdk.java.net; Core-Libs-Dev 
> 
> Subject: RE: Java 9 build 148 causes trouble in Apache 
> Lucene/Solr/Elasticsearch
> 
> I would highly recommend running with _JAVA_OPTIONS=- 
> Dsun.reflect.debugModuleAccessChecks=true
> It will tell you what add-options are required.
> One minor downside is that it will produce the warning in cases where 
> the software is already correctly handling the exception from 
> setAccessible, so there can be noise.



Re: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Kevin Rushforth
I second the recommendation of using 
"-Dsun.reflect.debugModuleAccessChecks=true". We use gradle to build 
JavaFX and I ended up needing the following to allow our build to run 
with jdk-9+148:


export _JAVA_OPTIONS="-Dsun.reflect.debugModuleAccessChecks=true 
--add-opens=java.base/java.lang=ALL-UNNAMED 
--add-opens=java.base/java.util=ALL-UNNAMED 
--add-opens=java.base/java.lang.invoke=ALL-UNNAMED 
--add-opens=java.base/java.io=ALL-UNNAMED 
--add-opens=java.base/java.util.concurrent=ALL-UNNAMED 
--add-opens=java.base/java.text=ALL-UNNAMED"


-- Kevin


Stephen Felts wrote:

I would highly recommend running with 
_JAVA_OPTIONS=-Dsun.reflect.debugModuleAccessChecks=true
It will tell you what add-options are required.
One minor downside is that it will produce the warning in cases where the 
software is already correctly handling the exception from setAccessible, so 
there can be noise.
  


RE: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Uwe Schindler
Hi,

Thanks for the hints to fix Groovy, although this is hard to do with ANT (which 
is our build system).

The -Dsun.reflect.debugModuleAccessChecks=true options help to debug, indeed, 
but it does not solve the underlying issue. Apache Solr/Lucene and 
Elasticsearch will no longer work with Java 9 unless you require users to add 
those strange options. Elasticsearch already runs with a SecurityManager by 
default, so the question is: why is this not handled by a security manager and 
a new permission like "crossModuleAccess/module/package"? Why must it be done 
on command line? This makes it impossible to ship something like Lucene that it 
work out of box together with correct policy files?

And as said in my previous mail: The direct bytebuffer unmapping has still no 
"official" way to do it, but it is critical to large scale database systems 
like Lucene/Solr/Elasticsearch. You have replacements in Java 9 for Unsafe 
(VarHandles,...), but still no way to allow unmapping of byte buffers that sit 
on huge resources or disallow deleting of files on windows. It was discussed on 
last FOSDEM to do something in Java 10 (I would like to get information how to 
propose the required change as Java 10 dev started now!), and in the meantime 
it was confirmed that some APIs in the JDK are "critical" and will be 
supported. But this is now 

So please re-add the special critical APIs back to the whitelist, so code like 
getting (legacy) Unsafe or unmapping direct buffers works without command line 
parameters that confuse people.

Uwe

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/
> -Original Message-
> From: Stephen Felts [mailto:stephen.fe...@oracle.com]
> Sent: Saturday, December 10, 2016 12:07 AM
> To: Uwe Schindler ; jigsaw-dev@openjdk.java.net;
> Core-Libs-Dev 
> Subject: RE: Java 9 build 148 causes trouble in Apache
> Lucene/Solr/Elasticsearch
> 
> I would highly recommend running with _JAVA_OPTIONS=-
> Dsun.reflect.debugModuleAccessChecks=true
> It will tell you what add-options are required.
> One minor downside is that it will produce the warning in cases where the
> software is already correctly handling the exception from setAccessible, so
> there can be noise.



Re: Review Request JDK-8169925: Organize licenses by module in source, JMOD file, and run-time image

2016-12-09 Thread Mandy Chung
Erik, Magnus,

I have made the change per your suggestion:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8169925/webrev.01/

Jeannette, Jeff, and I went through all .md files and I think asm.md is the 
only one got the characters messed up.  These files are contributed by 
Jeannette.

Also, per Naoto, cldr.md are now in both java.base and jdk.localedata.

thanks
Mandy

RE: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Stephen Felts
I would highly recommend running with 
_JAVA_OPTIONS=-Dsun.reflect.debugModuleAccessChecks=true
It will tell you what add-options are required.
One minor downside is that it will produce the warning in cases where the 
software is already correctly handling the exception from setAccessible, so 
there can be noise.


RE: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Stephen Felts
Gradle/groovy are known to have problems with the restricted module access of 
b148.

 

You can get around this specific problem by using the environment variable 
_JAVA_OPTIONS=--add-opens=java.base/java.lang=ALL-UNNAMED

 

The packages that you need to open depend on what Java methods your 
gradle/groovy files happen to step on.

In my case, I need to use the following for our gradle build.

--add-opens=java.base/java.io=ALL-UNNAMED

--add-opens=java.base/java.lang=ALL-UNNAMED

--add-opens=java.base/java.lang.invoke=ALL-UNNAMED

--add-opens=java.base/java.lang.reflect=ALL-UNNAMED

--add-opens=java.base/java.net=ALL-UNNAMED

--add-opens=java.base/java.text=ALL-UNNAMED

--add-opens=java.base/java.util=ALL-UNNAMED

--add-opens=java.base/java.util.concurrent=ALL-UNNAMED

--add-opens=java.base/java.util.regex=ALL-UNNAMED

--add-opens=java.base/java.util.jar=ALL-UNNAMED

--add-opens=java.base/java.util.zip=ALL-UNNAMED

--add-opens=java.base/javax.net=ALL-UNNAMED

--add-opens=java.base/sun.net.www=ALL-UNNAMED

--add-opens=java.base/sun.net.www.protocol.file=ALL-UNNAMED

--add-opens=java.base/sun.nio.fs=ALL-UNNAMED

--add-opens=java.xml/javax.xml.transform=ALL-UNNAMED

--add-opens=java.xml/javax.xml.transform.stream=ALL-UNNAMED

--add-opens=java.xml/com.sun.org.apache.xalan.internal.xsltc=ALL-UNNAMED

--add-opens=java.xml/com.sun.org.apache.xalan.internal.xsltc.compiler=ALL-UNNAMED

--add-opens=java.xml/com.sun.org.apache.xalan.internal.xsltc.trax=ALL-UNNAMED

 

-Original Message-
From: Uwe Schindler [mailto:uschind...@apache.org] 
Sent: Friday, December 09, 2016 5:33 PM
To: jigsaw-dev@openjdk.java.net; Core-Libs-Dev
Subject: Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

 

Hi,

 

I updated our Jenkins server for the JDK 9 preview testing to use build 148. 
Previously we had build 140 and build 147, which both worked without any 
issues. But after the update the following stuff goes wrong:

 

(1) Unmapping of direct buffers no longer works, although this API was marked 
as critical because there is no replacement up to now, so code can unmap memory 
mapped files, which is one of the most important things Apache Lucene needs to 
use to access huge random access files while reading the index. Without memory 
mapping, the slowdown for Lucene users will be huge

 

This is caused by the recent Jigsaw changes, published in build 148. 
Unfortunately we did not test the Jigsaw builds, so we would have noticed that 
earlier. Basically the following peace of code fails now (with or without 
doPrivileged and with/without security manager):

 

  final Class directBufferClass = 
Class.forName("java.nio.DirectByteBuffer");

  

  final Method m = directBufferClass.getMethod("cleaner");

  m.setAccessible(true);

  MethodHandle directBufferCleanerMethod = lookup.unreflect(m);

  Class cleanerClass = directBufferCleanerMethod.type().returnType();

  // build method handle for unmapping, full code is here: 
https://goo.gl/TfQWl6

 

>From my understanding previous lengthy discussions on OpenJDKs mailing lists 
>(and a discussion on last FOSDEM), we agreed, that this unmapping code will 
>still be supported with Java 9 (it was already adapted to work with Java 8 and 
>Java 9 in Lucene, see the full code at https://goo.gl/TfQWl6) and Java 10 
>might have a new "official" unmapping API. On FOSDEM we already had some 
>discussions how to implement this (with Andrew Haley and Mark Reinholds). We 
>would have a strong interest to start some proposal about this (JEP or 
>similar, but how to proceed). The idea was to allow unmapping in some special 
>Hotspot state and trigger a safepoint, keywords in the discussion were 
>"volatile only on safepoints".

 

So how to proceed with this. I know we can fix the issue with "--add-opens 
java.base/java.nio=ALL-UNNAMED", but for a product with huge impact like 
Elasticsearch or Solr, this is not a good way to go, especially as it will not 
work out of box anymore. From reading the module documentation, I had the 
feeling that there were some plans to support those "critical APIs" in the 
module definition (e.g., add exclusion from the checks for 
java.nio.DirectByteBuffer). An alternative is of course access to the (public) 
interface in the internal class sun.nio.ch.DirectBuffer, but we avoided this in 
our code, because it is also likely to break (I have not yet tried, if you have 
it declared as critical API).

 

I just refer to the following issues that explicitely added APIs to allow 
unmapping (jdk.internal.Cleaner implements Runnable,...):

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

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

http://cr.openjdk.java.net/~chegar/8148117/src/java.base/share/classes/jdk/internal/ref/Cleaner.java.udiff.html

 

(2) A second thing  we noticed is that Groovy no longer works and dies with 
strange error messages. This does not affect Lucene's program code, but breaks 
our build 

Java 9 build 148 causes trouble in Apache Lucene/Solr/Elasticsearch

2016-12-09 Thread Uwe Schindler
Hi,

I updated our Jenkins server for the JDK 9 preview testing to use build 148. 
Previously we had build 140 and build 147, which both worked without any 
issues. But after the update the following stuff goes wrong:

(1) Unmapping of direct buffers no longer works, although this API was marked 
as critical because there is no replacement up to now, so code can unmap memory 
mapped files, which is one of the most important things Apache Lucene needs to 
use to access huge random access files while reading the index. Without memory 
mapping, the slowdown for Lucene users will be huge

This is caused by the recent Jigsaw changes, published in build 148. 
Unfortunately we did not test the Jigsaw builds, so we would have noticed that 
earlier. Basically the following peace of code fails now (with or without 
doPrivileged and with/without security manager):

  final Class directBufferClass = 
Class.forName("java.nio.DirectByteBuffer");
  
  final Method m = directBufferClass.getMethod("cleaner");
  m.setAccessible(true);
  MethodHandle directBufferCleanerMethod = lookup.unreflect(m);
  Class cleanerClass = directBufferCleanerMethod.type().returnType();
  // build method handle for unmapping, full code is here: 
https://goo.gl/TfQWl6

>From my understanding previous lengthy discussions on OpenJDKs mailing lists 
>(and a discussion on last FOSDEM), we agreed, that this unmapping code will 
>still be supported with Java 9 (it was already adapted to work with Java 8 and 
>Java 9 in Lucene, see the full code at https://goo.gl/TfQWl6) and Java 10 
>might have a new "official" unmapping API. On FOSDEM we already had some 
>discussions how to implement this (with Andrew Haley and Mark Reinholds). We 
>would have a strong interest to start some proposal about this (JEP or 
>similar, but how to proceed). The idea was to allow unmapping in some special 
>Hotspot state and trigger a safepoint, keywords in the discussion were 
>"volatile only on safepoints".

So how to proceed with this. I know we can fix the issue with "--add-opens 
java.base/java.nio=ALL-UNNAMED", but for a product with huge impact like 
Elasticsearch or Solr, this is not a good way to go, especially as it will not 
work out of box anymore. From reading the module documentation, I had the 
feeling that there were some plans to support those "critical APIs" in the 
module definition (e.g., add exclusion from the checks for 
java.nio.DirectByteBuffer). An alternative is of course access to the (public) 
interface in the internal class sun.nio.ch.DirectBuffer, but we avoided this in 
our code, because it is also likely to break (I have not yet tried, if you have 
it declared as critical API).

I just refer to the following issues that explicitely added APIs to allow 
unmapping (jdk.internal.Cleaner implements Runnable,...):
https://bugs.openjdk.java.net/browse/JDK-8148117
https://bugs.openjdk.java.net/browse/JDK-8132928
http://cr.openjdk.java.net/~chegar/8148117/src/java.base/share/classes/jdk/internal/ref/Cleaner.java.udiff.html

(2) A second thing  we noticed is that Groovy no longer works and dies with 
strange error messages. This does not affect Lucene's program code, but breaks 
our build system (Ant-based but with some scripts inside). Also Elasticsearch's 
usage of Gradle is affected. 

We see this:
groovy.lang.MissingMethodException: No signature of method: static 
java.lang.Integer.valueOf() is applicable for argument types: 
(java.lang.String) values: [54]
Possible solutions: plus(java.lang.String)
at 
groovy.lang.MetaClassImpl.invokeStaticMissingMethod(MetaClassImpl.java:1500)
at groovy.lang.MetaClassImpl.invokeStaticMethod(MetaClassImpl.java:1486)
at 
org.codehaus.groovy.runtime.callsite.StaticMetaClassSite.call(StaticMetaClassSite.java:53)
at 
org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
at 
org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
at 
org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:125)
at 
embedded_script_in_C__Users_Uwe_Schindler_Projects_lucene_trunk_lusolr1_lucene_common_build_dot_xml$_run_closure1.doCall(embedded_script_in_C__Users_Uwe_Schindler_Projects_lucene_trunk_lusolr1_lucene_common_build_dot_xml:7)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at 
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at 
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:538)
at 
org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
at 

Re: RFR 8168925: MODULES property should be topologically ordered and space-separated list

2016-12-09 Thread Mandy Chung

> On Dec 9, 2016, at 12:49 AM, Sundararajan Athijegannathan 
>  wrote:
> 
> Please review http://cr.openjdk.java.net/~sundar/8168925/webrev.01/index.html 
> for https://bugs.openjdk.java.net/browse/JDK-8168925
> 

Is the order of Plugin.Category enums significant?  You moved COMPRESSOR down - 
is it necessary?

Can you look at DefaultImageBuilder::releaseProperties which I think this 
should be moved to ReleaseInfoPlugin?  The content of `release` should be 
written when the new entry "/java.base/release” is added to the resource pool.  
DefaultImageBuilder does not need to add any more properties to this `release` 
file as all properties in `release` are known once the graph is resolved.

 550 String[] arr = mods.split(" ");
I think you will need to remove the double-quotes before doing the split.

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/ResourcePoolManager.java
- is this change needed?

src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/ReleaseInfoPlugin.java
 137 release.put("MODULES", "\"" + new ModuleSorter(in.moduleView())
 138 
.sorted().map(ResourcePoolModule::name).collect(Collectors.joining(" "))
 139 + "\"”);

Since there is always at least one module, you can use Collectors.joining(“ “, 
"\””, "\””) instead of the prepending and appending.

test/tools/jlink/CustomPluginTest.java
   It’s one option to disable the releaseinfo plugin.  An alternative
   way to fix this test is [2].

test/tools/jlink/ModuleNamesOrderTest.java

  57 String moduleName = "bug8168925”;

Is it the output dir name?  This is not a module name.  @bug has the bug id.  I 
suggest to rename this to “image” or some other name.

One suggestion to the validation is to remove the double quotes from MODULES 
property value and split it into an array.  That may be a better way to verify 
the dependences.


> PS. Mandy Chung wrote ModuleSorter for another fix (yet to be pushed).  I'm 
> using it for this fix after discussion with her (private email).

ModuleSorter is part of the patch for JDK-8169925[1].  I will take it out if 
Sundar pushes this changeset before JDK-8169925.

Mandy
[1] 
http://mail.openjdk.java.net/pipermail/jigsaw-dev/2016-December/010416.html
[2] 
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8169925/webrev.00/jdk/test/tools/jlink/CustomPluginTest.java.udiff.html

Re: RFR [9] 8170859 : Run time and tool support for ModuleResolution

2016-12-09 Thread Alan Bateman

On 09/12/2016 14:09, Remi Forax wrote:


Thanks Alan,
so I withdraw my snarky comment.
Not a problem. There is cleanup to be done in several areas, this is 
one. For the short term then we need to get the implementation aligned 
with the proposals in the JSR and then get the cleanup done before ZBB.


-Alan


Re: RFR [9] 8170859 : Run time and tool support for ModuleResolution

2016-12-09 Thread Chris Hegarty

I'll work with Alan to get these changes re-based on top of
the refactoring in jake. Consider this review request withdrawn.

-Chris.

On 09/12/16 14:09, Remi Forax wrote:

Thanks Alan,
so I withdraw my snarky comment.

Rémi


On December 9, 2016 12:25:19 PM GMT+01:00, Alan Bateman 
 wrote:

On 08/12/2016 14:41, Remi Forax wrote:


Chris,
this patch is rather ugly.
Patching the ModuleDescriptor is not a good idea, maybe it's means

that we need a system of metadata associated with a ModuleDescriptor,

the hash of a module (which is not part of the module spec but the

OpenJDK implementation) can be also considered as metadata.



There is refactoring in flight that drops the JDK-specific meta data
from ModuleDescriptor. Once all the pieces are merged together then it
should look a lot better.

-Alan.




Re: RFR [9] 8170859 : Run time and tool support for ModuleResolution

2016-12-09 Thread Remi Forax
Thanks Alan,
so I withdraw my snarky comment.

Rémi


On December 9, 2016 12:25:19 PM GMT+01:00, Alan Bateman 
 wrote:
>On 08/12/2016 14:41, Remi Forax wrote:
>
>> Chris,
>> this patch is rather ugly.
>> Patching the ModuleDescriptor is not a good idea, maybe it's means
>that we need a system of metadata associated with a ModuleDescriptor,
>> the hash of a module (which is not part of the module spec but the
>OpenJDK implementation) can be also considered as metadata.
>>
>There is refactoring in flight that drops the JDK-specific meta data 
>from ModuleDescriptor. Once all the pieces are merged together then it 
>should look a lot better.
>
>-Alan.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


Re: RFR 8168925: MODULES property should be topologically ordered and space-separated list

2016-12-09 Thread Jim Laskey (Oracle)
+1

> On Dec 9, 2016, at 4:49 AM, Sundararajan Athijegannathan 
>  wrote:
> 
> Please review http://cr.openjdk.java.net/~sundar/8168925/webrev.01/index.html 
> for https://bugs.openjdk.java.net/browse/JDK-8168925
> 
> PS. Mandy Chung wrote ModuleSorter for another fix (yet to be pushed).  I'm 
> using it for this fix after discussion with her (private email).
> 
> Thanks,
> -Sundar



Re: RFR [9] 8170859 : Run time and tool support for ModuleResolution

2016-12-09 Thread Alan Bateman

On 08/12/2016 14:41, Remi Forax wrote:


Chris,
this patch is rather ugly.
Patching the ModuleDescriptor is not a good idea, maybe it's means that we need 
a system of metadata associated with a ModuleDescriptor,
the hash of a module (which is not part of the module spec but the OpenJDK 
implementation) can be also considered as metadata.

There is refactoring in flight that drops the JDK-specific meta data 
from ModuleDescriptor. Once all the pieces are merged together then it 
should look a lot better.


-Alan.


RFR 8168925: MODULES property should be topologically ordered and space-separated list

2016-12-09 Thread Sundararajan Athijegannathan
Please review 
http://cr.openjdk.java.net/~sundar/8168925/webrev.01/index.html for 
https://bugs.openjdk.java.net/browse/JDK-8168925


PS. Mandy Chung wrote ModuleSorter for another fix (yet to be pushed).  
I'm using it for this fix after discussion with her (private email).


Thanks,
-Sundar