Clarification of BaseStream.onClose() behavior

[Tagir F. Valeev]

Hello!

Current documentation for BaseStream.onClose() does not specify
explicitly how the stream would behave if additional close handlers
are registered after the stream is consumed or even closed. The
implementation actually allows registering the additional close
handlers after consumption or closure and close() method is
idempotent: newly registered handlers are perfectly called:

 Stream s = Stream.of("content");
 s = s.onClose(() -> System.out.println("A"));
 s.forEach(System.out::println);
 s = s.onClose(() -> System.out.println("B"));
 s.close(); // prints A and B
 s = s.onClose(() -> System.out.println("C"));
 s.close(); // prints C

(removing "s =" produces the same result).

I think if such behavior is intended, it should be explicitly noted in
the specification, as third-party implementations of BaseStream
interface should provide consistent behavior. Or at least it should be
noted that some BaseStream implementations may have idempotent close()
method, so the derived AutoCloseable objects (which own the BaseStream
object) should be aware about this behavior and provide idempotent
close() method as well. Without knowing this one may write:

class MyObject implements AutoCloseable {
  private BaseStream s;
  
  public MyObject(BaseStream s) {
this.s = s;
  }
  
  @Override
  public void close() throws Exception {
if(s != null) {
  s.close();
  s = null;
}
  }
}

Such code closes the stream only once, so newly registered handlers
will not be called if MyObject::close is called the second time.

However, to my opinion the current behavior is weird and should be
changed in order not to allow registering new close handles (throwing
IllegalStateException) when the stream is already closed, or even
better when the stream is linked/consumed. As far as I know currently
in JDK close handlers are registered only for non-consumed stream, so
such change would not break existing JDK code (though may break some
strange third party code). It should be noted that AutoCloseable
interface discourages idempotent close() methods (though not forbids
them).

What do you think?

With best regards,
Tagir Valeev.

Re: RFR: JDK-8144988: Unexpected timezone returned after parsing a date

[Masayoshi Okutsu]

Hi Ramanand,

test/java/text/Format/DateFormat/Bug8141243.java:

  28  * @run main Bug8141243
  29  * @run main/othervm -Djava.locale.providers=COMPAT Bug8141243

"COMPAT" is a new name of "JRE" in JDK 9. It's not supported in 8u. I 
think COMPAT is slightly ignored and that it becomes the same thing as 
line 28. Line 29 should be removed.


Otherwise, the fix looks OK to me.

Masayoshi

On 1/14/2016 9:21 PM, Ramanand Patil wrote:

Hi all,
  
Please review the fix for bug: https://bugs.openjdk.java.net/browse/JDK-8144988
  
Webrev: http://cr.openjdk.java.net/~rpatil/8141243/webrev.00
  
This is basically a backport of JDK9 bug: https://bugs.openjdk.java.net/browse/JDK-8141243
  
JDK9 changeset(for reference): http://hg.openjdk.java.net/jdk9/dev/jdk/rev/a1aa2671f281
  
Reason for the review request is because of:

i) Changes present in ResourceBundleGenerator.java file.
ii)   The patch from JDK9 does not automatically apply as is after using 
unshuffle_patch script. Few paths are adjusted as per the jdk8.
  
Since CLDR became the default locale data in JDK9 leading incompatible behavior with prior releases, the relevant code in ResourceBundleGenerator is also backported in this patch.

Even though JDK8 has CLDR locale provider disabled by default, the code change is done to 
be on safer side for cases where CLDR may be supporting "UTC" ID in the future.
  
  
Regards,

Ramanand.

Re: RFR[9u-dev] 8130425: libjvm crash due to stack overflow in executables with 32k tbss/tdata

[David Holmes]

On 15/01/2016 4:59 AM, Martin Buchholz wrote:

The process grim reaper ends up being the first point of failure since
it tries not to waste the user's memory and it's in a core library,
but in principle it's not special.  I think a more general workaround
would be to add a hotspot flag that would add a memory safety zone to
all threads.  If it's known that TLS is stealing 32k from every
thread's stack, then the flag should ensure that every thread stack is
32k larger.


I think we need a point fix for the process reaper case in the immediate 
term. We can then consider how to better address the general case in the 
medium to long term.



More generally, I was hoping that hotspot would ensure that the -Xss
size was available for actual java stack frames and OS overhead was
added on automatically, but that is hard to implement, so the best
alternative workaround is for users to be able to specify that
additional stack size padding.  Maybe -XX:StackSizeOverhead=32768 ?


Even this is still a band-aid - the user has to experience the problem, 
recognize it, and then figure out the right adjustment to add. Plus if 
any third-party library uses native TLS the requirement could change 
dynamically.


So I'd prefer a new RFE to look at this general issue.

Thanks,
David

Re: RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Wang Weijun]

> On Jan 14, 2016, at 11:00 PM, Magnus Ihse Bursie 
>  wrote:
> 
> On 2015-12-18 15:11, Wang Weijun wrote:
>> Hi Vinnie
>> 
>> I take a look and it includes a change for 
>> src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp in the 
>> Java_sun_security_mscapi_KeyStore_getKeyLength() function.
>> 
>> It seems there is no sun.security.mscapi.KeyStore#getKeyLength on the java 
>> side. Is the function useless now?
> 
> Max,
> 
> Is your intention here that you think the patch should remove the entire  
> Java_sun_security_mscapi_KeyStore_getKeyLength function?

Yes, I hope so.

--Max

