Re: JDK 9 RFR of JDK-8132548: java/lang/ThreadGroup/Stop.java fails with "RuntimeException: Failure"

2016-07-07 Thread Amy Lu
Yes, but I just thought that for a test that testing a deprecated (since 1.2) API, and failed with very very low frequency (actually, only one time during the years), we might not want to spend much effort on a big change, like rewrite with CountDownLatch :-) Thanks, Amy On 7/8/16 2:36 PM, M

Re: JDK 9 RFR of JDK-8132548: java/lang/ThreadGroup/Stop.java fails with "RuntimeException: Failure"

2016-07-07 Thread Martin Buchholz
CountDownLatch is a better way of waiting for events, like for the two threads to be started and for Thread.stop to have been called. The test should ensure that ThreadDeath is indeed thrown. If the threads in the group notify the main thread via a latch when they catch ThreadDeath, then all the

Re: JEP 293: Guidelines for JDK Command-Line Tool Options

2016-07-07 Thread Dan Smith
A suggestion for clarifying the text: I was confused about whether "may" refers to choices that the tool developer is allowed to make, or choices that the end user is allowed to make. For example: "Options can require an argument to be provided. For a long-form options, the argument may be sepa

Re: JDK 9 RFR of JDK-8132548: java/lang/ThreadGroup/Stop.java fails with "RuntimeException: Failure"

2016-07-07 Thread Amy Lu
Thank you Joe for your review. The intent is to give it more chance "for the thread group stop to be issued", not to extend the whole test execution timeout. I updated the webrev to make this in a retry, limit to 5 times of retry: http://cr.openjdk.java.net/~amlu/8132548/webrev.01/ Thanks, Am

RFR: 8132379: -J options can cause crash or "Warning: app args parsing error passing arguments as-is"

2016-07-07 Thread Henry Jen
Hi, Please review a fix for JDK-8132379, the fix is to build matching index to the original arguments for the application arguments, used later for the sanity check and wildcard expansion. The fix is specific to Windows platform. -J prefix used by launcher-based tools such as javac need to be f

Re: JDK 9 RFR of JDK-8132548: java/lang/ThreadGroup/Stop.java fails with "RuntimeException: Failure"

2016-07-07 Thread joe darcy
Hi Amy, I'm a bit uncomfortable with the fix as-is. Rather than hard-coding sleep values, if sleep values are needed I think it is a better practice to use ones that are scaled with the jtreg timeout factors, etc. used to run the tests. Please instead use something like the adjustTimeout meth

JDK 9 RFR of JDK-8132548: java/lang/ThreadGroup/Stop.java fails with "RuntimeException: Failure"

2016-07-07 Thread Amy Lu
Please review this trivial fix for test:java/lang/ThreadGroup/Stop.java Though this is a test for a deprecated API, failed with very very low frequency and hard to reproduce (I got no luck to reproduce it), I’d like to patch it as suggested: extend the sleep in the main thread from one second

Re: RFR: jsr166 jdk9 integration wave 7

2016-07-07 Thread Doug Lea
On 07/07/2016 10:06 AM, Peter Levart wrote: Why the naming wasn't more like the following: No matter which conventions you choose here, some people will be unhappy or confused. The current scheme seems to make the current users of both Unsafe and AtomicX least unhappy or confused. VarHandle.

RFR: 9: 8161011: Mark test/java/rmi/activation/ActivationGroup/downloadActivationGroup/DownloadActivationGroup.java as itnermittent

2016-07-07 Thread Rajan Halade
This test is seen to fail intermittently due to JDK-8085192. Please review following patch which marks it as intermittent. diff -r 848c94ca6394 test/java/rmi/activation/ActivationGroup/downloadActivationGroup/DownloadActivationGroup.java --- a/test/java/rmi/activation/ActivationGroup/downloadA

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread John Rose
On Jul 7, 2016, at 11:05 AM, Martin Buchholz wrote: > private static final jdk.internal.misc.Unsafe > java.util.concurrent.ConcurrentHashMap.U Unless the security manager is turned on, you can do setAcc to pick up all sorts of private fields. As Alan points out, it would be good to tighten th

