RFR(S) : 8180399 : move jdk.testlibrary.LockFreeLogManager to the top level test library

2017-05-16 Thread Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev//8180399/webrev.00/index.html
> 186 lines changed: 87 ins; 90 del; 9 mod; 

Hi all,

could you please review this small patch which moves LockFreeLogManager class 
from jdk test library to the top level test library?
this fix is a part of ongoing effort on merging and cleaning up our test 
libraries[1].

webrev: http://cr.openjdk.java.net/~iignatyev//8180397/webrev.00/index.html
JBS: https://bugs.openjdk.java.net/browse/JDK-8180397
testing: affected tests

[1]  https://bugs.openjdk.java.net/browse/JDK-8075327

Thanks,
-- Igor




Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread David Holmes

On 17/05/2017 6:21 AM, Martin Buchholz wrote:

On Tue, May 16, 2017 at 12:29 PM, Chris Hegarty 
wrote:



Re-reading/refreshing the env variables has come up before, in other
contexts. As David pointed out [1], re-reading introduces no more risk than
is already there ( if specified correctly ).



Adding a Java API for re-reading would be really weird because there would
be no Java API for writing


I don't see that as weird. We don't want an API for writing because 
writing from Java is a bad idea. The API for re-reading is a concession 
that in reality the process environment can change outside of the JVMs 
control. I view it as part of the process meta-data, for which we only 
provide read APIs in Java.



and there's no way on a Unix system to write at
all without introducing a risk of SEGV.


But the reality is that it is a technique that people use and have used 
for years without harm. Are they doing the "wrong thing"? - probably. 
But I have some sympathy to Gradle's predicament given:


a) Java never documented that getenv took an immutable snapshot
b) The env that is read depends on when the first call to getenv occurs 
(there is an 8u bug where an app suddenly started failing because JDK 
code started using getenv internally and so took the snapshot much earlier!)
c) this has only stopped working because of Jigsaw's strong 
encapsulation enforcement


As I said previously any such API must come with a very strong caveat 
that it is a "use at own risk" API. I know that doesn't sound very 
Java-ish, but getenv by its nature is not very Java-ish IMHO.


David
-




-Chris.

[1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-
May/047668.html


Re: JDK 9 doc-only RFR of 8180353: FileOutputStream documentation does not indicate properly whether files get truncated or not

2017-05-16 Thread Brian Burkhalter
Hi Chris,

Let’s skip the first revision (May 16, 2017, at 12:54 PM PDT) of the patch and 
go for this one instead:

--- a/src/java.base/share/classes/java/io/FileOutputStream.java
+++ b/src/java.base/share/classes/java/io/FileOutputStream.java
@@ -91,6 +91,10 @@
  * If the file exists but is a directory rather than a regular file, does
  * not exist but cannot be created, or cannot be opened for any other
  * reason then a FileNotFoundException is thrown.
+ * 
+ * @implSpec Invoking this constructor with the parameter {@code name} is
+ * equivalent to invoking {@link #FileOutputStream(String,boolean)
+ * new FileOutputStream(name, false)}.
  *
  * @param  name   the system-dependent filename
  * @exception  FileNotFoundException  if the file exists but is a directory

Thanks,

Brian

On May 16, 2017, at 12:54 PM, Brian Burkhalter  
wrote:

> Hi Chris,
> 
> Thanks for the review. Here is a revised version, thanks to a comment from 
> Daniel, which I think might be better:
> 
> Thanks,
> 
> Brian
> 
> --- a/src/java.base/share/classes/java/io/FileOutputStream.java
> +++ b/src/java.base/share/classes/java/io/FileOutputStream.java
> @@ -91,6 +91,10 @@
>  * If the file exists but is a directory rather than a regular file, does
>  * not exist but cannot be created, or cannot be opened for any other
>  * reason then a FileNotFoundException is thrown.
> + * 
> + * Invoking this constructor with the parameter {@code name} is 
> equivalent
> + * to invoking {@link #FileOutputStream(String,boolean)
> + * new FileOutputStream(name, false)}.
>  *
>  * @param  name   the system-dependent filename
>  * @exception  FileNotFoundException  if the file exists but is a 
> directory
> 
> On May 16, 2017, at 1:05 AM, Chris Hegarty  wrote:
> 
>> Looks good Brian.
>> 
>> -Chris.
>> 
>>> On 16 May 2017, at 02:27, Brian Burkhalter  
>>> wrote:
>>> 
>>> Please review at your convenience.
>>> 
>>> Issue:  https://bugs.openjdk.java.net/browse/JDK-8180353
>>> Patch:  [1]
>>> 
>>> Thanks,
>>> 
>>> Brian
>>> 
>>> [1] Hg diff
>>> 
>>> --- a/src/java.base/share/classes/java/io/FileOutputStream.java
>>> +++ b/src/java.base/share/classes/java/io/FileOutputStream.java
>>> @@ -91,6 +91,12 @@
>>>* If the file exists but is a directory rather than a regular file, does
>>>* not exist but cannot be created, or cannot be opened for any other
>>>* reason then a FileNotFoundException is thrown.
>>> + * 
>>> + * Invoking this constructor with the parameter {@code name} is 
>>> equivalent
>>> + * to invoking the constructor {@link #FileOutputStream(String,boolean)
>>> + * FileOutputStream(name,append)} with the same {@code String} 
>>> parameter
>>> + * {@code name} and the {@code boolean} parameter {@code append} equal 
>>> to
>>> + * {@code false}.
>>>*
>>>* @param  name   the system-dependent filename
>>>* @exception  FileNotFoundException  if the file exists but is a 
>>> directory
>> 
> 



Re: RFR 8u Backport: 8179515: Class java.util.concurrent.ThreadLocalRandom fails to Initialize when using SecurityManager

2017-05-16 Thread David Holmes

Thanks Roger!

David

On 17/05/2017 2:54 AM, Roger Riggs wrote:

Hi David,

Looks fine.

Roger


On 5/16/2017 3:02 AM, David Holmes wrote:

The existing code in 8u was a little different to 9, but the new code
is identical (other than package names):

webrev: http://cr.openjdk.java.net/~dholmes/8179515/webrev.8u/
Bug: https://bugs.openjdk.java.net/browse/JDK-8179515
9 changeset: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/bb4cdc198dc0
9 review thread:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-May/047615.html

Thanks,
David




RFR(S) : 8180397 : remove jdk.testlibrary.IOUtils

2017-05-16 Thread Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev//8180397/webrev.00/index.html
> 92 lines changed: 0 ins; 81 del; 11 mod; 

Hi all,

could you please review this small clean-up which removes 
jdk.testlibrary.IOUtils class from test library?
IOUtils provides one method readFully(InputStream), which can be replaced by 
InputStream::readAllBytes() introduced in JDK 9.

this fix is a part of ongoing effort on merging and cleaning up our test 
libraries[1].

webrev: http://cr.openjdk.java.net/~iignatyev//8180397/webrev.00/index.html
JBS: https://bugs.openjdk.java.net/browse/JDK-8180397
testing: affected tests

[1]  https://bugs.openjdk.java.net/browse/JDK-8075327

Thanks,
-- Igor




RFR(XS): move FilterClassLoader and ParentLastURLClassLoader to top level testlibrary

2017-05-16 Thread Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev//8180395/webrev.00/index.html
> 200 lines changed: 101 ins; 99 del; 0 mod;

Hi all,

could you please review this small fix which move FilterClassLoader and 
ParentLastURLClassLoader class from jdk testlibrary to top level testlibrary?
this fix is a part of ongoing effort on merging and cleaning up our test 
libraries[1].

webrev: http://cr.openjdk.java.net/~iignatyev//8180395/webrev.00/index.html
JBS: https://bugs.openjdk.java.net/browse/JDK-8180395

[1]  https://bugs.openjdk.java.net/browse/JDK-8075327

Thanks,
-- Igor

RFR(S): 8180391: move SerializationUtils to top level testlibrary

2017-05-16 Thread Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev//8180391/webrev.00/index.html
> 119 lines changed: 56 ins; 54 del; 9 mod;

Hi all,

could you please review this small fix which move SerializationUtils class from 
jdk testlibrary to top level testlibrary? jdk.testlibrary.SerializationUtils 
does not have dependency on any other test library classes, so we can simply 
move it to the top level testlibrary and update the tests which use it.

this fix is a part of ongoing effort on merging and cleaning up our test 
libraries[1].

webrev: http://cr.openjdk.java.net/~iignatyev//8180391/webrev.00/index.html
JBS: https://bugs.openjdk.java.net/browse/JDK-8180391
testing: affected tests (java/lang)

[1] https://bugs.openjdk.java.net/browse/JDK-8075327

Thanks,
-- Igor

Re: RFR: 8180385: fix HTML issues in java.xml module

2017-05-16 Thread Lance Andersen
Hi Jon,

Thank you for the explanation.  Makes sense.  No worries :-)