> 
> /Magnus
> 
>> 
>> Thanks
>> Max
>> 
>>> On Dec 16, 2015, at 9:50 PM, Magnus Ihse Bursie 
>>>  wrote:
>>> 
>>> There is an interest from the community to build OpenJDK using Visual 
>>> Studio 2015 Community edition.
>>> 
>>> This patch is provided by Timo Kinnunen . I am 
>>> sponsoring this patch.
>>> 
>>> The changes to the source code files are mostly casts to uintptr_t, but 
>>> there are some other changes as well.
>>> 
>>> I'm not quite sure who's the owner of all these files. If I'm missing some 
>>> group, please help me and forward the mail to them.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
>>> WebRev: 
>>> http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01
>>> 
>>> /Magnus
> 

Re: RFR: JDK-8144988: Unexpected timezone returned after parsing a date

[Naoto Sato]

Looks good to me.

Naoto

On 1/14/16 4:21 AM, Ramanand Patil wrote:

Hi all,

Please review the fix for bug: https://bugs.openjdk.java.net/browse/JDK-8144988

Webrev: http://cr.openjdk.java.net/~rpatil/8141243/webrev.00

This is basically a backport of JDK9 bug: 
https://bugs.openjdk.java.net/browse/JDK-8141243

JDK9 changeset(for reference): 
http://hg.openjdk.java.net/jdk9/dev/jdk/rev/a1aa2671f281

Reason for the review request is because of:
i) Changes present in ResourceBundleGenerator.java file.
ii)   The patch from JDK9 does not automatically apply as is after using 
unshuffle_patch script. Few paths are adjusted as per the jdk8.

Since CLDR became the default locale data in JDK9 leading incompatible behavior 
with prior releases, the relevant code in ResourceBundleGenerator is also 
backported in this patch.
Even though JDK8 has CLDR locale provider disabled by default, the code change is done to 
be on safer side for cases where CLDR may be supporting "UTC" ID in the future.


Regards,
Ramanand.

Re: RFR [9]: Remove sun.misc.ClassFileTransformer.

[Alan Bateman]

On 14/01/2016 19:53, Chris Hegarty wrote:

sun.misc.ClassFileTransformer has not been used since its hooks were
removed in JDK 8, see 8009645 [1]. It is now time to remove the class
itself.

hg rm src/java.base/share/classes/sun/misc/ClassFileTransformer.java


Got for it. If you look at the history then you'll see it was completely 
nobbled in JDK 8 with a view to removing.


-Alan

Re: RFR [9]: Remove sun.misc.ClassFileTransformer.

[Mandy Chung]

> On Jan 14, 2016, at 11:53 AM, Chris Hegarty  wrote:
> 
> sun.misc.ClassFileTransformer has not been used since its hooks were
> removed in JDK 8, see 8009645 [1]. It is now time to remove the class
> itself.
> 
> hg rm src/java.base/share/classes/sun/misc/ClassFileTransformer.java 

+1

Mandy

RFR [9]: Remove sun.misc.ClassFileTransformer.

[Chris Hegarty]

sun.misc.ClassFileTransformer has not been used since its hooks were
removed in JDK 8, see 8009645 [1]. It is now time to remove the class
itself.

hg rm src/java.base/share/classes/sun/misc/ClassFileTransformer.java 

-Chris.

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

RE: RFR 8124977 cmdline encoding challenges on Windows

[Martin Sawicki]

Thanks for the feedback. 
Investigating the regression failure.  
We'll get back as soon as we figure this out.  (and yes, we'll run this through 
some localized Windows VMs)

Cheers

-Original Message-
From: Kumar Srinivasan [mailto:kumar.x.sriniva...@oracle.com] 
Sent: Tuesday, January 12, 2016 2:35 PM
To: Martin Sawicki ; Vladimir Shcherbakov 

Cc: core-libs-dev Libs ; Naoto Sato 

Subject: Re: RFR 8124977 cmdline encoding challenges on Windows

Hi Martin, Vladimir,

It was suggested that this patch be tested on localized Windows machines 
and/or
trying with the various Windows native encodings, appreciate if you can 
verify
this as well.

Thanks
Kumar