RFR: 8158295 Add a multi-release jar validation mechanism to jar tool

2016-07-07 Thread Steve Drach
Hi, Please review the following: webrev: http://cr.openjdk.java.net/~sdrach/8158295/webrev.01/ issue: https://bugs.openjdk.java.net/browse/JDK-8158295 This changeset adds a multi-release

Re: Advice on cross-repo change: JDK-8160997

2016-07-07 Thread Coleen Phillimore
Hi, You can send this to core-libs-dev@openjdk.java.net and hotspot-...@openjdk.java.net The sponsor should be from the runtime group. I don't know this well enough, so I'm going to point you to Dan (and Jerry, except Jerry's not a committer). To run JPRT don't forget to use -testset hot

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Mandy Chung
Hi Volker, Thanks for adding a new test for it. Nit: can you wrap the long lines. As for the bad version, one possible change is to add assert Runtime.Version.parse(versionNumber) in parseVersionNumbers method and add -esa in @run tag. It’d be good to convert this to testng test where the da

Re: RFR JDK-8133170 : Deflater.deflate with small output buffers fails

2016-07-07 Thread Brian Burkhalter
This looks reasonable. +1. Brian On Jul 7, 2016, at 12:13 PM, Xueming Shen wrote: > Please help review the doc only change for this issue. > > issue: https://bugs.openjdk.java.net/browse/JDK-8133170 > webrev: http://cr.openjdk.java.net/~sherman/8133170/webrev > > As explained in the zlib.h (q

RFR JDK-8133170 : Deflater.deflate with small output buffers fails

2016-07-07 Thread Xueming Shen
Hi, Please help review the doc only change for this issue. issue: https://bugs.openjdk.java.net/browse/JDK-8133170 webrev: http://cr.openjdk.java.net/~sherman/8133170/webrev As explained in the zlib.h (quoted below), there are 5 bytes of flush marker being output to the output buffer repeatedly

JEP 293: Guidelines for JDK Command-Line Tool Options

2016-07-07 Thread mark . reinhold
New JEP Candidate: http://openjdk.java.net/jeps/293 - Mark

Re: RFR: 8153044: Allow a property to control location of tzdb.dat file

2016-07-07 Thread Seán Coffey
Thanks for the review Roger. As per other thread, I'm going to drop this feature for now. The TzdbZoneRulesProvider Object is already constructed in a doPrivileged block, so I believe it was covered on the security manager scenario. Regards, Sean. On 07/07/2016 16:32, Roger Riggs wrote: Hi

Re: RFR: 8153044: Allow a property to control location of tzdb.dat file

2016-07-07 Thread Seán Coffey
I'll (reluctantly) close this out then. I do think it was a convenient and low-maintenance feature that would allow end users the ability to bounce applications in an ordered fashion in order to pick up new tzdata rules. I'll investigate the upgradeable module approach. Regards, Sean. On 07/0

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Alan Bateman
On 07/07/2016 19:05, Martin Buchholz wrote: When jdk9 is released, an army of white, black, grey, and red hats will try to keep their old Unsafe hacks alive and maybe get their hands on a jdk.internal.misc.Unsafe. I assume these Unsafe usages are sun.misc.Unsafe so they should continue to work.

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Martin Buchholz
When jdk9 is released, an army of white, black, grey, and red hats will try to keep their old Unsafe hacks alive and maybe get their hands on a jdk.internal.misc.Unsafe. Here's some code that tries to do that. The call to setAccessible succeeds! And the code succeeds in getting hold of jdk.interna

Re: Improvements to BigDecimal(double, MathContext) constructor

2016-07-07 Thread Claes Redestad
On 2016-07-07 18:18, John Platts wrote: Here is the current significand normalization loop below in the BigDecimal(double, MathContext) constructor: // Normalize while ((significand & 1) == 0) { // i.e., significand is even significand >>= 1; exponent+

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Claes Redestad
On 2016-07-07 18:08, Volker Simonis wrote: Not sure how error checking could or should be improved: >VersionProps.parseVersionNumbers(String) will throw a NFE on most malformed >data, technically an IllegalArgumentException. Same thing would happen if >you ran an invalid string through Runtime.

Improvements to BigDecimal(double, MathContext) constructor

2016-07-07 Thread John Platts
Here is the current significand normalization loop below in the  BigDecimal(double, MathContext) constructor:         // Normalize         while ((significand & 1) == 0) { // i.e., significand is even             significand >>= 1;             exponent++;         } Here is a better way to normaliz

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Volker Simonis
On Thu, Jul 7, 2016 at 5:33 PM, Claes Redestad wrote: > Hi Volker, > > On 2016-07-07 15:59, Volker Simonis wrote: >> >> Hi, >> >> can I please have a review for the following change which makes >> VersionProps.versionNumbers() testable and the corresponding >> regression test: >> >> http://cr.open

Re: RFR 8054213: Class name repeated in output of Type.toString()

2016-07-07 Thread Svetlana Nikandrova
Hello all, seems like Joe is busy right now, so meanwhile I'll be happy to hear other opinions on this topic. Thank you, Svetlana On 30.06.2016 19:44, Svetlana Nikandrova wrote: Hello Joe, thank you for your advice! GenericStringTest looks really good with annotations. I refactored my test

Re: RFR: 8153044: Allow a property to control location of tzdb.dat file

2016-07-07 Thread mark . reinhold
2016/7/7 8:02:21 -0700, scolebou...@joda.org: > I won't block this change, but I remain unenthused. There are other > ways to control the time-zone data, and since tzdb.dat is currently a > private format file that is part of the internals of the JDK, it seems > odd to be exposing it. I agree. Th

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Claes Redestad
Hi Volker, On 2016-07-07 15:59, Volker Simonis wrote: Hi, can I please have a review for the following change which makes VersionProps.versionNumbers() testable and the corresponding regression test: http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564/ thanks for doing this! Changes to

Re: RFR: 8153044: Allow a property to control location of tzdb.dat file

2016-07-07 Thread Roger Riggs
Hi Sean, I think I would rename the property to "jdk.time.zone.tzdbfile" to reinforce it is captive to the zone implementation. Though the original code builds the path with string concatenation, I think it would be better to use java.nio.Paths.get() to build it. i.e. Paths.get(getProperty

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Volker Simonis
Hi Andrew, thanks a lot for the detailed explanation! Regards, Volker On Thu, Jul 7, 2016 at 4:54 PM, Andrew Dinn wrote: > On 07/07/16 14:59, Volker Simonis wrote: >> - I was a little bit surprised that I could reflectively access and >> execute java.lang.VersionProps.parseVersionNumbers() whe

Discussion: JDK-8160710 Enable Thread to grant VarHandle field access to ThreadLocalRandom/Striped64

2016-07-07 Thread Peter Levart
Hi, On 07/01/2016 10:36 AM, Paul Sandoz wrote: On 1 Jul 2016, at 00:53, Doug Lea wrote: On 06/30/2016 10:08 AM, Paul Sandoz wrote: Hi Peter, Thanks, that’s interesting. I am uncertain if our 166 fellows will be reluctant or not to pull in a dependency on jdk.internal.misc.SharedSecrets. B

Better implementation of Long.divideUnsigned and Long.remainderUnsigned

2016-07-07 Thread John Platts
In the implementations of Long.divideUnsigned and Long.remainderUnsigned, there is a better way to handle the case where dividend > Long.MAX_VALUE and divisor <= Long.MAX_VALUE than doing it through a BigInteger. Here is the current implementation of Long.divideUnsigned and Long.remainderUnsign

Re: RFR: 8153044: Allow a property to control location of tzdb.dat file

2016-07-07 Thread Stephen Colebourne
I won't block this change, but I remain unenthused. There are other ways to control the time-zone data, and since tzdb.dat is currently a private format file that is part of the internals of the JDK, it seems odd to be exposing it. Having the tzdb.dat as a versioned module that could be replaced,

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Andrew Dinn
On 07/07/16 14:59, Volker Simonis wrote: > - I was a little bit surprised that I could reflectively access and > execute java.lang.VersionProps.parseVersionNumbers() where both the > class and the method are package private. Maybe this is related to > Jigsaw issue #ReflectiveAccessToNonExportedType

RFR: 8153044: Allow a property to control location of tzdb.dat file

2016-07-07 Thread Seán Coffey
Looking to add a new implementation specific system property which would allow the user to specify the location of the tzdb.dat file. https://bugs.openjdk.java.net/browse/JDK-8153044 http://cr.openjdk.java.net/~coffeys/webrev.8153044/webrev/ -- Regards, Sean.

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Volker Simonis
On Thu, Jul 7, 2016 at 4:26 PM, Alan Bateman wrote: > On 07/07/2016 14:59, Volker Simonis wrote: > >> : >> >> - I was a little bit surprised that I could reflectively access and >> execute java.lang.VersionProps.parseVersionNumbers() where both the >> class and the method are package private. Mayb

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Alan Bateman
On 07/07/2016 14:59, Volker Simonis wrote: : - I was a little bit surprised that I could reflectively access and execute java.lang.VersionProps.parseVersionNumbers() where both the class and the method are package private. Maybe this is related to Jigsaw issue #ReflectiveAccessToNonExportedType

Re: RFR: jsr166 jdk9 integration wave 7

2016-07-07 Thread Peter Levart
Hi Paul, On 07/07/2016 12:39 PM, Paul Sandoz wrote: On 30 Jun 2016, at 13:20, Martin Buchholz wrote: Webrev regenerated with updates. Lots of rework for the atomic VarHandle specs. http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/ Here is a spec diff for the atomi

RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

2016-07-07 Thread Volker Simonis
Hi, can I please have a review for the following change which makes VersionProps.versionNumbers() testable and the corresponding regression test: http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564/ https://bugs.openjdk.java.net/browse/JDK-8160564 During the review for "8160457: VersionProp

Re: RFR: JDK-8160829 - Remove ASMPool support from jlink

2016-07-07 Thread Sundararajan Athijegannathan
+1 On 7/5/2016 7:48 PM, Jim Laskey (Oracle) wrote: > Much of the removed code seems unnecessary since the same functionality can > be accomplished with much simpler code. An example is provided with > ClassForNamePlugin.java (temporary.) A shipping byte code optimizer plugin > will be suppli

Re: RFR 8160885 Unsafe.compareAndExchangeDouble/FloatAcquire should defer to compareAndExchangeLong/IntAcquire

2016-07-07 Thread Michael Haupt
Hi Paul, thumbs up. Best, Michael > Am 07.07.2016 um 12:06 schrieb Paul Sandoz : > > Hi, > > Please review the simple patch below. > > As spotted by Andrew Haley the Unsafe.compareAndExchangeDouble/FloatAcquire > methods were incorrectly deferring to compareAndExchangeLong/IntVolatile > ra

Re: RFR: jsr166 jdk9 integration wave 7

2016-07-07 Thread Paul Sandoz
> On 30 Jun 2016, at 13:20, Martin Buchholz wrote: > > Webrev regenerated with updates. > Lots of rework for the atomic VarHandle specs. > http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jsr166-jdk9-integration/ > Here is a spec diff for the atomics: http://cr.openjdk.java.net/~psandoz/jd

RFR 8160885 Unsafe.compareAndExchangeDouble/FloatAcquire should defer to compareAndExchangeLong/IntAcquire

2016-07-07 Thread Paul Sandoz
Hi, Please review the simple patch below. As spotted by Andrew Haley the Unsafe.compareAndExchangeDouble/FloatAcquire methods were incorrectly deferring to compareAndExchangeLong/IntVolatile rather than compareAndExchangeLong/IntAcquire. Other, related, double/float Unsafe methods defer correc