Best
Lance
> On May 16, 2017, at 7:02 PM, Jonathan Gibbons  
> wrote:
> 
> Lance,
> 
> The name seemed unnecessary given the context of the method associated with 
> the doc comment (i.e getNamespaceURI) and the preceding sentence:
> 
> >> the following table describes the returned Namespace URI value for all 
> >> possible prefix values: 
> 
> If that was editorial overreach, I apologise, and can add the text back, if 
> you would prefer.
> 
> See 
> http://cr.openjdk.java.net/~jjg/8180385/api.02/javax/xml/namespace/NamespaceContext.html#getNamespaceURI-java.lang.String
>  
> -
> 
> -- Jon
> 
> On 05/16/2017 03:55 PM, Lance Andersen wrote:
>> Hi Jon
>> 
>> src/java.xml/share/classes/javax/xml/namespace/NamespaceContext.java  -  Was 
>> there a reason you omitted {@code getNamespaceURI(prefix)} when creating the 
>> caption?
>> Looks pretty good otherwise
>>> On May 16, 2017, at 6:27 PM, Jonathan Gibbons >> > wrote:
>>> 
>>> Please review these fixes to update the public doc comments in the java.xml
>>> module for HTML 5.
>>> 
>>> Webrev: http://cr.openjdk.java.net/~jjg/8180385/webrev.02/ 
>>> 
>>> API: http://cr.openjdk.java.net/~jjg/8180385/api.02/ 
>>> 
>>> 
>>> -- Jon
>> 
>>  
>> 
>>   
>> 
>>  Lance Andersen| 
>> Principal Member of Technical Staff | +1.781.442.2037
>> Oracle Java Engineering 
>> 1 Network Drive 
>> Burlington, MA 01803
>> lance.ander...@oracle.com 
>> 
>> 
>> 
> 

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 





Re: RFR: 8180385: fix HTML issues in java.xml module

2017-05-16 Thread Jonathan Gibbons

Lance,

The name seemed unnecessary given the context of the method associated 
with the doc comment (i.e getNamespaceURI) and the preceding sentence:


>> the following table describes the returned Namespace URI value for 
all possible prefix values:


If that was editorial overreach, I apologise, and can add the text back, 
if you would prefer.


See 
http://cr.openjdk.java.net/~jjg/8180385/api.02/javax/xml/namespace/NamespaceContext.html#getNamespaceURI-java.lang.String-


-- Jon

On 05/16/2017 03:55 PM, Lance Andersen wrote:

Hi Jon

src/java.xml/share/classes/javax/xml/namespace/NamespaceContext.java  -  Was 
there a reason you omitted {@code getNamespaceURI(prefix)} when creating the 
caption?
Looks pretty good otherwise
On May 16, 2017, at 6:27 PM, Jonathan Gibbons 
> wrote:


Please review these fixes to update the public doc comments in the 
java.xml

module for HTML 5.

Webrev: http://cr.openjdk.java.net/~jjg/8180385/webrev.02/ 

API: http://cr.openjdk.java.net/~jjg/8180385/api.02/ 



-- Jon




Lance 
Andersen| Principal Member of Technical Staff | +1.781.442.2037

Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com 







Re: RFR: 8180385: fix HTML issues in java.xml module

2017-05-16 Thread huizhe wang

Hi Jon,

Looks good. Thanks again!

-Joe

On 5/16/2017 3:27 PM, Jonathan Gibbons wrote:
Please review these fixes to update the public doc comments in the 
java.xml

module for HTML 5.

Webrev: http://cr.openjdk.java.net/~jjg/8180385/webrev.02/
API: http://cr.openjdk.java.net/~jjg/8180385/api.02/

-- Jon




Re: RFR: 8180385: fix HTML issues in java.xml module

2017-05-16 Thread Lance Andersen
Hi Jon

src/java.xml/share/classes/javax/xml/namespace/NamespaceContext.java  -  Was 
there a reason you omitted {@code getNamespaceURI(prefix)} when creating the 
caption?

Looks pretty good otherwise

> On May 16, 2017, at 6:27 PM, Jonathan Gibbons  
> wrote:
> 
> Please review these fixes to update the public doc comments in the java.xml
> module for HTML 5.
> 
> Webrev: http://cr.openjdk.java.net/~jjg/8180385/webrev.02/
> API: http://cr.openjdk.java.net/~jjg/8180385/api.02/
> 
> -- Jon

 
  

 Lance Andersen| 
Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com 





Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Kirk Pepperdine
Hi,


> On May 17, 2017, at 3:11 AM, Cédric Champeau  
> wrote:
> 
> Hi Uwe,
> 
> I already explained multiple times why we need this. Short answer: because
> we must. Slightly longer answer: because the build environment, the daemon,
> has to reflect the environment from the CLI (or the IDE, in case we call
> using the tooling API), at the moment the build is executed. Why? Because a
> user wouldn't understand that a script which does:
> 
> println System.getenv('MY_VAR')
> 
> doesn't print "foo" after doing:
> 
> MY_VAR=foo gradle printVar

I disagree, this would be totally expected behavior. The daemon and this 
process would run in different shells and I am unaware of any daemon process 
that auto-magically reconfigures it’s self to adapt to any other arbitrary 
shell’s changed environment variables. In fact, IMHO, this seems like a 
fundamentally flawed way for the deamon process to behave. I believe the client 
communicating with the deamon that should be providing information to the 
daemon.

Kind regards,
Kirk



RFR: 8180385: fix HTML issues in java.xml module

2017-05-16 Thread Jonathan Gibbons

Please review these fixes to update the public doc comments in the java.xml
module for HTML 5.

Webrev: http://cr.openjdk.java.net/~jjg/8180385/webrev.02/
API: http://cr.openjdk.java.net/~jjg/8180385/api.02/

-- Jon


Re: [9] RFR: 8180375: Rename Provider to .spi.Provider

2017-05-16 Thread Mandy Chung
Naoto,

The javadoc of getBundle(String, Module) and getBundle(String,Locale,Module) 
methods also mention the service type “baseName”Provider that needs update as 
well.

Mandy

> On May 16, 2017, at 2:52 PM, Mandy Chung  wrote:
> 
> 
>> On May 16, 2017, at 11:14 AM, Naoto Sato  wrote:
>> 
>> Hello,
>> 
>> Please review the changes to the following issue:
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8180375
>> 
>> The proposed fix is located at:
>> 
>> http://cr.openjdk.java.net/~naoto/8180375/webrev.00/
>> 
>> This is to change the package name of the resource bundle provider to a 
>> different one, by appending ".spi" to the original package name. This change 
>> effectively avoids possible split package issue if resource bundles are 
>> provided from other named modules.
> 
> This would ease migration in particular when the provider modules are loaded 
> in a layer defined to multiple loader.  Existing resource bundles can be kept 
> in the same package.
> 
> 247  * The service type is designated by {@code package name + ".spi." + 
> simple name +"Provider"}. For
> 
> It may be clearer to say {@code  + “.spi.” +  + 
> “Provider”}.
> 
> 
> test/java/util/ResourceBundle/modules/appbasic/src/test/jdk/test/resources/spi/MyResourcesProviderImpl.java
> test/java/util/ResourceBundle/modules/appbasic2/src/test/jdk/test/resources/spi/MyResourcesProviderImpl.java
>   - they are provider implmentation classes.  They don’t need to be renamed. 
> If you want to rename them, maybe better to move them to 
> jdk.test.resources.internal package.
> 
> test/java/util/ResourceBundle/modules/layer/src/Main.java
> test/java/util/ResourceBundle/modules/layer/src/m1/p/Main.java
>   Nit: several long lines that can be wrapped.
> 
> Otherwise looks fine.
> 
> Mandy



Re: [9] RFR: 8180375: Rename Provider to .spi.Provider

2017-05-16 Thread Mandy Chung

> On May 16, 2017, at 11:14 AM, Naoto Sato  wrote:
> 
> Hello,
> 
> Please review the changes to the following issue:
> 
> https://bugs.openjdk.java.net/browse/JDK-8180375
> 
> The proposed fix is located at:
> 
> http://cr.openjdk.java.net/~naoto/8180375/webrev.00/
> 
> This is to change the package name of the resource bundle provider to a 
> different one, by appending ".spi" to the original package name. This change 
> effectively avoids possible split package issue if resource bundles are 
> provided from other named modules.

This would ease migration in particular when the provider modules are loaded in 
a layer defined to multiple loader.  Existing resource bundles can be kept in 
the same package.

 247  * The service type is designated by {@code package name + ".spi." + 
simple name +"Provider"}. For

It may be clearer to say {@code  + “.spi.” +  + 
“Provider”}.


test/java/util/ResourceBundle/modules/appbasic/src/test/jdk/test/resources/spi/MyResourcesProviderImpl.java
test/java/util/ResourceBundle/modules/appbasic2/src/test/jdk/test/resources/spi/MyResourcesProviderImpl.java
   - they are provider implmentation classes.  They don’t need to be renamed. 
If you want to rename them, maybe better to move them to 
jdk.test.resources.internal package.

test/java/util/ResourceBundle/modules/layer/src/Main.java
test/java/util/ResourceBundle/modules/layer/src/m1/p/Main.java
   Nit: several long lines that can be wrapped.

Otherwise looks fine.

Mandy

Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Martin Buchholz
On Tue, May 16, 2017 at 12:31 PM, Cédric Champeau  wrote:

>
> So far the only suitable workaround I can think of is an agent that would
> rewrite `System.getenv` to call our internal APIs. I'll suggest that in our
> next meeting (which is happening 30 mins from now).
>

Yes, given that you have control over your java processes, I think this is
what you should do, at least medium-term.  Longer-term, figure out some way
to migrate to a strategy that does not involve accessing jdk internals.


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Remi Forax
Hi Jonathan,
Bazel is a cathedral when Gradle is a bazaar,
Bazel uses a controlled subset of Python as build language, and a strictly 
defined API that do not let you extend Bazel in an arbitrary way. 

Those two approaches, a strict DSL or a general purpose languages retargeted to 
be used as a DSL have both there advantages and inconveniences, but i'm sure 
that everybody will agree that for the extensions/plugins migration, it's 
better to have a cleanly sandboxed environment. 