On 1/11/2016 1:10 PM, Kumar Srinivasan wrote:
> Hi,
>
> Was on vacation, I started to prepare the patch from webrev.04
> for integration. Please note: made some adjustments to your
> patch to pass jcheck, ie. usage of tabs and space at line endings,
> and modifications to Copyright dates.
>
> Also fixed a minor bug on unix replaced JLI_TRUE with JNI_TRUE.
> I have attached a patch to for your reference.
>
> However, there is a regression test failure on Windows,
> jdk/test/tools/launcher/I18NTest.java
>
> ---Test info
> Executed command: C:\mmm\jdk\bin\javac.exe i18nH▒lloWorld.java
>
> Test Output
> javac: file not found: i18nHélloWorld.java
> End test info-
>
> Have you run all the launcher regression tests with this changeset ?
>
> Thanks
> Kumar
>
>> Hi Kumar, just wondering if there are any updates on processing this 
>> submission.
>> Thanks!
>>
>> -Original Message-
>> From: Vladimir Shcherbakov
>> Sent: Wednesday, November 25, 2015 2:38 PM
>> To: Kumar Srinivasan ; Martin Sawicki 
>> 
>> Cc: Kirk Shoop ; core-libs-dev Libs 
>> 
>> Subject: RE: RFR 8124977 cmdline encoding challenges on Windows
>>
>> Hi Kumar,
>>
>> Please find updated webreview here:
>> https://na01.safelinks.protection.outlook.com/?url=http:%2f%2fcr.openjdk.java.net%2f~kshoop%2f8124977%2fwebrev.04%2f&data=01%7C01%7Cmarcins%40microsoft.com%7C13ff309b775c4c019fc308d31ba0c43c%7C72f988bf86f141af91ab2d7cd011db47%7C1&sdata=3hhbO5mNPyTvtrTb4kCR42zsWGPGzDhqnmjpNfwnbIw%3d
>>
>> Thanks,
>> Vladimir.
>>
>> -Original Message-
>> From: Kumar Srinivasan [mailto:kumar.x.sriniva...@oracle.com]
>> Sent: Sunday, November 22, 2015 8:14 AM
>> To: Martin Sawicki 
>> Cc: Kirk Shoop ; Vladimir Shcherbakov 
>> ; core-libs-dev Libs 
>> 
>> Subject: Re: RFR 8124977 cmdline encoding challenges on Windows
>>
>>
>> Hi Martin, et. al.,
>>
>> Sorry for not getting back earlier, I am very busy right now with my 
>> other large commitments for JDK9.
>>
>> I will sponsor this "enhancement/bug fix" sometime in the new year, 
>> meanwhile, there is the changeset  [1] which is likely to cause merge 
>> conflicts, and perhaps logic issues.
>>
>> Thanks
>> Kumar
>>
>> [1] 
>> https://na01.safelinks.protection.outlook.com/?url=http%3a%2f%2fhg.openjdk.java.net%2fjdk9%2fdev%2fjdk%2frev%2f3b201a9ef918&data=01%7c01%7cvlashch%40microsoft.com%7c4d49ae546dba4d29b7be08d2f3589ee1%7c72f988bf86f141af91ab2d7cd011db47%7c1&sdata=I2FKvBn82%2fxhW3D%2fi%2bRWaNOJk7Mg4lt2P0sdzLS%2fT9Q%3d
>>> Hi all
>>> Here's an updated webrev attempting to take into account the various 
>>> pieces of feedback we have received:
>>>
>>> Issue:
>>> https://na01.safelinks.protection.outlook.com/?url=https%3a%2f%2fbugs.
>>> openjdk.java.net%2fbrowse%2fJDK-8124977&data=01%7c01%7cvlashch%40micro
>>> soft.com%7c4d49ae546dba4d29b7be08d2f3589ee1%7c72f988bf86f141af91ab2d7c
>>> d011db47%7c1&sdata=FjmfM%2fnPbWB%2fMsUU8uDzAUo3aPu3zOELVsJO%2fsUIq9E%3
>>> d
>>> Webrev:
>>> https://na01.safelinks.protection.outlook.com/?url=http:%2f%2fcr.openj
>>> dk.java.net%2f~kshoop%2f8124977%2fwebrev.03%2f&data=01%7C01%7Cvlashch%
>>> 40microsoft.com%7C4d49ae546dba4d29b7be08d2f3589ee1%7C72f988bf86f141af9
>>> 1ab2d7cd011db47%7C1&sdata=101HBPar2AZ63GJWyubWH0DiKmNI%2bOxknN667BJnWY
>>> 0%3d
>>>
>>> (Vladimir Shcherbakov is now working on this from our side)
>>>
>>> Looking forward to any other feedback.
>>> Thanks
>>>
>>> -Original Message-
>>> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net] On
>>> Behalf Of Kumar Srinivasan
>>> Sent: Thursday, June 25, 2015 6:26 AM
>>> To: Kirk Shoop (MS OPEN TECH) 
>>> Cc: Valery Kopylov (Akvelon) ; core-libs-dev
>>> Libs 
>>> Subject: Re: RFR 8124977 cmdline encoding challenges on Windows
>>>
>>> Hi Kirk,
>>>
>>> Thanks for proposing this change.
>>>
>>> If you notice all the posix calls are wrapped in JLI_* this gives us 
>>> the ability to use "W" functions.  I almost got it done, several 
>>> years ago, but we upgraded to VS2010 and my work based on VS2003 
>>> keeled over, meanwhile my focus was  "shifted" to something else.
>>>
>>> main.c: is really envisioned to be a stub  compiled by the tool
>>> launchers, like java, javac, javah, jar etc. I prefer to see all the
>>> heavy logic in this file moved to the platform specific file
>>> windows/java_md.*
>>>
>>> For the reason s

Re: RFR[9u-dev] 8130425: libjvm crash due to stack overflow in executables with 32k tbss/tdata

[Martin Buchholz]

The process grim reaper ends up being the first point of failure since
it tries not to waste the user's memory and it's in a core library,
but in principle it's not special.  I think a more general workaround
would be to add a hotspot flag that would add a memory safety zone to
all threads.  If it's known that TLS is stealing 32k from every
thread's stack, then the flag should ensure that every thread stack is
32k larger.

More generally, I was hoping that hotspot would ensure that the -Xss
size was available for actual java stack frames and OS overhead was
added on automatically, but that is hard to implement, so the best
alternative workaround is for users to be able to specify that
additional stack size padding.  Maybe -XX:StackSizeOverhead=32768 ?

Re: RFR [9] 7067728: Remove stopThread default java.policy

[Chris Hegarty]

On 14 Jan 2016, at 17:29, Mandy Chung  wrote:

> 
>> On Jan 14, 2016, at 9:19 AM, Chris Hegarty  wrote:
>> 
>>> There are existing tests whose grants this "stopThread” RuntimePermission 
>>> that may not be needed for the test.  The test policy likely copies that 
>>> from the default system java.policy.  We should update these test policy as 
>>> well.
>> 
>> I do see a few of these, and some will need discussion. Ok if I file a 
>> separate
>> bug on these, they are not directly related to this change, and do still 
>> pass, just
>> that the permission is superfluous.
>> 
> 
> Taking it out from the test policy file should be non-controversial and 
> trivial to verify.  

Right. I was thinking that maybe these tests should be updated to use the new
jtreg machanism, java.security.policy, rather than just removing stopThread? 

> I can see why you prefer to separate the test update from this change and I’m 
> okay.

Thanks. I’ll file a bug on it.

> 
> I would have expected some tests to need modifying here (or other 
> places!).
 
 I haven’t seen any test failures resulting from this change ( not sure
 if that is a good or a bad thing! ).  Though, there were several 
 implementation
 bugs that needed to be resolved before being able to remove default grant.
