Re: RFR: 8195976: Add JNDI test javax/naming/dns/AttributeTests/GetAny.java

2018-01-22 Thread Alan Bateman
On 23/01/2018 07:01, Chris Yin wrote: Please review the added JNDI test javax/naming/dns/AttributeTests/GetAny.java, thanks You may want to move it to com/sun/jndi/dns so that it's with the other tests for the DNS provider (as there is no javax.naming.dns API). Also I suspect you don't

Re: RFR(XS): 8195824: tools/launcher/HelpFlagsTest.java fails with java.lang.AssertionError

2018-01-22 Thread David Holmes
Hi Goetz, On 23/01/2018 5:36 PM, Lindenmaier, Goetz wrote: Hi, javacpl seems not to have a help message, so it just needs to be excluded from the test. Please review. http://cr.openjdk.java.net/~goetz/wr18/8195824-fixHelpTest2/webrev/ That seems okay. Did you test it on Windows? Also ...

RFR(XS): 8195824: tools/launcher/HelpFlagsTest.java fails with java.lang.AssertionError

2018-01-22 Thread Lindenmaier, Goetz
Hi, javacpl seems not to have a help message, so it just needs to be excluded from the test. Please review. http://cr.openjdk.java.net/~goetz/wr18/8195824-fixHelpTest2/webrev/ (I sent similar mail in reply to RE: JDK 11 RFR of JDK-8195987,,Problem list tools/launcher/HelpFlagsTest.java fails

RE: JDK 11 RFR of JDK-8195987,,Problem list tools/launcher/HelpFlagsTest.java fails on windows

2018-01-22 Thread Lindenmaier, Goetz
Hi, I recommend pushing the real fix instead. javacpl seems not to have a help message, so it just needs to be excluded from the test: http://cr.openjdk.java.net/~goetz/wr18/8195824-fixHelpTest2/webrev/ Best regards, Goetz. > -Original Message- > From: David Holmes

Re: JDK 11 RFR of JDK-8195987,,Problem list tools/launcher/HelpFlagsTest.java fails on windows

2018-01-22 Thread David Holmes
Looks good! I hope this is the last failure we see from this one! Thanks, David On 23/01/2018 4:53 PM, joe darcy wrote: Hello, The test     tools/launcher/HelpFlagsTest.java has been failing on windows (JDK-8195824) since the push for JDK-8195663. The test needs to be problem listed on

RFR: 8195976: Add JNDI test javax/naming/dns/AttributeTests/GetAny.java