Rémi

- Mail original -
> De: "Jonathan Bluett-Duncan" 
> À: "Cédric Champeau" 
> Cc: "core-libs-dev" 
> Envoyé: Mardi 16 Mai 2017 22:03:49
> Objet: Re: Getting a live view of environment variables (Gradle and JDK 9)

> Hi Cédric,
> 
> I don't know if it's been considered, but has anyone from the Gradle team
> asked the Bazel team about this problem?
> 
> They may have useful insight about this problem with `System.getenv`,
> considering that Bazel is apparently very fast and (partially) written in
> Java.
> 
> I hope this helps.
> 
> Best regards,
> Jonathan


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Martin Buchholz
On Tue, May 16, 2017 at 1:03 PM, Jonathan Bluett-Duncan <
jbluettdun...@gmail.com> wrote:

> Hi Cédric,
>
> I don't know if it's been considered, but has anyone from the Gradle team
> asked the Bazel team about this problem?
>

I'm not on the Bazel team, but ... they are focused on being hermetic - if
they had any kind of build server process, I guess it would simply not
incorporate user environment variables, which are highly non-hermetic.


> They may have useful insight about this problem with `System.getenv`,
> considering that Bazel is apparently very fast and (partially) written in
> Java.
>


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Martin Buchholz
On Tue, May 16, 2017 at 12:29 PM, Chris Hegarty 
wrote:

>
> Re-reading/refreshing the env variables has come up before, in other
> contexts. As David pointed out [1], re-reading introduces no more risk than
> is already there ( if specified correctly ).
>

Adding a Java API for re-reading would be really weird because there would
be no Java API for writing, and there's no way on a Unix system to write at
all without introducing a risk of SEGV.