>>> 
>>> jtreg policy tag overrides the system default security policy with the 
>>> specified file.  Tests that call Thread::stop and tested with security 
>>> manager must have  "stopThread” RuntimePermission set in the test policy.  
>>> jtreg was enhanced to add a new java.security.policy tag to extend the 
>>> system security policy [1].  
>> 
>> Thanks for this explanation. I always get confused with how jtreg supports
>> this.
>> 
>>> Only tests using java.security.policy tag and calling Thread::stop will 
>>> need to be modified.
>> 
>> I can find no such tests.
> 
> That matches what I expect since most of the tests using the new 
> java.security.policy tag are related to deprivileging work and new tests only.

Great.

-Chris.

Re: RFR [9] 7067728: Remove stopThread default java.policy

[Mandy Chung]

> On Jan 14, 2016, at 9:19 AM, Chris Hegarty  wrote:
> 
>> There are existing tests whose grants this "stopThread” RuntimePermission 
>> that may not be needed for the test.  The test policy likely copies that 
>> from the default system java.policy.  We should update these test policy as 
>> well.
> 
> I do see a few of these, and some will need discussion. Ok if I file a 
> separate
> bug on these, they are not directly related to this change, and do still 
> pass, just
> that the permission is superfluous.
> 

Taking it out from the test policy file should be non-controversial and trivial 
to verify.  I can see why you prefer to separate the test update from this 
change and I’m okay.


 I would have expected some tests to need modifying here (or other places!).
>>> 
>>> I haven’t seen any test failures resulting from this change ( not sure
>>> if that is a good or a bad thing! ).  Though, there were several 
>>> implementation
>>> bugs that needed to be resolved before being able to remove default grant.
>> 
>> jtreg policy tag overrides the system default security policy with the 
>> specified file.  Tests that call Thread::stop and tested with security 
>> manager must have  "stopThread” RuntimePermission set in the test policy.  
>> jtreg was enhanced to add a new java.security.policy tag to extend the 
>> system security policy [1].  
> 
> Thanks for this explanation. I always get confused with how jtreg supports
> this.
> 
>> Only tests using java.security.policy tag and calling Thread::stop will need 
>> to be modified.
> 
> I can find no such tests.

That matches what I expect since most of the tests using the new 
java.security.policy tag are related to deprivileging work and new tests only.

Mandy

Re: RFR [9] 7067728: Remove stopThread default java.policy

[Chris Hegarty]

On 14 Jan 2016, at 16:52, Mandy Chung  wrote:

>> On Jan 14, 2016, at 2:05 AM, Chris Hegarty  wrote:
>> 
>> The "stopThread” RuntimePermission is granted by default. The Thread.stop
>> methods have been deprecated for more than 15 years. It seems reasonable,
>> in a major release, to remove the default grant of stopThread.
> 
> +1 to remove "stopThread” RuntimePermission from java.policy.

Thanks for the review Mandy.

> There are existing tests whose grants this "stopThread” RuntimePermission 
> that may not be needed for the test.  The test policy likely copies that from 
> the default system java.policy.  We should update these test policy as well.

I do see a few of these, and some will need discussion. Ok if I file a separate
bug on these, they are not directly related to this change, and do still pass, 
just
that the permission is superfluous.

>>> I would have expected some tests to need modifying here (or other places!).
>> 
>> I haven’t seen any test failures resulting from this change ( not sure
>> if that is a good or a bad thing! ).  Though, there were several 
>> implementation
>> bugs that needed to be resolved before being able to remove default grant.
> 
> jtreg policy tag overrides the system default security policy with the 
> specified file.  Tests that call Thread::stop and tested with security 
> manager must have  "stopThread” RuntimePermission set in the test policy.  
> jtreg was enhanced to add a new java.security.policy tag to extend the system 
> security policy [1].  

Thanks for this explanation. I always get confused with how jtreg supports
this.

> Only tests using java.security.policy tag and calling Thread::stop will need 
> to be modified.

I can find no such tests.

-Chris.

> Mandy
> [1] https://bugs.openjdk.java.net/browse/CODETOOLS-7900898

Re: RFR [9] 7067728: Remove stopThread default java.policy

[Mandy Chung]

> On Jan 14, 2016, at 2:05 AM, Chris Hegarty  wrote:
> 
> The "stopThread” RuntimePermission is granted by default. The Thread.stop
> methods have been deprecated for more than 15 years. It seems reasonable,
> in a major release, to remove the default grant of stopThread.

+1 to remove "stopThread” RuntimePermission from java.policy.

There are existing tests whose grants this "stopThread” RuntimePermission that 
may not be needed for the test.  The test policy likely copies that from the 
default system java.policy.  We should update these test policy as well.


>> 
>> I would have expected some tests to need modifying here (or other places!).
> 
> I haven’t seen any test failures resulting from this change ( not sure
> if that is a good or a bad thing! ).  Though, there were several 
> implementation
> bugs that needed to be resolved before being able to remove default grant.

jtreg policy tag overrides the system default security policy with the 
specified file.  Tests that call Thread::stop and tested with security manager 
must have  "stopThread” RuntimePermission set in the test policy.  jtreg was 
enhanced to add a new java.security.policy tag to extend the system security 
policy [1].  Only tests using java.security.policy tag and calling Thread::stop 
will need to be modified.

Mandy
[1] https://bugs.openjdk.java.net/browse/CODETOOLS-7900898

Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

[Mandy Chung]