2018-01-22 Thread Chris Yin
Please review the added JNDI test javax/naming/dns/AttributeTests/GetAny.java, thanks It’s to cover the case that we can get the attributes of a DNS entry using special qualifiers. This test will use DNSServer to playback dump dns message (which created by DNSTracer when run test against real

JDK 11 RFR of JDK-8195987,,Problem list tools/launcher/HelpFlagsTest.java fails on windows

2018-01-22 Thread joe darcy
Hello, The test     tools/launcher/HelpFlagsTest.java has been failing on windows (JDK-8195824) since the push for JDK-8195663. The test needs to be problem listed on windows until a full fix is developed. Please review the patch below. Cheers, -Joe --- a/test/jdk/ProblemList.txt    Tue

Re: [11] RFR of 8146656: Wrong Months Array for DateFormatSymbols

2018-01-22 Thread Rachna Goel
Hi, Kindly review updated patch for this doc fix: patch: http://cr.openjdk.java.net/%7Ergoel/8146656/webrev.02/ Approved CSR : https://bugs.openjdk.java.net/browse/JDK-8191414 Thanks, Rachna On 20/12/17 10:44 PM, joe darcy wrote: Hi Rachna, I think the revised version with the @implSpec

Re: RFR: JDK-8161348 Several tools/jlink tests failed due to time out

2018-01-22 Thread Andrey Nazarov
> On 22 Jan 2018, at 15:33, Andrey Nazarov wrote: > > > >> On 22 Jan 2018, at 13:07, Joseph D. Darcy wrote: >> >> Hello, >> >> I'm wary of increasing the timeout to 5 minutes. When such tests are run on >> CI servers, the effective

Re: [JDK 11] RFR: 8195096: Exception printed on console with custom LogManager on starting Apache Tomcat

2018-01-22 Thread mandy chung
Hi Daniel, On 1/22/18 8:47 AM, Daniel Fuchs wrote: http://cr.openjdk.java.net/~dfuchs/webrev_8195096/webrev.01/ : I see... so what I think is happening is that in JDK 8, the handlers from ".handlers" where added to the root logger from within addLogger(rootLogger) and because

Re: RFR: JDK-8161348 Several tools/jlink tests failed due to time out

2018-01-22 Thread Andrey Nazarov
> On 22 Jan 2018, at 13:07, Joseph D. Darcy wrote: > > Hello, > > I'm wary of increasing the timeout to 5 minutes. When such tests are run on > CI servers, the effective timeout can be 10 minutes or more given a timeout > factor used for the test run. We can move them

Re: RFR: 8193710 - jcmd -l and jps commands do not list Java processes running in Docker containers

2018-01-22 Thread David Holmes
Thanks Bob. Seems okay. David On 23/01/2018 3:20 AM, Bob Vandette wrote: Please review this change that resolves the detection of Java processes that are running in cgroup based containers. This latest (and hopefully final) update of this fix addresses comments from David Holmes and Mandy

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Brian Burkhalter
On Jan 22, 2018, at 1:22 PM, Alan Bateman wrote: > On 22/01/2018 21:12, Brian Burkhalter wrote: >> >> I can change it. The reason I used that verbiage was to match that of >> read(byte[]): >> >> "The read(b) method for class InputStream has the same effect as: >>

Re: [JDK 11] RFR: 8195096: Exception printed on console with custom LogManager on starting Apache Tomcat

2018-01-22 Thread Jason Mehrens
Daniel, Fantastic! The patch looks good as is. The only thing that sticks out is that both the 'if' and the 'else if' have the same code inside them. That could be refactored a bit. Jason From: core-libs-dev on

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Alan Bateman
On 22/01/2018 21:12, Brian Burkhalter wrote: I can change it. The reason I used that verbiage was to match that of read(byte[]):  "The |read(b)| method for class |InputStream| has the same effect as:  read(b, 0, b.length)" True but the equivalent in readAllBytes won't guarantee that

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Brian Burkhalter
On Jan 22, 2018, at 1:09 PM, Alan Bateman wrote: > On 22/01/2018 20:49, Brian Burkhalter wrote: >> All of the comments included below are addressed in [1]. The difference >> versus webrev.01 are in [2]. The CSR [3] will have to move back to Draft, >> updated, and

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Alan Bateman
On 22/01/2018 20:49, Brian Burkhalter wrote: All of the comments included below are addressed in [1]. The difference versus webrev.01 are in [2]. The CSR [3] will have to move back to Draft, updated, and re-finalized but I will hold off on that until there is a final consensus on the verbiage.

Re: RFR: JDK-8161348 Several tools/jlink tests failed due to time out

2018-01-22 Thread Joseph D. Darcy
Hello, I'm wary of increasing the timeout to 5 minutes. When such tests are run on CI servers, the effective timeout can be 10 minutes or more given a timeout factor used for the test run. Should using 1GB of memory on the @run line have an earlier @requires guard for an amount of memory on

RFR: JDK-8161348 Several tools/jlink tests failed due to time out

2018-01-22 Thread Andrey Nazarov
Hi, please review Jlink tests. I’ve increased default timeout to 5 minutes and skip tests in “-Xcomp” VM mode. CR: http://cr.openjdk.java.net/~anazarov/JDK-8161348/webrev.00/ Bug: https://bugs.openjdk.java.net/browse/JDK-8161348

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Brian Burkhalter
All of the comments included below are addressed in [1]. The difference versus webrev.01 are in [2]. The CSR [3] will have to move back to Draft, updated, and re-finalized but I will hold off on that until there is a final consensus on the verbiage. Thanks, Brian [1]

Re: JDK-6372077: JarFile.getManifest() should handle manifest attribute names up to 70 bytes

2018-01-22 Thread Roger Riggs
Hi Philipp, I'm tending to agree with the suggestion about line length interpretation. To meet OpenJDK IP requirements, please attach the .patch file or include it in the body of the message. Thanks, Roger On 12/18/2017 11:17 PM, Philipp Kunz wrote: Hi Roger, My suggested and also

Re: [JDK 11] RFR: 8195096: Exception printed on console with custom LogManager on starting Apache Tomcat

2018-01-22 Thread Mark Thomas
On 22/01/18 18:16, Daniel Fuchs wrote: > Hi Mark, > > On 22/01/2018 17:33, Mark Thomas wrote: >> Notes: >> - You should see a stack trace if you use Java10 ea38 > > I'm happy to report that I am observing the stack trace > in catalina.out without my fix, and that it is no longer > there with my

Re: RFR: 8193710 - jcmd -l and jps commands do not list Java processes running in Docker containers

2018-01-22 Thread mandy chung
On 1/22/18 9:20 AM, Bob Vandette wrote: : Webrev: http://cr.openjdk.java.net/~bobv/8193710/webrev.02/ Thanks for the update.  Looks good to me. It would be good to add a comment in PlatformSupport::getInstance to explain that PlatformSupport provides the default implementation that can

Re: [JDK 11] RFR: 8195096: Exception printed on console with custom LogManager on starting Apache Tomcat

2018-01-22 Thread Daniel Fuchs
Hi Mark, On 22/01/2018 17:33, Mark Thomas wrote: Notes: - You should see a stack trace if you use Java10 ea38 I'm happy to report that I am observing the stack trace in catalina.out without my fix, and that it is no longer there with my fix applied. - You wont see a stack trace if you build

Re: [JDK 11] RFR: 8195096: Exception printed on console with custom LogManager on starting Apache Tomcat

2018-01-22 Thread Mark Thomas
On 22/01/18 16:47, Daniel Fuchs wrote: > Hi guys, > > Here is my second take at solving the issue. > There's no change in the public API (no CSR, Yay!), > and it shouldn't require any changes on the Tomcat > side. However I haven't tested my changes with Tomcat > yet. > > Mark - would you be

RFR: 8193710 - jcmd -l and jps commands do not list Java processes running in Docker containers

2018-01-22 Thread Bob Vandette
Please review this change that resolves the detection of Java processes that are running in cgroup based containers. This latest (and hopefully final) update of this fix addresses comments from David Holmes and Mandy Chung. Bug: https://bugs.openjdk.java.net/browse/JDK-8193710 Webrev:

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Brian Burkhalter
Certainly some verbiage like that could be added. Going back to last month in the discussion about improving the performance of readAllBytes() I calculated the exact number of bytes allocated [1]. For the initial implementation in that change this was B + L for L <= B N = B +

Re: [JDK 11] RFR: 8195096: Exception printed on console with custom LogManager on starting Apache Tomcat

2018-01-22 Thread Daniel Fuchs
Hi guys, Here is my second take at solving the issue. There's no change in the public API (no CSR, Yay!), and it shouldn't require any changes on the Tomcat side. However I haven't tested my changes with Tomcat yet. Mark - would you be able to help me with that?

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Adam Petcher
The spec of the new method doesn't give me enough information to determine whether it is safe to call it when the value of the length argument is much larger than the number of bytes I expect to actually read. This use case comes up frequently in security libraries, because we have to handle

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Brian Burkhalter
On Jan 22, 2018, at 7:05 AM, Weijun Wang wrote: >> Both methods that throw and not throw have been proposed. But adding two >> methods >> seems like to too much clutter in the API and the methods appear too similar. > > Sorry I wasn't aware of earlier discussions on

Re: Bug in javac release flag?

2018-01-22 Thread Jonathan Gibbons
Forwarding to compiler-dev, which is a better place to discuss this issue. -- Jon On 1/22/18 7:11 AM, Stephen Colebourne wrote: I think I have a problem with the java release flag, reported by a user here: https://github.com/ThreeTen/threeten-extra/issues/91 The particular case that is a

Bug in javac release flag?

2018-01-22 Thread Stephen Colebourne
I think I have a problem with the java release flag, reported by a user here: https://github.com/ThreeTen/threeten-extra/issues/91 The particular case that is a problem are the new methods on java.lang.Math. Java 8 had these methods: floorDiv(long,long) floorDiv(int,int) Java 9 has these

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Weijun Wang
> On Jan 22, 2018, at 10:16 PM, Roger Riggs wrote: > > Hi Max, > > Both methods that throw and not throw have been proposed. But adding two > methods > seems like to too much clutter in the API and the methods appear too similar. Sorry I wasn't aware of earlier

Re: Microsecond support in java.time.Duration/Instant?

2018-01-22 Thread Stephen Colebourne
On 22 January 2018 at 02:58, Kurt Alfred Kluever wrote: > I'm curious how these sets of units were chosen or decided upon? I > understand that the line must be drawn somewhere (or else someone may come > along asking for centisecond support), but I'm curious as to the rational.

Re: Microsecond support in java.time.Duration/Instant?

2018-01-22 Thread Roger Riggs
Hi Kurt, In JSR 310, the thinking was that higher precision was desirable and that adding micros methods just bulked up the API without sufficient payback. At the time there were insufficient use cases to motivate the addition.  If the use is more common than previously thought it is a

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Weijun Wang
> On Jan 22, 2018, at 5:01 PM, Weijun Wang wrote: > > I wonder when readNBytes(n) will be useful if the return value has less than > n bytes. I mean I have seen protocols saying reading rest of the content, or reading N bytes, but never reading at most N bytes. I

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Alan Bateman
On 22/01/2018 08:44, Peter Levart wrote: The delegation to public method (readAllBytes -> readNBytes) should then be documented so that subclasses know that overriding readNBytes, if needed, is sufficient. It doesn't delegate in the latest version of the patch so the documentation is right.

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Weijun Wang
I wonder when readNBytes(n) will be useful if the return value has less than n bytes. > On Jan 22, 2018, at 4:52 PM, Alan Bateman wrote: > > > > On 17/01/2018 16:24, Brian Burkhalter wrote: >> : >> >> A negative value of ‘len’ will now cause an

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Alan Bateman
On 17/01/2018 16:24, Brian Burkhalter wrote: : A negative value of ‘len’ will now cause an IllegalArgumentException instead of an IndexOutOfBoundsException. Also some verbiage has been improved. http://cr.openjdk.java.net/~bpb/8139206/webrev.01/ The updated version looks good. I just wonde

Re: RFR 8139206: Add InputStream readNBytes(int len)

2018-01-22 Thread Peter Levart
Hi, On 01/19/2018 08:49 PM, Roger Riggs wrote: Hi Brian, Looks good, A pre-existing typo:     line 67 "{@code skip()}" should be "{@code skip(*long*)}". Since the public readNBytes suffices for readAllBytes, I would rename the private readAtMostNBytes to readNBytes and avoid the