> -Chris.
>
> [1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-
> May/047668.html


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Jonathan Bluett-Duncan
Hi Cédric,

I don't know if it's been considered, but has anyone from the Gradle team
asked the Bazel team about this problem?

They may have useful insight about this problem with `System.getenv`,
considering that Bazel is apparently very fast and (partially) written in
Java.

I hope this helps.

Best regards,
Jonathan


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Cédric Champeau
I don't deny some people do not have the problem :)

2017-05-16 21:56 GMT+02:00 Uwe Schindler :

> Hi,
>
> Another example is not having your problem is Jenkins CI: It maintains a
> map of environment variables throughout the build. And you have to use that
> a a state container throughout the build. I have never seen a plug-in not
> using that, because it's fundamental to the whole build server.
>
> If you don't have that, then you have a fundamental API design problem! I
> don't think Java should take care of that. Backwards compatibility for some
> broken plugins is a no-go. You have to communicate that to developers. I'd
> run the build in a security manager and whenever somebody calls getenv()
> print a warning and in later version fail completely. Elasticsearch is
> doing similar stuff for their plugins.
>
> Uwe
>
>
> Am 16. Mai 2017 20:11:10 MESZ schrieb "Cédric Champeau" <
> cedric.champ...@gmail.com>:
>>
>> Hi Uwe,
>>
>> I already explained multiple times why we need this. Short answer:
>> because we must. Slightly longer answer: because the build environment, the
>> daemon, has to reflect the environment from the CLI (or the IDE, in case we
>> call using the tooling API), at the moment the build is executed. Why?
>> Because a user wouldn't understand that a script which does:
>>
>> println System.getenv('MY_VAR')
>>
>> doesn't print "foo" after doing:
>>
>> MY_VAR=foo gradle printVar
>>
>> (that's a silly example, but the simplest we can come with). Not
>> everything runs in a separate process: there are things that execute in the
>> daemon itself. A lot of things (starting from build scripts). And yes, we
>> can provide a different API to get environment variables, but:
>>
>> 1. it's a breaking change
>> 2. there are lots of plugins which use libraries, which do NOT depend on
>> the Gradle API, so that API wouldn't be available
>>
>> I'm honestly starting to get tired of explaining again and again WHY we
>> need this. If it was easy, we would have done it differently for years. We
>> worked around a bug in the JDK, which doesn't return the true environment
>> variables but the ones snapshotted when the process started. Now in JDK 9
>> we cannot workaround anymore, which either just kills Gradle performance or
>> forces us to write even nastier code with bytecode manipulating agents to
>> replace what `System.getenv` does.
>>
>> 2017-05-16 19:46 GMT+02:00 Uwe Schindler :
>>
>>> Hi,
>>>
>>> I still don't understand why you need to change the environment
>>> variables of the actual process. I was talking with Rémi about that last
>>> week, and it's not obvious to me why you need that. Sure, it is easier to
>>> change the environment of the actual process and then spawn a child process
>>> for some non-java build tool like GCC or even another standalone java
>>> program with option fork=true. But: Why not use the ProcessBuilder API when
>>> spawning those childs? So you just add an "official" build API inside
>>> Gradle (an official one, documented that allows Gradle plugins to modify
>>> the environment variables for the current build running). If you missed to
>>> add such an API from the beginning (IMHO its essential for a build system
>>> like Gradle), then you now have to tell your users: "Sorry we did something
>>> wrong and all our (bad) hacks to allow you to change environment variables
>>> are no longer working in the future, so please change your code. We are so
>>> sorry!"
>>>
>>> If some plugin is not using that new API, then it's not your problem.
>>> You document that you *have* to use the Environment API, because plugins
>>> cannot rely on the fact that another plugin or maybe another build running
>>> at a later time is changing the Gradle process environment.
>>>
>>> At some point you have to break backwards compatibility and tell users:
>>> Please update your code, otherwise plugin won't work anymore with Gradle
>>> version x.y (the one that's compatible to Java 9).
>>>
>>> Uwe
>>>
>>> -
>>> Uwe Schindler
>>> uschind...@apache.org
>>> ASF Member, Apache Lucene PMC / Committer
>>> Bremen, Germany
>>> http://lucene.apache.org/
>>>
>>> > -Original Message-
>>> > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
>>> > Behalf Of Cédric Champeau
>>> > Sent: Thursday, May 11, 2017 9:02 AM
>>> > To: Martin Buchholz 
>>> > Cc: core-libs-dev 
>>> > Subject: Re: Getting a live view of environment variables (Gradle and
>>> JDK 9)
>>> >
>>> > Thanks for the answers, folks, but I think they are kind of missing the
>>> > point. The fact is that environment variables *are* mutable. Java
>>> happens
>>> > to return an immutable view of the environment variables when the VM
>>> was
>>> > started, which is not the reality. We cannot trust what
>>> `System.geteenv`
>>> > returns. The Javadocs say "current system environment" but it's just
>>> not
>>> > true. The method 

Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Uwe Schindler
Hi,

Another example is not having your problem is Jenkins CI: It maintains a map of 
environment variables throughout the build. And you have to use that a a state 
container throughout the build. I have never seen a plug-in not using that, 
because it's fundamental to the whole build server.

If you don't have that, then you have a fundamental API design problem! I don't 
think Java should take care of that. Backwards compatibility for some broken 
plugins is a no-go. You have to communicate that to developers. I'd run the 
build in a security manager and whenever somebody calls getenv() print a 
warning and in later version fail completely. Elasticsearch is doing similar 
stuff for their plugins.

Uwe

Am 16. Mai 2017 20:11:10 MESZ schrieb "Cédric Champeau" 
:
>Hi Uwe,
>
>I already explained multiple times why we need this. Short answer:
>because
>we must. Slightly longer answer: because the build environment, the
>daemon,
>has to reflect the environment from the CLI (or the IDE, in case we
>call
>using the tooling API), at the moment the build is executed. Why?
>Because a
>user wouldn't understand that a script which does:
>
>println System.getenv('MY_VAR')
>
>doesn't print "foo" after doing:
>
>MY_VAR=foo gradle printVar
>
>(that's a silly example, but the simplest we can come with). Not
>everything
>runs in a separate process: there are things that execute in the daemon
>itself. A lot of things (starting from build scripts). And yes, we can
>provide a different API to get environment variables, but:
>
>1. it's a breaking change
>2. there are lots of plugins which use libraries, which do NOT depend
>on
>the Gradle API, so that API wouldn't be available
>
>I'm honestly starting to get tired of explaining again and again WHY we
>need this. If it was easy, we would have done it differently for years.
>We
>worked around a bug in the JDK, which doesn't return the true
>environment
>variables but the ones snapshotted when the process started. Now in JDK
>9
>we cannot workaround anymore, which either just kills Gradle
>performance or
>forces us to write even nastier code with bytecode manipulating agents
>to
>replace what `System.getenv` does.
>
>2017-05-16 19:46 GMT+02:00 Uwe Schindler :
>
>> Hi,
>>
>> I still don't understand why you need to change the environment
>variables
>> of the actual process. I was talking with Rémi about that last week,
>and
>> it's not obvious to me why you need that. Sure, it is easier to
>change the
>> environment of the actual process and then spawn a child process for
>some
>> non-java build tool like GCC or even another standalone java program
>with
>> option fork=true. But: Why not use the ProcessBuilder API when
>spawning
>> those childs? So you just add an "official" build API inside Gradle
>(an
>> official one, documented that allows Gradle plugins to modify the
>> environment variables for the current build running). If you missed
>to add
>> such an API from the beginning (IMHO its essential for a build system
>like
>> Gradle), then you now have to tell your users: "Sorry we did
>something
>> wrong and all our (bad) hacks to allow you to change environment
>variables
>> are no longer working in the future, so please change your code. We
>are so
>> sorry!"
>>
>> If some plugin is not using that new API, then it's not your problem.
>You
>> document that you *have* to use the Environment API, because plugins
>cannot
>> rely on the fact that another plugin or maybe another build running
>at a
>> later time is changing the Gradle process environment.
>>
>> At some point you have to break backwards compatibility and tell
>users:
>> Please update your code, otherwise plugin won't work anymore with
>Gradle
>> version x.y (the one that's compatible to Java 9).
>>
>> Uwe
>>
>> -
>> Uwe Schindler
>> uschind...@apache.org
>> ASF Member, Apache Lucene PMC / Committer
>> Bremen, Germany
>> http://lucene.apache.org/
>>
>> > -Original Message-
>> > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net]
>On
>> > Behalf Of Cédric Champeau
>> > Sent: Thursday, May 11, 2017 9:02 AM
>> > To: Martin Buchholz 
>> > Cc: core-libs-dev 
>> > Subject: Re: Getting a live view of environment variables (Gradle
>and
>> JDK 9)
>> >
>> > Thanks for the answers, folks, but I think they are kind of missing
>the
>> > point. The fact is that environment variables *are* mutable. Java
>happens
>> > to return an immutable view of the environment variables when the
>VM was
>> > started, which is not the reality. We cannot trust what
>`System.geteenv`
>> > returns. The Javadocs say "current system environment" but it's
>just not
>> > true. The method should be called `getEnvWhenTheVMWasStarted`.
>> >
>> > We also have the problem that the environment of the client and the
>> > daemon
>> > differ over time, as I explained in the previous email. And it is
>pretty
>> > easy to understand 

Re: JDK 9 doc-only RFR of 8180353: FileOutputStream documentation does not indicate properly whether files get truncated or not

2017-05-16 Thread Brian Burkhalter
Hi Chris,

Thanks for the review. Here is a revised version, thanks to a comment from 
Daniel, which I think might be better:

Thanks,

Brian

--- a/src/java.base/share/classes/java/io/FileOutputStream.java
+++ b/src/java.base/share/classes/java/io/FileOutputStream.java
@@ -91,6 +91,10 @@
  * If the file exists but is a directory rather than a regular file, does
  * not exist but cannot be created, or cannot be opened for any other
  * reason then a FileNotFoundException is thrown.
+ * 
+ * Invoking this constructor with the parameter {@code name} is equivalent
+ * to invoking {@link #FileOutputStream(String,boolean)
+ * new FileOutputStream(name, false)}.
  *
  * @param  name   the system-dependent filename
  * @exception  FileNotFoundException  if the file exists but is a directory

On May 16, 2017, at 1:05 AM, Chris Hegarty  wrote:

> Looks good Brian.
> 
> -Chris.
> 
>> On 16 May 2017, at 02:27, Brian Burkhalter  
>> wrote:
>> 
>> Please review at your convenience.
>> 
>> Issue:   https://bugs.openjdk.java.net/browse/JDK-8180353
>> Patch:   [1]
>> 
>> Thanks,
>> 
>> Brian
>> 
>> [1] Hg diff
>> 
>> --- a/src/java.base/share/classes/java/io/FileOutputStream.java
>> +++ b/src/java.base/share/classes/java/io/FileOutputStream.java
>> @@ -91,6 +91,12 @@
>> * If the file exists but is a directory rather than a regular file, does
>> * not exist but cannot be created, or cannot be opened for any other
>> * reason then a FileNotFoundException is thrown.
>> + * 
>> + * Invoking this constructor with the parameter {@code name} is 
>> equivalent
>> + * to invoking the constructor {@link #FileOutputStream(String,boolean)
>> + * FileOutputStream(name,append)} with the same {@code String} parameter
>> + * {@code name} and the {@code boolean} parameter {@code append} equal 
>> to
>> + * {@code false}.
>> *
>> * @param  name   the system-dependent filename
>> * @exception  FileNotFoundException  if the file exists but is a 
>> directory
> 



Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Cédric Champeau
Hi Mario,

I'm not suggesting to change the JDK. My original email is about having a
new API to get a live view. Alternatives, like new methods to refresh the
view, are non-breaking (I think). I perfectly understand, and fully
respect, the willingness not to break millions of developers, and that's
not what I'm suggesting.

>
> Regarding your example:
>
> """
> println System.getenv('MY_VAR')
>
> doesn't print "foo" after doing:
>
> MY_VAR=foo gradle printVar
> """
>
> I agree that the environment variables may change during the program
> execution and that perhaps Java may eventually need to reflect that,
> but this example is not really appropriate, the Java process started
> with an environment variable not set, the one that starts with
> MY_VAR=foo is technically a different process...
>

Yes, that's one of our use cases. We mutate the environment variables so
that the process of the daemon sees the new environment variables, as well
as all Java code running there, because that's what users, and tools
executed from Gradle, expect. For forked process we can have our own API
which reads environment variables and passes them to the forked process
(which can itself be a long running process, when we re-use workers). The
issue is mostly with Java code running in the daemon itself (people calling
getenv, be it in a plugin, or in libraries).

So far the only suitable workaround I can think of is an agent that would
rewrite `System.getenv` to call our internal APIs. I'll suggest that in our
next meeting (which is happening 30 mins from now).


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Chris Hegarty

> On 16 May 2017, at 20:19, Mario Torre  wrote:
> ..
> Regarding your example:
> 
> """
> println System.getenv('MY_VAR')
> 
> doesn't print "foo" after doing:
> 
> MY_VAR=foo gradle printVar
> """
> 
> I agree that the environment variables may change during the program
> execution and that perhaps Java may eventually need to reflect that,
> but this example is not really appropriate, the Java process started
> with an environment variable not set, the one that starts with
> MY_VAR=foo is technically a different process…

Sure, but the “daemon” ( in terms of Gradle ) should behave “as if” it were
the initiating process. I have some sympathy for this.

Re-reading/refreshing the env variables has come up before, in other
contexts. As David pointed out [1], re-reading introduces no more risk than
is already there ( if specified correctly ).

-Chris.

[1] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-May/047668.html

Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Mario Torre
2017-05-16 20:38 GMT+02:00 Cédric Champeau :
> Let me rephrase: it's tiring to have to repeat why we need it, and why we
> honor the contract of environment variables. Why is it so hard to admit the
> JDK has a bug?

Hello Cédric,

I hope you don't take it wrong or personal, but we really are here to
help, unfortunately this attitude is not helping us helping you. I
suggest to re-read what Brian just said.

The reason why you get suggestion to change how you work is because
changing the JDK effect billions of users that may be affected
negatively by the changes.

Regarding your example:

"""
println System.getenv('MY_VAR')

doesn't print "foo" after doing:

MY_VAR=foo gradle printVar
"""

I agree that the environment variables may change during the program
execution and that perhaps Java may eventually need to reflect that,
but this example is not really appropriate, the Java process started
with an environment variable not set, the one that starts with
MY_VAR=foo is technically a different process...

Cheers,
Mario

-- 
pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF
Fingerprint: BA39 9666 94EC 8B73 27FA  FC7C 4086 63E3 80F2 40CF

Java Champion - Blog: http://neugens.wordpress.com - Twitter: @neugens
Proud GNU Classpath developer: http://www.classpath.org/
OpenJDK: http://openjdk.java.net/projects/caciocavallo/

Please, support open standards:
http://endsoftpatents.org/


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Cédric Champeau
Let me rephrase: it's tiring to have to repeat why we need it, and why we
honor the contract of environment variables. Why is it so hard to admit the
JDK has a bug? I proposed different solutions for this, including non
breaking changes (a new method). All I have in return is suggestions to
change how *we* work, while discarding why it is important for us that the
contract of environment variables is respected. And let me say that this is
not inconvenient for *us* at Gradle. It's inconvenient for thousands of
developers who will suffer slow build times if we don't find a solution. My
personal situation doesn't matter. I care about what users will see
(including the larger ecosystem, Android and native).

2017-05-16 20:23 GMT+02:00 Brian Goetz :

> I’m sorry you find it tiring, but I think you underestimate how high the
> bar is (and should be) for changing how Java works just because it’s
> inconvenient for your situation.  In my view, you’re not remotely there on
> justifying this.  By all means continue, but “this is too tiring” isn’t a
> valid argument.
>
> > I'm honestly starting to get tired of explaining again and again WHY we
> > need this.
>


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Brian Goetz
I’m sorry you find it tiring, but I think you underestimate how high the bar is 
(and should be) for changing how Java works just because it’s inconvenient for 
your situation.  In my view, you’re not remotely there on justifying this.  By 
all means continue, but “this is too tiring” isn’t a valid argument.  

> I'm honestly starting to get tired of explaining again and again WHY we
> need this. 


[9] RFR: 8180375: Rename Provider to .spi.Provider

2017-05-16 Thread Naoto Sato

Hello,

Please review the changes to the following issue:

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

The proposed fix is located at:

http://cr.openjdk.java.net/~naoto/8180375/webrev.00/

This is to change the package name of the resource bundle provider to a 
different one, by appending ".spi" to the original package name. This 
change effectively avoids possible split package issue if resource 
bundles are provided from other named modules.


Naoto


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Cédric Champeau
Hi Uwe,

I already explained multiple times why we need this. Short answer: because
we must. Slightly longer answer: because the build environment, the daemon,
has to reflect the environment from the CLI (or the IDE, in case we call
using the tooling API), at the moment the build is executed. Why? Because a
user wouldn't understand that a script which does:

println System.getenv('MY_VAR')

doesn't print "foo" after doing:

MY_VAR=foo gradle printVar

(that's a silly example, but the simplest we can come with). Not everything
runs in a separate process: there are things that execute in the daemon
itself. A lot of things (starting from build scripts). And yes, we can
provide a different API to get environment variables, but:

1. it's a breaking change
2. there are lots of plugins which use libraries, which do NOT depend on
the Gradle API, so that API wouldn't be available

I'm honestly starting to get tired of explaining again and again WHY we
need this. If it was easy, we would have done it differently for years. We
worked around a bug in the JDK, which doesn't return the true environment
variables but the ones snapshotted when the process started. Now in JDK 9
we cannot workaround anymore, which either just kills Gradle performance or
forces us to write even nastier code with bytecode manipulating agents to
replace what `System.getenv` does.

2017-05-16 19:46 GMT+02:00 Uwe Schindler :

> Hi,
>
> I still don't understand why you need to change the environment variables
> of the actual process. I was talking with Rémi about that last week, and
> it's not obvious to me why you need that. Sure, it is easier to change the
> environment of the actual process and then spawn a child process for some
> non-java build tool like GCC or even another standalone java program with
> option fork=true. But: Why not use the ProcessBuilder API when spawning
> those childs? So you just add an "official" build API inside Gradle (an
> official one, documented that allows Gradle plugins to modify the
> environment variables for the current build running). If you missed to add
> such an API from the beginning (IMHO its essential for a build system like
> Gradle), then you now have to tell your users: "Sorry we did something
> wrong and all our (bad) hacks to allow you to change environment variables
> are no longer working in the future, so please change your code. We are so
> sorry!"
>
> If some plugin is not using that new API, then it's not your problem. You
> document that you *have* to use the Environment API, because plugins cannot
> rely on the fact that another plugin or maybe another build running at a
> later time is changing the Gradle process environment.
>
> At some point you have to break backwards compatibility and tell users:
> Please update your code, otherwise plugin won't work anymore with Gradle
> version x.y (the one that's compatible to Java 9).
>
> Uwe
>
> -
> Uwe Schindler
> uschind...@apache.org
> ASF Member, Apache Lucene PMC / Committer
> Bremen, Germany
> http://lucene.apache.org/
>
> > -Original Message-
> > From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> > Behalf Of Cédric Champeau
> > Sent: Thursday, May 11, 2017 9:02 AM
> > To: Martin Buchholz 
> > Cc: core-libs-dev 
> > Subject: Re: Getting a live view of environment variables (Gradle and
> JDK 9)
> >
> > Thanks for the answers, folks, but I think they are kind of missing the
> > point. The fact is that environment variables *are* mutable. Java happens
> > to return an immutable view of the environment variables when the VM was
> > started, which is not the reality. We cannot trust what `System.geteenv`
> > returns. The Javadocs say "current system environment" but it's just not
> > true. The method should be called `getEnvWhenTheVMWasStarted`.
> >
> > We also have the problem that the environment of the client and the
> > daemon
> > differ over time, as I explained in the previous email. And it is pretty
> > easy to understand that _when the build runs_, we need to get the
> > environment variables of the _client_, not the daemon. Imagine something
> > as
> > simple as this:
> >
> > String toolPath = System.getenv('SOMETOOL_HOME')
> >
> > and imagine that this code runs in the daemon. When the daemon is
> started,
> > there's absolutely no guarantee that this variable is going to exist.
> > However, we know that when we're going to execute the build, this
> variable
> > *has* to be defined. Obviously, it's going to be done from the CLI:
> >
> > $ export SOMETOOL_HOME = ...
> > $ ./gradlew run ---> connects to the daemon, passes environment
> variables,
> > mutates those of the daemon, which then executes the build
> >
> > Similarly, from a *single* CLI/terminal, it's very common to change the
> > values of environment variables (because, hey, they are mutable!):
> >
> > $ export SOMETOOL_HOME = another path I want to try out
> > $ 

RE: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Uwe Schindler
Hi,

I still don't understand why you need to change the environment variables of 
the actual process. I was talking with Rémi about that last week, and it's not 
obvious to me why you need that. Sure, it is easier to change the environment 
of the actual process and then spawn a child process for some non-java build 
tool like GCC or even another standalone java program with option fork=true. 
But: Why not use the ProcessBuilder API when spawning those childs? So you just 
add an "official" build API inside Gradle (an official one, documented that 
allows Gradle plugins to modify the environment variables for the current build 
running). If you missed to add such an API from the beginning (IMHO its 
essential for a build system like Gradle), then you now have to tell your 
users: "Sorry we did something wrong and all our (bad) hacks to allow you to 
change environment variables are no longer working in the future, so please 
change your code. We are so sorry!"

If some plugin is not using that new API, then it's not your problem. You 
document that you *have* to use the Environment API, because plugins cannot 
rely on the fact that another plugin or maybe another build running at a later 
time is changing the Gradle process environment.

At some point you have to break backwards compatibility and tell users: Please 
update your code, otherwise plugin won't work anymore with Gradle version x.y 
(the one that's compatible to Java 9).

Uwe

-
Uwe Schindler
uschind...@apache.org 
ASF Member, Apache Lucene PMC / Committer
Bremen, Germany
http://lucene.apache.org/

> -Original Message-
> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
> Behalf Of Cédric Champeau
> Sent: Thursday, May 11, 2017 9:02 AM
> To: Martin Buchholz 
> Cc: core-libs-dev 
> Subject: Re: Getting a live view of environment variables (Gradle and JDK 9)
> 
> Thanks for the answers, folks, but I think they are kind of missing the
> point. The fact is that environment variables *are* mutable. Java happens
> to return an immutable view of the environment variables when the VM was
> started, which is not the reality. We cannot trust what `System.geteenv`
> returns. The Javadocs say "current system environment" but it's just not
> true. The method should be called `getEnvWhenTheVMWasStarted`.
> 
> We also have the problem that the environment of the client and the
> daemon
> differ over time, as I explained in the previous email. And it is pretty
> easy to understand that _when the build runs_, we need to get the
> environment variables of the _client_, not the daemon. Imagine something
> as
> simple as this:
> 
> String toolPath = System.getenv('SOMETOOL_HOME')
> 
> and imagine that this code runs in the daemon. When the daemon is started,
> there's absolutely no guarantee that this variable is going to exist.
> However, we know that when we're going to execute the build, this variable
> *has* to be defined. Obviously, it's going to be done from the CLI:
> 
> $ export SOMETOOL_HOME = ...
> $ ./gradlew run ---> connects to the daemon, passes environment variables,
> mutates those of the daemon, which then executes the build
> 
> Similarly, from a *single* CLI/terminal, it's very common to change the
> values of environment variables (because, hey, they are mutable!):
> 
> $ export SOMETOOL_HOME = another path I want to try out
> $ ./gradlew run --> ... must update env vars again
> 
> So, to work around the problem that Java doesn't provide a way to mutate
> the current environment variables, we perform a JNI call to do it. *Then*
> we need to mutate the "immutable view" that Java provides, or all Java code
> is going to get wrong information, and we would never succeed. Note that
> having to resort on JNI to mutate the environment is not the problem. We
> can live with that. Once can think it's a bad idea to allow mutation, in
> practice, it is necessary, but I reckon it could be a bad idea to have an
> API for this. The *real* issue is that `System.getenv` does *not* return
> the real values, because it's an immutable view of what existed at the VM
> startup.
> 
> Note that it's not recommended to mutate environment variables in a
> multi-threaded environment. But in practice, you can assimilate what we do
> to the VM startup: we get environment variables, mutate them, _then_ the
> build runs (and only at that moment we would effectively be
> multi-threaded). We can live with potential issues of mutating the
> environment: the benefits outperforms the drawbacks by orders of
> magnitude.
> When the daemon is activated, we're not talking about 10% faster builds.
> They can effectively be 3, 5 or 10x faster!
> 
> Now, back to the problem with JDK 9:
> 
> - first, the implementation has changed. But we could adapt our code.
> - second, calling `setAccessible` is not allowed anymore. And this is a
> MAJOR pita. The problem is that we're trying to access the java 

Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Cédric Champeau
2017-05-16 18:57 GMT+02:00 Sanne Grinovero :

> Hi Cédric,
>
> we use Gradle a lot in our team (Hibernate) and all your efforts to
> make it faster are highly appreciated.
>
> I can assure you that during a normal day of work while we might
> rebuild a project a few hundred times, I'll rarely have to change
> environment variables.
>
> Obviously I don't represent all users, but as you said yourself the
> need to change environment variables is both something you have to
> support and something which is extremely rare.

So can't you simply keep the daemon around, as long as environment
> variables don't change?
>

That's one of the suggestions in the original email, but as you can see,
it's not uncommon to have environment variables mutated "behind you back",
even in a simple command line. Sometimes, it's as easy as having 2
different terminals open (which I think is far from being a rare case):
then you cannot share the same daemon instance, which is either very bad
for performance or memory usage. Why? Because the open terminal has some
specific variables (WINDOWID, ...).


>
> For the rare case in which environment variables have been mutated,
> you could trigger a full restart.
>
> As a second step, I should mention that on a typical workstation I
> might have multiple terminals open, each configured with a different
> environment and possibly working on a different project or different
> branch - or just testing on a different environment.
> It would be even nicer if one could have a different Gradle daemon
> instances running for each environment; incidentally this might have
> other benefits, like we often need to switch JDK build or even vendor.
> I'd be happy to be able to give them some name as identifier.
>
>
That's something we consider, but it's not the same use case at all (my
typical setup is pretty much the opposite: a lot of terminals open for the
same environment/build), so it has to be supported differently. I don't
think we should mix this in the game.


> I think this solution would improve Gradle and allow the JDK to move
> on with this quite nice cleanup.
>
> HTH
>
> Thanks,
> Sanne
>
>
> On Tue, May 16, 2017 at 11:05 AM, Cédric Champeau
>  wrote:
> > Thanks Peter for your thoughts, but I don't think it's as simple as that.
> > As I explained in my original email, there are multiple ways the
> > environment variables can be mutated, and it can be done _externally_.
> > Typically, during a task execution, a JNI call performed by a random
> native
> > tool could mutate the environment. That's something we, as a build tool,
> > have to consider as a use case. It's very unlikely but it can happen.
> This
> > means that for the same classloader, the environment can change. And for
> > performance reasons, we cache classloaders between builds, and reuse them
> > as much as possible (because classloading is far from being cheap).
> >
> > 2017-05-14 19:51 GMT+02:00 Peter Levart :
> >
> >> Hi Cedric,
> >>
> >> Chiming in with my thoughts...
> >>
> >> It's unfortunate that Gradle plugins or libraries used by plugins use
> >> environment variables at all. I wonder who was the first? Did Gradle
> >> introduce the feature of passing client environment to the daemon and
> >> establishing the view of System.getenv for plugins 1st or did libraries
> >> used by plugins happen to use environment variables using System.getenv
> and
> >> Gradle was just kind enough to provide them a dynamic view controlled by
> >> client?
> >>
> >> In the end it doesn't matter. The fact is that System.getenv is part of
> >> standard Java API and anyone using it should be aware that by doing so,
> >> they are limiting their software to be (re)configured only by spawning
> new
> >> process(es). UNIX environment was not designed to be mutated during the
> >> course of a long-running process. Maybe just at process startup/setup
> when
> >> conditions are under control (i.e. a single running thread) but later,
> the
> >> environment is meant to be read only.
> >>
> >> Maybe there is a solution for Gradle and othert though. But that
> solution,
> >> I think, is not in exposing a "live" view of process environment through
> >> System.getenv or new methods to "refresh" the view as you are proposing,
> >> since that would only encourage people to mutate the process environment
> >> which, as was established on this thread, is not safe in a long-running
> >> multi-threaded process, which Java processes usually are. Perhaps the
> >> solution is in extending the System.getenv API with ways to provide
> >> "custom" views of variables for code that runs in a "container".
> >>
> >> Gradle is, among other things, a container for plugins. Is Gradle
> running
> >> plugins in a separate ClassLoader? Does it use a separate ClassLoader
> for
> >> each "build" which might bring with it a new set of environment
> variables
> >> from the client? In such a setup, one could 

Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Sanne Grinovero
Hi Cédric,

we use Gradle a lot in our team (Hibernate) and all your efforts to
make it faster are highly appreciated.

I can assure you that during a normal day of work while we might
rebuild a project a few hundred times, I'll rarely have to change
environment variables.

Obviously I don't represent all users, but as you said yourself the
need to change environment variables is both something you have to
support and something which is extremely rare.
So can't you simply keep the daemon around, as long as environment
variables don't change?

For the rare case in which environment variables have been mutated,
you could trigger a full restart.

As a second step, I should mention that on a typical workstation I
might have multiple terminals open, each configured with a different
environment and possibly working on a different project or different
branch - or just testing on a different environment.
It would be even nicer if one could have a different Gradle daemon
instances running for each environment; incidentally this might have
other benefits, like we often need to switch JDK build or even vendor.
I'd be happy to be able to give them some name as identifier.

I think this solution would improve Gradle and allow the JDK to move
on with this quite nice cleanup.

HTH

Thanks,
Sanne


On Tue, May 16, 2017 at 11:05 AM, Cédric Champeau
 wrote:
> Thanks Peter for your thoughts, but I don't think it's as simple as that.
> As I explained in my original email, there are multiple ways the
> environment variables can be mutated, and it can be done _externally_.
> Typically, during a task execution, a JNI call performed by a random native
> tool could mutate the environment. That's something we, as a build tool,
> have to consider as a use case. It's very unlikely but it can happen. This
> means that for the same classloader, the environment can change. And for
> performance reasons, we cache classloaders between builds, and reuse them
> as much as possible (because classloading is far from being cheap).
>
> 2017-05-14 19:51 GMT+02:00 Peter Levart :
>
>> Hi Cedric,
>>
>> Chiming in with my thoughts...
>>
>> It's unfortunate that Gradle plugins or libraries used by plugins use
>> environment variables at all. I wonder who was the first? Did Gradle
>> introduce the feature of passing client environment to the daemon and
>> establishing the view of System.getenv for plugins 1st or did libraries
>> used by plugins happen to use environment variables using System.getenv and
>> Gradle was just kind enough to provide them a dynamic view controlled by
>> client?
>>
>> In the end it doesn't matter. The fact is that System.getenv is part of
>> standard Java API and anyone using it should be aware that by doing so,
>> they are limiting their software to be (re)configured only by spawning new
>> process(es). UNIX environment was not designed to be mutated during the
>> course of a long-running process. Maybe just at process startup/setup when
>> conditions are under control (i.e. a single running thread) but later, the
>> environment is meant to be read only.
>>
>> Maybe there is a solution for Gradle and othert though. But that solution,
>> I think, is not in exposing a "live" view of process environment through
>> System.getenv or new methods to "refresh" the view as you are proposing,
>> since that would only encourage people to mutate the process environment
>> which, as was established on this thread, is not safe in a long-running
>> multi-threaded process, which Java processes usually are. Perhaps the
>> solution is in extending the System.getenv API with ways to provide
>> "custom" views of variables for code that runs in a "container".
>>
>> Gradle is, among other things, a container for plugins. Is Gradle running
>> plugins in a separate ClassLoader? Does it use a separate ClassLoader for
>> each "build" which might bring with it a new set of environment variables
>> from the client? In such a setup, one could provide a separate set of
>> environment variables for each ClassLoader, simply by passing them to the
>> ClassLoader constructor. System.getenv would need to be a @CallerSensitive
>> method which would return caller's ClassLoader view of variables, if any
>> such view was established, or simply the view inherited from the parent
>> ClassLoader.
>>
>> Such would be a "functional" approach, which would keep environment
>> variables immutable, but allow child "contexts" to have different views of
>> them. Such approach would also play well with idioms like:
>>
>> class AbcPlugin {
>> static final String SOME_SETTING = System.getenv("SOME_SETTING");
>>
>> ...and would also help other containers (such as app servers) support
>> existing libraries that use environment variables to be configured
>> differently in different apps deployed in the same VM process.
>>
>> Just a thought.
>>
>> Regards, Peter
>>
>>
>>
>> On 05/11/2017 09:02 AM, Cédric Champeau wrote:
>>

Re: RFR 8u Backport: 8179515: Class java.util.concurrent.ThreadLocalRandom fails to Initialize when using SecurityManager

2017-05-16 Thread Roger Riggs

Hi David,

Looks fine.

Roger


On 5/16/2017 3:02 AM, David Holmes wrote:
The existing code in 8u was a little different to 9, but the new code 
is identical (other than package names):


webrev: http://cr.openjdk.java.net/~dholmes/8179515/webrev.8u/
Bug: https://bugs.openjdk.java.net/browse/JDK-8179515
9 changeset: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/bb4cdc198dc0
9 review thread: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-May/047615.html


Thanks,
David




Re: RFR(S): 8180195: remove jaxp testlibrary

2017-05-16 Thread Igor Ignatyev
Hi Joe, Frank,

thank you for the review!

Cheers,
-- Igor

> On May 15, 2017, at 8:21 PM, huizhe wang  wrote:
> 
> +1. Thanks Frank for checking.
> 
> -Joe
> 
> On 5/15/2017 7:50 PM, Frank Yuan wrote:
>> Looks fine, although I am not a reviewer.
>> 
>> Thanks
>> Frank
>> 
>>> -Original Message-
>>> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On 
>>> Behalf Of Igor Ignatyev
>>> Subject: RFR(S): 8180195: remove jaxp testlibrary
>>> 
>>> http://cr.openjdk.java.net/~iignatyev//8180195/webrev.00/index.html
 3049 lines changed: 114 ins; 2927 del; 8 mod;
>>> Hi all,
>>> 
>>> could you please review this small patch which removes a fork of 
>>> testlibrary from jaxp repo? there were a few differences b/w the
>> testlibraries:
>>> top level testlibrary did not have CompilerUtils class, its ProcessTools 
>>> did not have executeTestJava (which is basically an alias
>> for
>>> executeTestJvm) and its OutputAnalyzer did not have methods to dump stdout, 
>>> stderr into specific streams.
>>> 
>>> this fix is a part of ongoing effort on merging and cleaning up our test 
>>> libraries[1].
>>> 
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8180195
>>> webrev: http://cr.openjdk.java.net/~iignatyev//8180195/webrev.00/index.html
>>> testing: :jaxp_all
>>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8075327
>>> 
>>> Thanks,
>>> -- Igor
>> 
> 



Re: Java-Time Clock Implementation Note

2017-05-16 Thread Stephen Colebourne
I'll let you log the issue, as you'll know what flags to set to get it
through the process!
Stephen

On 16 May 2017 at 15:39, Daniel Fuchs  wrote:
> Thanks Stephen,
>
> On 16/05/2017 15:32, Stephen Colebourne wrote:
>>
>> Its not terrible, but I think confusing enough it should be fixed.
>>
>> Maybe:
>> "The clock implementation provided here is based on the same
>> underlying clock as System.currentTimeMillis(), but may have a
>> precision finer
>> than milliseconds if available. However, little to no guarantee is
>> provided about the accuracy of the underlying clock. Applications
>> requiring a more accurate clock must implement this abstract class
>> themselves using a different external clock, such as an NTP server."
>
>
> I like this. Let's turn it into a docs-only bug for JDK 9: we can
> still fix those... I will log the issue - unless you prefer to do it?
>
> best regards,
>
> -- daniel
>
>>
>> Stephen
>>
>>
>>
>> On 16 May 2017 at 11:59, Daniel Fuchs  wrote:
>>>
>>> Hi Stephen,
>>>
>>> Thanks for pointing that out.
>>>
>>> On 16/05/2017 11:27, Stephen Colebourne wrote:


 In JDK 9, the implementation of Clock has been improved to be better
 than millisecond in most cases [1]. However, I've just noticed that
 the Javadoc of the "Implementation Note" is now wrong. It says:

 "The clock implementation provided here is based on
 System.currentTimeMillis(). That method provides little to no
 guarantee about the accuracy of the clock. Applications requiring a
 more accurate clock must implement this abstract class themselves
 using a different external clock, such as an NTP server."

 This needs to be updated to indicate that the implementation is no
 longer based on System.currentTimeMillis(). Since this is an
 "implementation note" I hope that this doesn't affect the
 specification, or need too much process.
>>>
>>>
>>>
>>> Strictly speaking the implementation is based on the *same* clock
>>> than System.currentTimeMillis() is based on. This is very important
>>> as it means that times obtained by System.currentTimeMillis() and
>>> times obtained by the system clock are consistent with each other.
>>>
>>> So I am not sure the note [2] is actually wrong?
>>>
>>> Would the following be a better wording (syntax/grammar may need
>>> correction)?
>>>
>>> <<
>>> The clock implementation provided here is based on the same clock than
>>> System.currentTimeMillis() is based on, but may have a precision finer
>>> than milliseconds if provided by the underlying clock.
>>> However, little to no guarantee is provided about the accuracy of the
>>> underlying clock. Applications requiring a more accurate clock must
>>> implement this abstract class themselves using a different external
>>> clock,
>>> such as an NTP server.
>
>
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> [2] http://download.java.net/java/jdk9/docs/api/java/time/Clock.html
>>>
>>>
>>>
>>>

 Is anyone willing to take this up as a JDK 9 bug?

 thanks
 Stephen

 [1] https://bugs.openjdk.java.net/browse/JDK-8068730

>>>
>


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Mario Torre
2017-05-15 7:14 GMT+02:00 David Holmes :
>> There would have to be a caveat on System.getenv(true) if we went that
>> path, that it is up to the user to ensure it is called in as safe a
>> manner as possible having regard to any concurrent updates in their
>> application code and how the environment is managed on a given platform.

If we get a System.setEnv in Java we can synchronise that method
access, either internally or by asking the user to explicitly
synchronise.

That would not protect from direct system calls, but it with a proper
Java API that would be the recommended way to set environment
variables, thus lowering the risk.

The question is if this use case is really that compelling that
justify an official wrapper against set/putEnv.

Cheers,
Mario
-- 
pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF
Fingerprint: BA39 9666 94EC 8B73 27FA  FC7C 4086 63E3 80F2 40CF

Java Champion - Blog: http://neugens.wordpress.com - Twitter: @neugens
Proud GNU Classpath developer: http://www.classpath.org/
OpenJDK: http://openjdk.java.net/projects/caciocavallo/

Please, support open standards:
http://endsoftpatents.org/


Re: [XS] JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent

2017-05-16 Thread Dmitry Samersoff
Matthias,

Looks good to me.

-Dmitry

On 2017-05-16 13:21, Baesken, Matthias wrote:
>  Sure, I forward it to  serviceability-dev .
> 
> -Original Message-
> From: Alan Bateman [mailto:alan.bate...@oracle.com] 
> Sent: Dienstag, 16. Mai 2017 11:51
> To: Baesken, Matthias ; 
> core-libs-dev@openjdk.java.net
> Cc: Simonis, Volker 
> Subject: Re: [XS] JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent
> 
> 
> 
> On 16/05/2017 10:04, Baesken, Matthias wrote:
>> Hello, could you please review this small change :
>>
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8180413/
>>
>> it fixes a number of places in   jdk.jdwp.agent   where in case of an error 
>> it is attempted to write to NULL .
>>
>> Bug :  JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent
>>
>>
>> https://bugs.openjdk.java.net/browse/JDK-8180413
>>
>>
> Can you bring this to serviceability-dev as that is the mailing list 
> where the JDWP agent is maintained?
> 
> -Alan
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.


Java-Time Clock Implementation Note

2017-05-16 Thread Stephen Colebourne
In JDK 9, the implementation of Clock has been improved to be better
than millisecond in most cases [1]. However, I've just noticed that
the Javadoc of the "Implementation Note" is now wrong. It says:

"The clock implementation provided here is based on
System.currentTimeMillis(). That method provides little to no
guarantee about the accuracy of the clock. Applications requiring a
more accurate clock must implement this abstract class themselves
using a different external clock, such as an NTP server."

This needs to be updated to indicate that the implementation is no
longer based on System.currentTimeMillis(). Since this is an
"implementation note" I hope that this doesn't affect the
specification, or need too much process.

Is anyone willing to take this up as a JDK 9 bug?

thanks
Stephen

[1] https://bugs.openjdk.java.net/browse/JDK-8068730


RE: [XS] JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent

2017-05-16 Thread Baesken, Matthias
 Sure, I forward it to  serviceability-dev .

-Original Message-
From: Alan Bateman [mailto:alan.bate...@oracle.com] 
Sent: Dienstag, 16. Mai 2017 11:51
To: Baesken, Matthias ; core-libs-dev@openjdk.java.net
Cc: Simonis, Volker 
Subject: Re: [XS] JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent



On 16/05/2017 10:04, Baesken, Matthias wrote:
> Hello, could you please review this small change :
>
> http://cr.openjdk.java.net/~mbaesken/webrevs/8180413/
>
> it fixes a number of places in   jdk.jdwp.agent   where in case of an error 
> it is attempted to write to NULL .
>
> Bug :  JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent
>
>
> https://bugs.openjdk.java.net/browse/JDK-8180413
>
>
Can you bring this to serviceability-dev as that is the mailing list 
where the JDWP agent is maintained?

-Alan


Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Cédric Champeau
Thanks Peter for your thoughts, but I don't think it's as simple as that.
As I explained in my original email, there are multiple ways the
environment variables can be mutated, and it can be done _externally_.
Typically, during a task execution, a JNI call performed by a random native
tool could mutate the environment. That's something we, as a build tool,
have to consider as a use case. It's very unlikely but it can happen. This
means that for the same classloader, the environment can change. And for
performance reasons, we cache classloaders between builds, and reuse them
as much as possible (because classloading is far from being cheap).

2017-05-14 19:51 GMT+02:00 Peter Levart :

> Hi Cedric,
>
> Chiming in with my thoughts...
>
> It's unfortunate that Gradle plugins or libraries used by plugins use
> environment variables at all. I wonder who was the first? Did Gradle
> introduce the feature of passing client environment to the daemon and
> establishing the view of System.getenv for plugins 1st or did libraries
> used by plugins happen to use environment variables using System.getenv and
> Gradle was just kind enough to provide them a dynamic view controlled by
> client?
>
> In the end it doesn't matter. The fact is that System.getenv is part of
> standard Java API and anyone using it should be aware that by doing so,
> they are limiting their software to be (re)configured only by spawning new
> process(es). UNIX environment was not designed to be mutated during the
> course of a long-running process. Maybe just at process startup/setup when
> conditions are under control (i.e. a single running thread) but later, the
> environment is meant to be read only.
>
> Maybe there is a solution for Gradle and othert though. But that solution,
> I think, is not in exposing a "live" view of process environment through
> System.getenv or new methods to "refresh" the view as you are proposing,
> since that would only encourage people to mutate the process environment
> which, as was established on this thread, is not safe in a long-running
> multi-threaded process, which Java processes usually are. Perhaps the
> solution is in extending the System.getenv API with ways to provide
> "custom" views of variables for code that runs in a "container".
>
> Gradle is, among other things, a container for plugins. Is Gradle running
> plugins in a separate ClassLoader? Does it use a separate ClassLoader for
> each "build" which might bring with it a new set of environment variables
> from the client? In such a setup, one could provide a separate set of
> environment variables for each ClassLoader, simply by passing them to the
> ClassLoader constructor. System.getenv would need to be a @CallerSensitive
> method which would return caller's ClassLoader view of variables, if any
> such view was established, or simply the view inherited from the parent
> ClassLoader.
>
> Such would be a "functional" approach, which would keep environment
> variables immutable, but allow child "contexts" to have different views of
> them. Such approach would also play well with idioms like:
>
> class AbcPlugin {
> static final String SOME_SETTING = System.getenv("SOME_SETTING");
>
> ...and would also help other containers (such as app servers) support
> existing libraries that use environment variables to be configured
> differently in different apps deployed in the same VM process.
>
> Just a thought.
>
> Regards, Peter
>
>
>
> On 05/11/2017 09:02 AM, Cédric Champeau wrote:
>
> Thanks for the answers, folks, but I think they are kind of missing the
> point. The fact is that environment variables *are* mutable. Java happens
> to return an immutable view of the environment variables when the VM was
> started, which is not the reality. We cannot trust what `System.geteenv`
> returns. The Javadocs say "current system environment" but it's just not
> true. The method should be called `getEnvWhenTheVMWasStarted`.
>
> We also have the problem that the environment of the client and the daemon
> differ over time, as I explained in the previous email. And it is pretty
> easy to understand that _when the build runs_, we need to get the
> environment variables of the _client_, not the daemon. Imagine something as
> simple as this:
>
> String toolPath = System.getenv('SOMETOOL_HOME')
>
> and imagine that this code runs in the daemon. When the daemon is started,
> there's absolutely no guarantee that this variable is going to exist.
> However, we know that when we're going to execute the build, this variable
> *has* to be defined. Obviously, it's going to be done from the CLI:
>
> $ export SOMETOOL_HOME = ...
> $ ./gradlew run ---> connects to the daemon, passes environment variables,
> mutates those of the daemon, which then executes the build
>
> Similarly, from a *single* CLI/terminal, it's very common to change the
> values of environment variables (because, hey, they are mutable!):
>
> $ export SOMETOOL_HOME = another path I 

Re: Getting a live view of environment variables (Gradle and JDK 9)

2017-05-16 Thread Dalibor Topic
Thanks for the explanation, David. That doesn't sound much more risky than what 
we already do today in getenv. 

Cheers,
Dalibor

--
 Dalibor Topic | Principal Product Manager
Phone: +494089091214 | Mobile: +491737185961


ORACLE Deutschland B.V. & Co. KG | Kühnehöfe 5 | 22761 Hamburg

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher

 Oracle is committed to developing
practices and products that help protect the environment

> On 15. May 2017, at 07:14, David Holmes  wrote:
> 
> Re-sending to core-libs-dev
> 
>> On 13/05/2017 5:56 PM, David Holmes wrote:
>> Hi Dalibor,
>> 
>>> On 12/05/2017 11:28 PM, dalibor topic wrote:
 On 11.05.2017 18:29, Cédric Champeau wrote:
 
 
Unfortunately, they are not safely mutable in multi-threaded
programs on many operating system/libc combinations.
 
 But the problem is less about mutating, that it is about reading: the VM
 returns wrong values at some point, because it _assumes_ that the
 environment variables are not mutated.
>>> 
>>> Right. Assuming that another thread could be simultaneously writing to
>>> the same data structure holding environment variables (char **), reading
>>> itself becomes problematic at such points in time, as you might read a
>>> temporarily corrupted data structure.
>>> 
>>> I guess the question underneath is if there is a safe point in time when
>>> reading the data could be preformed and no concurrent write from JNI
>>> code corrupting the data when it's partially read is possible.
>> 
>> I'm afraid no such safe point guarantee exists at all - even for the
>> initial reading of the process environment on the first call to System
>> getenv(). There could always potentially be some JNI, or other native
>> in-process code, mutating the environ char** at the same time as we
>> first read it in the JVM.
>> 
>> But we're not trying to protect against random concurrent updates in the
>> current scenario, things are more structured:
>> - request comes in with data that says to update certain env vars
>> - JNI code updates the env vars
>> - the daemon's java code (currently) causes the System.getenv map to be
>> updated
>> - the "client" code is executed and reads the env var and sees the right
>> value
>> 
>> There would have to be a caveat on System.getenv(true) if we went that
>> path, that it is up to the user to ensure it is called in as safe a
>> manner as possible having regard to any concurrent updates in their
>> application code and how the environment is managed on a given platform.
>> 
>> Cheers,
>> David
>> -
>> 
>>> cheers,
>>> dalibor topic


Re: [XS] JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent

2017-05-16 Thread Alan Bateman



On 16/05/2017 10:04, Baesken, Matthias wrote:

Hello, could you please review this small change :

http://cr.openjdk.java.net/~mbaesken/webrevs/8180413/

it fixes a number of places in   jdk.jdwp.agent   where in case of an error it 
is attempted to write to NULL .

Bug :  JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent


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


Can you bring this to serviceability-dev as that is the mailing list 
where the JDWP agent is maintained?


-Alan


RE: [XS] JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent

2017-05-16 Thread Langer, Christoph
Hi Matthias,

this definitely makes sense. If 'data' is NULL, one should not access its 
fields, except a hard crash is desired... Reviewed. Don't forget to update the 
copyright year.

Best regards
Christoph

From: Baesken, Matthias
Sent: Dienstag, 16. Mai 2017 11:05
To: core-libs-dev@openjdk.java.net
Cc: Simonis, Volker ; Langer, Christoph 

Subject: [XS] JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent

Hello, could you please review this small change :

http://cr.openjdk.java.net/~mbaesken/webrevs/8180413/

it fixes a number of places in   jdk.jdwp.agent   where in case of an error it 
is attempted to write to NULL .

Bug :  JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent


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


Best regards, Matthias



[XS] JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent

2017-05-16 Thread Baesken, Matthias
Hello, could you please review this small change :

http://cr.openjdk.java.net/~mbaesken/webrevs/8180413/

it fixes a number of places in   jdk.jdwp.agent   where in case of an error it 
is attempted to write to NULL .

Bug :  JDK-8180413 : avoid accessing NULL in jdk.jdwp.agent


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


Best regards, Matthias



RFR 8087307/9, new tests for ServiceLoader updates for module

2017-05-16 Thread Felix Yang

Hi there,

please review the new tests added for ServiceLoader updates for 
module system.


Test bug:

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

Webrev:

http://cr.openjdk.java.net/~xiaofeya/8087307/webrev.00/

Related product Changes:

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

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

Thanks,

Felix




Re: JDK 9 doc-only RFR of 8180353: FileOutputStream documentation does not indicate properly whether files get truncated or not

2017-05-16 Thread Chris Hegarty
Looks good Brian.

-Chris.

> On 16 May 2017, at 02:27, Brian Burkhalter  
> wrote:
> 
> Please review at your convenience.
> 
> Issue:https://bugs.openjdk.java.net/browse/JDK-8180353
> Patch:[1]
> 
> Thanks,
> 
> Brian
> 
> [1] Hg diff
> 
> --- a/src/java.base/share/classes/java/io/FileOutputStream.java
> +++ b/src/java.base/share/classes/java/io/FileOutputStream.java
> @@ -91,6 +91,12 @@
>  * If the file exists but is a directory rather than a regular file, does
>  * not exist but cannot be created, or cannot be opened for any other
>  * reason then a FileNotFoundException is thrown.
> + * 
> + * Invoking this constructor with the parameter {@code name} is 
> equivalent
> + * to invoking the constructor {@link #FileOutputStream(String,boolean)
> + * FileOutputStream(name,append)} with the same {@code String} parameter
> + * {@code name} and the {@code boolean} parameter {@code append} equal to
> + * {@code false}.
>  *
>  * @param  name   the system-dependent filename
>  * @exception  FileNotFoundException  if the file exists but is a 
> directory



Re: RFR 9 (doc) 8180319 : Update Serialization spec to omit obsolete serialver -show and change history

2017-05-16 Thread Chris Hegarty
On 15 May 2017, at 16:58, Roger Riggs  wrote:
> 
> Please review editorial updates to the serialization spec to remove
> obsolete serialver -show option and the obsolete change history.
> 
> Webrev:
>  http://cr.openjdk.java.net/~rriggs/webrev-serialver-noui-8180319/

Looks good to me Roger.

-Chris.


RFR 8u Backport: 8179515: Class java.util.concurrent.ThreadLocalRandom fails to Initialize when using SecurityManager

2017-05-16 Thread David Holmes
The existing code in 8u was a little different to 9, but the new code is 
identical (other than package names):


webrev: http://cr.openjdk.java.net/~dholmes/8179515/webrev.8u/
Bug: https://bugs.openjdk.java.net/browse/JDK-8179515
9 changeset: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/bb4cdc198dc0
9 review thread: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-May/047615.html


Thanks,
David


RFR: JDK-8180202: -XXaltjvm is not working anymore on MacOSX

2017-05-16 Thread Kumar Srinivasan

Hi David,

Please review simple fix for JDK-8180202, recall we hard coded "server",
this is incorrect because the function CheckJvmType will determin
the correct jvmtype based on jvm.cfg *or* path/jvmtype specified by
-XXaltjvm.  IMO all this needs to be cleaned up when jvm.cfg is removed.

Thanks
Kumar

PS: please also approve the removal of the test from the internal 
ProblemList.txt

diff in the JBS issue.

diff --git a/src/java.base/macosx/native/libjli/java_md_macosx.c 
b/src/java.base/macosx/native/libjli/java_md_macosx.c

--- a/src/java.base/macosx/native/libjli/java_md_macosx.c
+++ b/src/java.base/macosx/native/libjli/java_md_macosx.c
@@ -424,7 +424,7 @@
  * macosx client library is built thin, i386 only.
  * 64 bit client requests must load server library
  */
-JLI_Snprintf(jvmpath, jvmpathsize, "%s/lib/server/" JVM_DLL, 
jrepath);
+JLI_Snprintf(jvmpath, jvmpathsize, "%s/lib/%s/" JVM_DLL, 
jrepath, jvmtype);

 }

 JLI_TraceLauncher("Does `%s' exist ... ", jvmpath);