> On Jan 13, 2016, at 9:27 PM, Iris Clark  wrote:
> 
> These are the diffs to address both of your comments:
> 
> $ diff Version.java_save Version.java
> 28a29,30
>> import java.security.AccessController;
>> import java.security.PrivilegedAction;
> 154,155d155
> <  * @see  http://openjdk.java.net/jeps/223";>JEP 223: New 
> Version-String Scheme
> <  *
> 268a269,274
>> * @throws  SecurityException
>> *  If a security manager exists and its {@link
>> *  SecurityManager#checkPropertyAccess(String)
>> *  checkPropertyAccess} method does not allow access to the
>> *  system property "java.version"
>> *
> 272,273c278,285
> <   if (current == null)
> <   current = parse(System.getProperty("java.version"));
> ---
>>  if (current == null) {
>>  current = parse(AccessController.doPrivileged(
>>new PrivilegedAction<>() {
>>public String run() {
>>return System.getProperty("java.version");
>>}
>>}));
>>  }

Looks fine.

Thanks
Mandy

RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

[Iris Clark]

Hi, Magnus.

Thanks for taking the time to look at this again.

> I noted that your java source file had a lot of initial tabs instead of 
> spaces. 
> I'm not sure if we have any code guidelines about this, 

jcheck prevents tabs in comments and source.  I remove them, one of my
editors adds them when I make changes.  It's annoying.

They won't be in the checked in source.

Regards,
iris

-Original Message-
From: Magnus Ihse Bursie 
Sent: Thursday, January 14, 2016 3:38 AM
To: Iris Clark; Joe Darcy; Mandy Chung; Roger Riggs; Alan Bateman
Cc: core-libs-dev@openjdk.java.net; verona-...@openjdk.java.net
Subject: Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

On 2016-01-11 22:44, Iris Clark wrote:
> Hi, Joe, Roger, Alan, Magnus, and Mandy.
>
> At the end of December (shortly before the Christmas/Winter break and 
> my vacation), I provided responses to your messages and an updated 
> webrev:
>
>http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
>
> I didn't hear from anybody, so I'd like to optimistically assume that 
> you were satisfied.  Is that correct?

I'm basically happy. As other reviewers, I think the regexes are a bit tough to 
parse (regexes always are), and some additional comments on the effect would be 
appreciated. Thanh Hong Dai had some good suggestions as well on how to improve 
readability.

I noted that your java source file had a lot of initial tabs instead of spaces. 
I'm not sure if we have any code guidelines about this, but I have not 
encountered that in the JDK source base before, so I'd recommend you turn them 
into spaces.

/Magnus


>
> For you convenience, here's a reference to the December and November
> threads:
>
>  
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.html
>  
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036
> 904.html
>
> I'd like to wrap up this work for the initial implementation of 
> jdk.Version soon.
>
> Regards,
> iris
>
> -Original Message-
> From: Iris Clark
> Sent: Friday, December 18, 2015 1:55 PM
> To: Joe Darcy; Mandy Chung
> Cc: core-libs-dev@openjdk.java.net; verona-...@openjdk.java.net
> Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion
>
> Hi, Joe.
>
> Thanks for the review comments.
>
  http://cr.openjdk.java.net/~irisa/verona/8072379/webrev.1/
>> Is the intention that downstream JDK distributions, such as IcedTea, 
>> whether based on OpenJDK or otherwise, would provide their own 
>> specialization of the jdk.Version class?
> No.  Downstream users may provide their own specialization, but there is no 
> requirement that they must do so.
>
> jdk.OracleVersion has been removed.  The functionality it provided, access to 
> the fourth element of the version number, is trivially accessed by existing 
> methods in jdk.Version.
>
> I've filed the following bug to address the more general use case:
>
>  8145793:  Provide vendor-specific interpretation of a JDK version string
>  https://bugs.openjdk.java.net/browse/JDK-8145793
>
>> The equals / hashCode behavior of Version and OracleVersion is bit 
>> surprising and I think somewhat underspecified given the possibility 
>> of defining subclasses.
> Given the above regarding downstream use, I've make jdk.Version final and the 
> constructor is now private.  Oracle or any other JDK distribution wishing to 
> impose additional semantics to the version string interpretation will need to 
> do so via encapsulation rather than inheritance.
>
>> How is this API supposed to behave if the component version strings 
>> have a numerical value greater than Integer.MAX_VALUE?
>  From the specification of the only instance method, Version.parse():
>
>   * @throws  NumberFormatException
>   *  If an element of the version number or the build number 
> cannot
>   *  be represented as an {@link Integer}
>
>> Was using Longs to record numerical versions rather than Integers 
>> considered?
> Yes.  In an informal poll conducted during implementation, it was thought 
> that 2^32 bits would be more than sufficient to represent the components of 
> typical version numbers.
>
> An updated webrev is forthcoming.
>
> Thanks,
> iris
>
> PS:  I believe that we are both out the week of 21 December, so I don't 
> expect to hear back from you until the New Year.

Re: RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Magnus Ihse Bursie]

On 2016-01-05 03:25, Kim Barrett wrote:

On Dec 18, 2015, at 7:41 PM, Kim Barrett  wrote:

On Dec 16, 2015, at 8:50 AM, Magnus Ihse Bursie  
wrote:

Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01

/Magnus

I only looked at hotspot files.

[…]
There are a couple of "TBD"s below that I need to spend more time on.


Back from holiday, and here’s my comments on those two TBDs


Kim,

Thank you for your feedback.

Since I'm only the sponsor of this patch, not the developer, I'll let 
Timo answer your questions and discuss his choices.


/Magnus



--
hotspot/agent/src/share/native/sadis.c
  96   return (int)strlen(buf);

The only call to the (file-scoped) getLastErrorString function is
Java_sun_jvm_hotspot_asm_Disassembler_load_1library, which ignores the
result.  It would be better to change the return type of
getLastErrorString to void and update the return statements
accordingly.

--
hotspot/src/share/vm/adlc/arena.hpp
  74   // Usual (non-placement) deallocation function to allow placement delete 
use size_t
  75   // See 3.7.4.2 [basic.stc.dynamic.deallocation] paragraph 2.
  76   void  operator delete(void* p);

and associated code in the .cpp file.

I think this is another C++11 change:
http://open-std.org/JTC1/SC22/WG21/docs/cwg_defects.html#429

I think the proposed code change is OK, although the comment is
problematic: [basic.stc.dynamic.deallocation] is C++03 3.7.3.2 and
C++11 3.7.4.2.  Also, the standard change that leads to this code
change is in C++11 5.3.4 [expr.new] paragraph 20 (C++02 p 19).

Also, I *think* with the addition of the one-arg operator delete we
don't actually use the two-arg form.  If so, then I suggest removing
it and the proposed new comment for the one-arg form.

--

Re: RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Magnus Ihse Bursie]

On 2015-12-18 15:11, Wang Weijun wrote:

Hi Vinnie

I take a look and it includes a change for 
src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp in the 
Java_sun_security_mscapi_KeyStore_getKeyLength() function.

It seems there is no sun.security.mscapi.KeyStore#getKeyLength on the java 
side. Is the function useless now?


Max,

Is your intention here that you think the patch should remove the 
entire  Java_sun_security_mscapi_KeyStore_getKeyLength function?


/Magnus



Thanks
Max


On Dec 16, 2015, at 9:50 PM, Magnus Ihse Bursie  
wrote:

There is an interest from the community to build OpenJDK using Visual Studio 
2015 Community edition.

This patch is provided by Timo Kinnunen . I am 
sponsoring this patch.

The changes to the source code files are mostly casts to uintptr_t, but there 
are some other changes as well.

I'm not quite sure who's the owner of all these files. If I'm missing some 
group, please help me and forward the mail to them.

Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01

/Magnus

Re: RFR [9] 7067728: Remove stopThread default java.policy

[Sean Mullan]

Looks good to me.

--Sean

On 01/14/2016 05:05 AM, Chris Hegarty wrote:

The "stopThread” RuntimePermission is granted by default. The Thread.stop
methods have been deprecated for more than 15 years. It seems reasonable,
in a major release, to remove the default grant of stopThread.

diff --git a/src/java.base/share/conf/security/java.policy 
b/src/java.base/share/conf/security/java.policy
--- a/src/java.base/share/conf/security/java.policy
+++ b/src/java.base/share/conf/security/java.policy
@@ -94,17 +94,6 @@
  // default permissions granted to all domains

  grant {
-// Allows any thread to stop itself using the java.lang.Thread.stop()
-// method that takes no argument.
-// Note that this permission is granted by default only to remain
-// backwards compatible.
-// It is strongly recommended that you either remove this permission
-// from this policy file or further restrict it to code sources
-// that you specify, because Thread.stop() is potentially unsafe.
-// See the API specification of java.lang.Thread.stop() for more
-// information.
-permission java.lang.RuntimePermission "stopThread";
-

As a result of this change, untrusted code calling Thread.stop will throw a
SecurityException, as it will not have the required permission. The solution,
from the deployers perspective, is to add "stopThread” RuntimePermission
to the policy file.

-Chris.

Re: RFR(S): 8147078: MethodHandles.catchException does not enforce Throwable subtype

[Paul Sandoz]

> On 14 Jan 2016, at 12:22, Michael Haupt  wrote:
> 
> Dear all,
> 
> please review this fix.
> Bug: https://bugs.openjdk.java.net/browse/JDK-8147078
> Webrev: http://cr.openjdk.java.net/~mhaupt/8147078/webrev.00
> 
> Unlike MethodHandles.throwException, MethodHandles.catchException does not 
> enforce that the passed exception type is indeed a subtype of Throwable. The 
> change adds the corresponding check, and a test.
> 

Erasure bites! +1

Paul.

RFR: JDK-8144988: Unexpected timezone returned after parsing a date

[Ramanand Patil]

Hi all,
 
Please review the fix for bug: https://bugs.openjdk.java.net/browse/JDK-8144988
 
Webrev: http://cr.openjdk.java.net/~rpatil/8141243/webrev.00
 
This is basically a backport of JDK9 bug: 
https://bugs.openjdk.java.net/browse/JDK-8141243
 
JDK9 changeset(for reference): 
http://hg.openjdk.java.net/jdk9/dev/jdk/rev/a1aa2671f281
 
Reason for the review request is because of: 
i) Changes present in ResourceBundleGenerator.java file.
ii)   The patch from JDK9 does not automatically apply as is after using 
unshuffle_patch script. Few paths are adjusted as per the jdk8.
 
Since CLDR became the default locale data in JDK9 leading incompatible behavior 
with prior releases, the relevant code in ResourceBundleGenerator is also 
backported in this patch.
Even though JDK8 has CLDR locale provider disabled by default, the code change 
is done to be on safer side for cases where CLDR may be supporting "UTC" ID in 
the future.
 
 
Regards,
Ramanand.

Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

[Magnus Ihse Bursie]

On 2016-01-11 22:44, Iris Clark wrote:

Hi, Joe, Roger, Alan, Magnus, and Mandy.

At the end of December (shortly before the Christmas/Winter
break and my vacation), I provided responses to your messages
and an updated webrev:

   http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/

I didn't hear from anybody, so I'd like to optimistically assume
that you were satisfied.  Is that correct?


I'm basically happy. As other reviewers, I think the regexes are a bit 
tough to parse (regexes always are), and some additional comments on the 
effect would be appreciated. Thanh Hong Dai had some good suggestions as 
well on how to improve readability.


I noted that your java source file had a lot of initial tabs instead of 
spaces. I'm not sure if we have any code guidelines about this, but I 
have not encountered that in the JDK source base before, so I'd 
recommend you turn them into spaces.


/Magnus




For you convenience, here's a reference to the December and November
threads:

 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.html
 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036904.html

I'd like to wrap up this work for the initial implementation of
jdk.Version soon.

Regards,
iris

-Original Message-
From: Iris Clark
Sent: Friday, December 18, 2015 1:55 PM
To: Joe Darcy; Mandy Chung
Cc: core-libs-dev@openjdk.java.net; verona-...@openjdk.java.net
Subject: RE: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

Hi, Joe.

Thanks for the review comments.


 http://cr.openjdk.java.net/~irisa/verona/8072379/webrev.1/

Is the intention that downstream JDK distributions, such as IcedTea,
whether based on OpenJDK or otherwise, would provide their own
specialization of the jdk.Version class?

No.  Downstream users may provide their own specialization, but there is no 
requirement that they must do so.

jdk.OracleVersion has been removed.  The functionality it provided, access to 
the fourth element of the version number, is trivially accessed by existing 
methods in jdk.Version.

I've filed the following bug to address the more general use case:

 8145793:  Provide vendor-specific interpretation of a JDK version string
 https://bugs.openjdk.java.net/browse/JDK-8145793


The equals / hashCode behavior of Version and OracleVersion is bit
surprising and I think somewhat underspecified given the possibility
of defining subclasses.

Given the above regarding downstream use, I've make jdk.Version final and the 
constructor is now private.  Oracle or any other JDK distribution wishing to 
impose additional semantics to the version string interpretation will need to 
do so via encapsulation rather than inheritance.


How is this API supposed to behave if the component version strings
have a numerical value greater than Integer.MAX_VALUE?

 From the specification of the only instance method, Version.parse():

  * @throws  NumberFormatException
  *  If an element of the version number or the build number cannot
  *  be represented as an {@link Integer}


Was using Longs to record numerical versions rather than Integers
considered?

Yes.  In an informal poll conducted during implementation, it was thought that 
2^32 bits would be more than sufficient to represent the components of typical 
version numbers.

An updated webrev is forthcoming.

Thanks,
iris

PS:  I believe that we are both out the week of 21 December, so I don't expect 
to hear back from you until the New Year.

Re: RFR(S): 8147078: MethodHandles.catchException does not enforce Throwable subtype

[Sundararajan Athijegannathan]

+1

-Sundar

On 1/14/2016 4:52 PM, Michael Haupt wrote:

Dear all,

please review this fix.
Bug: https://bugs.openjdk.java.net/browse/JDK-8147078
Webrev: http://cr.openjdk.java.net/~mhaupt/8147078/webrev.00

Unlike MethodHandles.throwException, MethodHandles.catchException does not 
enforce that the passed exception type is indeed a subtype of Throwable. The 
change adds the corresponding check, and a test.

Thanks,

Michael

RFR(S): 8147078: MethodHandles.catchException does not enforce Throwable subtype

[Michael Haupt]

Dear all,

please review this fix.
Bug: https://bugs.openjdk.java.net/browse/JDK-8147078
Webrev: http://cr.openjdk.java.net/~mhaupt/8147078/webrev.00

Unlike MethodHandles.throwException, MethodHandles.catchException does not 
enforce that the passed exception type is indeed a subtype of Throwable. The 
change adds the corresponding check, and a test.

Thanks,

Michael

-- 

 
Dr. Michael Haupt | Principal Member of Technical Staff
Phone: +49 331 200 7277 | Fax: +49 331 200 7561
Oracle Java Platform Group | LangTools Team | Nashorn
Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany
  Oracle is committed to developing 
practices and products that help protect the environment

Re: RFR [9] 7067728: Remove stopThread default java.policy

[Chris Hegarty]

On 14 Jan 2016, at 10:37, David Holmes  wrote:

> Hi Chris,
> 
> I would have expected some tests to need modifying here (or other places!).

I haven’t seen any test failures resulting from this change ( not sure
if that is a good or a bad thing! ).  Though, there were several implementation
bugs that needed to be resolved before being able to remove default grant.

-Chris.

> David
> 
> On 14/01/2016 8:05 PM, Chris Hegarty wrote:
>> The "stopThread” RuntimePermission is granted by default. The Thread.stop
>> methods have been deprecated for more than 15 years. It seems reasonable,
>> in a major release, to remove the default grant of stopThread.
>> 
>> diff --git a/src/java.base/share/conf/security/java.policy 
>> b/src/java.base/share/conf/security/java.policy
>> --- a/src/java.base/share/conf/security/java.policy
>> +++ b/src/java.base/share/conf/security/java.policy
>> @@ -94,17 +94,6 @@
>>  // default permissions granted to all domains
>> 
>>  grant {
>> -// Allows any thread to stop itself using the 
>> java.lang.Thread.stop()
>> -// method that takes no argument.
>> -// Note that this permission is granted by default only to remain
>> -// backwards compatible.
>> -// It is strongly recommended that you either remove this permission
>> -// from this policy file or further restrict it to code sources
>> -// that you specify, because Thread.stop() is potentially unsafe.
>> -// See the API specification of java.lang.Thread.stop() for more
>> -// information.
>> -permission java.lang.RuntimePermission "stopThread";
>> -
>> 
>> As a result of this change, untrusted code calling Thread.stop will throw a
>> SecurityException, as it will not have the required permission. The solution,
>> from the deployers perspective, is to add "stopThread” RuntimePermission
>> to the policy file.
>> 
>> -Chris.
>> 

Re: RFR [9] 7067728: Remove stopThread default java.policy

[David Holmes]

Hi Chris,

I would have expected some tests to need modifying here (or other places!).

David

On 14/01/2016 8:05 PM, Chris Hegarty wrote:

The "stopThread” RuntimePermission is granted by default. The Thread.stop
methods have been deprecated for more than 15 years. It seems reasonable,
in a major release, to remove the default grant of stopThread.

diff --git a/src/java.base/share/conf/security/java.policy 
b/src/java.base/share/conf/security/java.policy
--- a/src/java.base/share/conf/security/java.policy
+++ b/src/java.base/share/conf/security/java.policy
@@ -94,17 +94,6 @@
  // default permissions granted to all domains

  grant {
-// Allows any thread to stop itself using the java.lang.Thread.stop()
-// method that takes no argument.
-// Note that this permission is granted by default only to remain
-// backwards compatible.
-// It is strongly recommended that you either remove this permission
-// from this policy file or further restrict it to code sources
-// that you specify, because Thread.stop() is potentially unsafe.
-// See the API specification of java.lang.Thread.stop() for more
-// information.
-permission java.lang.RuntimePermission "stopThread";
-

As a result of this change, untrusted code calling Thread.stop will throw a
SecurityException, as it will not have the required permission. The solution,
from the deployers perspective, is to add "stopThread” RuntimePermission
to the policy file.

-Chris.

Re: RFR [9] 7067728: Remove stopThread default java.policy

[Alan Bateman]

On 14/01/2016 10:05, Chris Hegarty wrote:

The "stopThread” RuntimePermission is granted by default. The Thread.stop
methods have been deprecated for more than 15 years. It seems reasonable,
in a major release, to remove the default grant of stopThread.
This looks okay to me, we should have dropped granting this permission 
by default a long time ago.


-Alan

RFR [9] 7067728: Remove stopThread default java.policy

[Chris Hegarty]

The "stopThread” RuntimePermission is granted by default. The Thread.stop
methods have been deprecated for more than 15 years. It seems reasonable,
in a major release, to remove the default grant of stopThread.
 
diff --git a/src/java.base/share/conf/security/java.policy 
b/src/java.base/share/conf/security/java.policy
--- a/src/java.base/share/conf/security/java.policy
+++ b/src/java.base/share/conf/security/java.policy
@@ -94,17 +94,6 @@
 // default permissions granted to all domains
 
 grant {
-// Allows any thread to stop itself using the java.lang.Thread.stop()
-// method that takes no argument.
-// Note that this permission is granted by default only to remain
-// backwards compatible.
-// It is strongly recommended that you either remove this permission
-// from this policy file or further restrict it to code sources
-// that you specify, because Thread.stop() is potentially unsafe.
-// See the API specification of java.lang.Thread.stop() for more
-// information.
-permission java.lang.RuntimePermission "stopThread";
-
 
As a result of this change, untrusted code calling Thread.stop will throw a
SecurityException, as it will not have the required permission. The solution,
from the deployers perspective, is to add "stopThread” RuntimePermission
to the policy file.

-Chris. 

Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

[Alan Bateman]

On 13/01/2016 21:54, Iris Clark wrote:

:
This diff has been applied to modules.xml:

This looks okay.



:

When this came up earlier, I filed this bug to track finding
a more appropriate module for jdk.Version:

 8144062: Determine appropriate module for jdk.Version
 https://bugs.openjdk.java.net/browse/JDK-8144062


Thanks, I've changed this to be a P2 as we will need to decide on this soon.

-Alan.

Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

[Victor Polischuk]

Hi Iris,

Do you consider an option to let community reuse JDK versioning style for their 
own purposes. Probably defining an interface with basic default methods which 
can be extended by various libraries to provide unified way to gather version 
information from MANIFEST.MF, ClassLoader's jars and simply to allow developers 
inventing one wheel lesser.
  
/Victor

 --- Original message ---
From: "Iris Clark" 
Date: 11 January 2016, 23:45:56
 
> Hi, Joe, Roger, Alan, Magnus, and Mandy.
> 
> At the end of December (shortly before the Christmas/Winter 
> break and my vacation), I provided responses to your messages 
> and an updated webrev:
> 
> http://cr.openjdk.java.net/~iris/verona/8072379/webrev.2/
> 
> I didn't hear from anybody, so I'd like to optimistically assume
> that you were satisfied.  Is that correct? 
> 
> For you convenience, here's a reference to the December and November
> threads:
> 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-December/037062.html
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-November/036904.html
> 
> I'd like to wrap up this work for the initial implementation of 
> jdk.Version soon.  
> 
> Regards,
> iris
 
 

Re: RFR: 8072379: Implement jdk.Version and jdk.OracleVersion

[Dmitry Samersoff]

Iris,

Did you consider to split version string into array of groups first
(using String.split()), then validate each group separately?

It may make the code better readable and more resilient to possible
future changes.

-Dmitry


On 2015-11-25 04:54, Iris Clark wrote:
> Hi.
> 
> Please review the new classes jdk.Version and jdk.OracleVersion.  These are
> simple Java APIs to parse, validate, and compare version numbers.
> 
>   Bug
> 
> 8072379: Implement jdk.Version and jdk.OracleVersion
> https://bugs.openjdk.java.net/browse/JDK-8072379
> 
>   Webrev
> 
> http://cr.openjdk.java.net/~iris/verona/8072379/webrev.1/
> 
>   JavaDoc
> 
> http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/Version.html
> 
> http://cr.openjdk.java.net/~iris/verona/8072379/doc.1/jdk/OracleVersion.html
> 
> The .java files are predominantly javadoc. The code is relatively
> straight-forward.
> 
> jdk.Version is the representation of the JDK version string as described in
> JEP 223 ([0], 8061493).  The javadoc is largely taken from the description
> section in the JEP.  The API is described in the "API" section.
> 
> jdk.OracleVersion extends jdk.Version and is the representation of the Oracle
> JDK version string.  Its only purpose is to interpret the fourth element of
> the version number as a patch release.
> 
> There are some minor discrepancies between the implementation and the JEP.
> The JEP will need to be updated.  The most notable is the name of the package
> ("jdk" vs. the original "jdk.util").  The rename was recommended by Mark.
> 
> Thanks,
> iris
>  
> [0] http://openjdk.java.net/jeps/223
> 


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