Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Peter Levart
Hi Martin, On 02/14/2018 07:58 AM, Peter Levart wrote: It may be that the intent was to refrain from using the shared 'lock' lock for the 2nd and subsequent calls to runFinalizer() and only use the more fine-grained 'this' lock in this case? If someone was able to call runFinalizer() on the s

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Peter Levart
Hi Again, While studying the code of Finalizer I found a strange discrepancy between handling of 'unfinalized' field during processing of Finalizer(s) taken from ReferenceQueue and taken from the 'unfinalized' pointer itself (as in runAllFinalizers()). The remove() method is as follows:    

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Peter Levart
Hi, I may have found a situation described here... On 02/14/2018 09:47 AM, Peter Levart wrote: When Finalizer(s) are taken from ReferenceQueue, they are processed in arbitrary order, so once in a while it can happen that the Finalizer at the head of the list (pointed to by 'unfinalized') is

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Peter Levart
I could even claim that simplifying the if statement in remove() to:     if (unfinalized == this) {         unfinalized = this.next;     } makes checking for hasBeenFinalized() in runFinalizer() redundant as it would not be possible for runFinalizer() to be called more than once for each Final

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Peter Levart
I take back this claim. Of course the the following race is possible: - Thread1: calls runAllFinalizers and takes a Finalizer from 'unprocessed' list. - Thread2: takee the same Finalizer instance from ReferenceQueue and calls runFinalizer() - Thread1: calls runFinalizer() with the same instan

[1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread Volker Simonis
Hi, can I please get a review for the following tiny fix: http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/ https://bugs.openjdk.java.net/browse/JDK-8197927 The new Java 10 specification makes the 'java.vendor.version' property mandatory [1] but the current implementations doesn't allow

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Peter Levart
Hi Martin, On 02/14/2018 10:58 AM, Peter Levart wrote: I take back this claim. Of course the the following race is possible: - Thread1: calls runAllFinalizers and takes a Finalizer from 'unprocessed' list. - Thread2: takee the same Finalizer instance from ReferenceQueue and calls runFinalize

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread Thomas Stüfe
Fix is fine, trivial and I do not think there is any risk attached to it. I am not in any position to comment whether this is P1. Copyright year needs adjusting. Kind Regards, Thomas On Wed, Feb 14, 2018 at 11:20 AM, Volker Simonis wrote: > Hi, > > can I please get a review for the following ti

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Peter Levart
On 02/14/2018 11:49 AM, Peter Levart wrote: Hi Martin, On 02/14/2018 10:58 AM, Peter Levart wrote: I take back this claim. Of course the the following race is possible: - Thread1: calls runAllFinalizers and takes a Finalizer from 'unprocessed' list. - Thread2: takee the same Finalizer inst

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread Volker Simonis
Thanks Thomas. The sole reason for making it P1 is that it is currently not possible to build a Java 10 conforming version of OpenJDK with an empty vendor version. The specification doesn't mandate a non-empty vendor version and I don't think OpenJDK should mandate on either. Regards, Volker On

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-14 Thread Alan Bateman
On 14/02/2018 01:23, yumin qi wrote: Hi,   I have update the webrev: http://cr.openjdk.java.net/~minqi/8194154/webrev1/   In this version, as suggested by Alan(thanks!), property of "user.dir" is cached and behave like it is 'read only'.

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

2018-02-14 Thread David Holmes
Adding in core-libs-dev as there's nothing related to hotspot directly here. David On 14/02/2018 9:32 PM, Adam Farley8 wrote: Hi All, Currently, diagnostic core files generated from OpenJDK seem to lump all of the native memory usages together, making it near-impossible for someone to figure o

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

2018-02-14 Thread David Holmes
On 14/02/2018 10:43 PM, David Holmes wrote: Adding in core-libs-dev as there's nothing related to hotspot directly here. Correction, this is of course leading to a proposed change in hotspot to implement the new Unsafe methods and perform the native memory tracking. Of course we already have

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

2018-02-14 Thread Adam Farley8
> Adding in core-libs-dev as there's nothing related to hotspot directly here. > > David Thought it was best to pass this through hotspot lists first, as full completion of the native side of things will probably require hotspot changes. You're quite right though, I should have cc'd hotspot *an

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

2018-02-14 Thread Thomas Stüfe
On Wed, Feb 14, 2018 at 1:53 PM, David Holmes wrote: > On 14/02/2018 10:43 PM, David Holmes wrote: > >> Adding in core-libs-dev as there's nothing related to hotspot directly >> here. >> > > Correction, this is of course leading to a proposed change in hotspot to > implement the new Unsafe method

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

2018-02-14 Thread Adam Farley8
>> Adding in core-libs-dev as there's nothing related to hotspot directly >> here. > > Correction, this is of course leading to a proposed change in hotspot to > implement the new Unsafe methods and perform the native memory tracking. Hah, I wrote the same thing in a parallel reply. Jinx. :)

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

2018-02-14 Thread Thomas Stüfe
On Wed, Feb 14, 2018 at 2:32 PM, Adam Farley8 wrote: > >> Adding in core-libs-dev as there's nothing related to hotspot directly > >> here. > > > > Correction, this is of course leading to a proposed change in hotspot to > > > implement the new Unsafe methods and perform the native memory trackin

RFR: JDK-8190187: C++ code calling JNI_CreateJavaVM can be killed by Java

2018-02-14 Thread Adam Farley8
Hi All, -- Short version -- Could a committer please take the fix for JDK-8190187 (full code included in the bug) and: 1) Complete the CSR process for the new JNI Return code. 2) Commit the changes that contain (a) the new return code, and (b) the non-Hotspot code that handles the new code. B

Re: RFR: JDK-8190187: C++ code calling JNI_CreateJavaVM can be killed by Java

2018-02-14 Thread Alan Bateman
On 14/02/2018 14:13, Adam Farley8 wrote: Hi All, -- Short version -- Could a committer please take the fix for JDK-8190187 (full code included in the bug) and: 1) Complete the CSR process for the new JNI Return code. 2) Commit the changes that contain (a) the new return code, and (b) the non-H

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread mark . reinhold
2018/2/14 2:20:22 -0800, volker.simo...@gmail.com: > can I please get a review for the following tiny fix: > > http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/ > https://bugs.openjdk.java.net/browse/JDK-8197927 > > The new Java 10 specification makes the 'java.vendor.version' property >

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

2018-02-14 Thread Zhengyu Gu
On 02/14/2018 08:16 AM, Thomas Stüfe wrote: On Wed, Feb 14, 2018 at 1:53 PM, David Holmes wrote: On 14/02/2018 10:43 PM, David Holmes wrote: Adding in core-libs-dev as there's nothing related to hotspot directly here. Correction, this is of course leading to a proposed change in hotspot

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native Memory Usage for Direct Byte Buffers

2018-02-14 Thread Zhengyu Gu
Think of it as an NMT upgrade. Here's an example of what the output should look like: https://developer.ibm.com/answers/questions/288697/why- does-nativememinfo-in-javacore-show-incorrect.html?sort=oldest - Adam I think NMT walks the stack, so we should get allocation points grouped by cal

Re: [PATCH] RFR Bug-pending: Enable Hotspot to Track Native MemoryUsage for Direct Byte Buffers

2018-02-14 Thread Bernd Eckenfels
Maybe instead adding a „allocation request type“ argment to allocate Memory? (and wrap it with a typed allocator in the buffer interfacessomewhere?) the „DBB“ part Looks especially cryptic. We have similiar concepts for NMT in the native Code. Besides I mentioned a while back that the JMX part

Re: [1] RFR(XXS): 8197927: Can't set mandatory 'java.vendor.version' property to empty string

2018-02-14 Thread Volker Simonis
On Wed, Feb 14, 2018 at 4:26 PM, wrote: > 2018/2/14 2:20:22 -0800, volker.simo...@gmail.com: >> can I please get a review for the following tiny fix: >> >> http://cr.openjdk.java.net/~simonis/webrevs/2018/8197927/ >> https://bugs.openjdk.java.net/browse/JDK-8197927 >> >> The new Java 10 specifica

Re: RFR: 8197893: Mistaken type check in CheckedEntrySet.toArray

2018-02-14 Thread Paul Sandoz
> On Feb 13, 2018, at 5:33 PM, Martin Buchholz wrote: > > http://cr.openjdk.java.net/~martin/webrevs/jdk/CheckedEntrySet-toArray/ > https://bugs.openjdk.java.net/browse/JDK-8197893 +1 Paul.

Re: RFR JDK-8164278: java.util.Base64.EncOutputStream/DecInputStream is slower than corresponding version in javax.mail package

2018-02-14 Thread Roger Riggs
Hi Sherman, I found updates in http://cr.openjdk.java.net/~sherman/8164278/webrev.04 That looks fine. Typo:  line 1008: "neve" Roger On 2/7/2018 10:32 PM, Xueming Shen wrote: Hi Roger, Given the concern of the possible incompatible behavior change of over reading bytes from the underlyin

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-14 Thread yumin qi
Hi, Alan Thanks. Updated on same link http://cr.openjdk.java.net/~minqi/8194154/webrev1/ as your recommendation. Yumin On Wed, Feb 14, 2018 at 4:42 AM, Alan Bateman wrote: > On 14/02/2018 01:23, yumin qi wrote: > >> Hi, >> >> I have update the webrev: >> http://cr.openjdk.java.net/~mi

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

2018-02-14 Thread Roger Riggs
Hi, In the test, the message can be improved; though the exception should never occur it will be misleading if it does. Instead:     "Changing property user.dir should have no effect on the getCanonicalPath" It is cleaner to throw AssertionError for the test failure. Throwing FileSystemExcep

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-14 Thread Brian Burkhalter
Hello, In the test you have /* @test requires (os.family == "linux") | (os.family == "mac") | (os.family == "solaris") | (os.family == "aix") @bug 8194154 @summary Test parsing path with double slashes on unix like platforms. */ but I think you need to have the @requires annotation on a se

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread mandy chung
On 2/14/18 1:58 AM, Peter Levart wrote: I take back this claim. Of course the the following race is possible: - Thread1: calls runAllFinalizers and takes a Finalizer from 'unprocessed' list. - Thread2: takee the same Finalizer instance from ReferenceQueue and calls runFinalizer() - Thread1:

RFR(xs): 8193909: Obsolete(remove) Co-operative Memory Management (CMM)

2018-02-14 Thread sangheon.kim
Hi all, Could I have some reviews for CMM removal? This is closed CR but some public codes also need small modifications. This CR is for removing stuff related to an Oracle JDK module/package. Changes are just removing CMM from lists or in a test to skip the testing logic. CR: https://bugs.op

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Martin Buchholz
Indeed I had suppressed all memory of runFinalizersOnExit. (Sorry about that.) I support removing it in jdk11. Mandy, would you like to file the CSR? On Wed, Feb 14, 2018 at 1:15 PM, mandy chung wrote: > > > On 2/14/18 1:58 AM, Peter Levart wrote: > > > I take back this claim. Of course the the

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread mandy chung
On 2/14/18 1:54 PM, Martin Buchholz wrote: Indeed I had suppressed all memory of runFinalizersOnExit. (Sorry about that.) I support removing it in jdk11. Great. Mandy, would you like to file the CSR? Yes I plan to do that for:     https://bugs.openjdk.java.net/browse/JDK-4240589 Mandy

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

2018-02-14 Thread yumin qi
HI, Roger and Brian Updated again in same link. I could not run jtreg so please have a look if the options are good for @run Thanks Yumin On Wed, Feb 14, 2018 at 11:13 AM, Roger Riggs wrote: > Hi, > > In the test, the message can be improved; though the exception should > never occur > it

Re: RFR(xs): 8193909: Obsolete(remove) Co-operative Memory Management (CMM)

2018-02-14 Thread David Holmes
That all seems trivially fine. Thanks, David On 15/02/2018 7:45 AM, sangheon.kim wrote: Hi all, Could I have some reviews for CMM removal? This is closed CR but some public codes also need small modifications. This CR is for removing stuff related to an Oracle JDK module/package. Changes are

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

2018-02-14 Thread Brian Burkhalter
Yumin, As for the test options you would need to make this change: --- a/test/jdk/java/io/File/ParseCanonicalPath.java +++ b/test/jdk/java/io/File/ParseCanonicalPath.java @@ -25,7 +25,7 @@ (os.family == "solaris") | (os.family == "aix") @bug 8194154 @summary Test parsin

Re: RFR(xs): 8193909: Obsolete(remove) Co-operative Memory Management (CMM)

2018-02-14 Thread jesper . wilhelmsson
Looks good! /Jesper > On 14 Feb 2018, at 22:45, sangheon.kim wrote: > > Hi all, > > Could I have some reviews for CMM removal? > This is closed CR but some public codes also need small modifications. This > CR is for removing stuff related to an Oracle JDK module/package. > Changes are just re

Re: RFR(xs): 8193909: Obsolete(remove) Co-operative Memory Management (CMM)

2018-02-14 Thread sangheon.kim
Hi David and Jesper, Thank you for the review. Sangheon On 02/14/2018 02:53 PM, jesper.wilhelms...@oracle.com wrote: Looks good! /Jesper On 14 Feb 2018, at 22:45, sangheon.kim wrote: Hi all, Could I have some reviews for CMM removal? This is closed CR but some public codes also need smal

Re: RFR(xs): 8193909: Obsolete(remove) Co-operative Memory Management (CMM)

2018-02-14 Thread mandy chung
+1 Mandy On 2/14/18 1:45 PM, sangheon.kim wrote: Hi all, Could I have some reviews for CMM removal? This is closed CR but some public codes also need small modifications. This CR is for removing stuff related to an Oracle JDK module/package. Changes are just removing CMM from lists or in a te

Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-14 Thread yumin qi
Brian, Updated using RuntimeException, which will not need -ea so removed. http://cr.openjdk.java.net/~minqi/8194154/webrev1/ Thanks Yumin On Wed, Feb 14, 2018 at 11:17 AM, Brian Burkhalter < brian.burkhal...@oracle.com> wrote: > Hello, > > In the test you have > > /* @test requires (os.fam

Re: RFR(xs): 8193909: Obsolete(remove) Co-operative Memory Management (CMM)

2018-02-14 Thread sangheon.kim
Hi Mandy, Thank you for the review! Sangheon On 02/14/2018 03:45 PM, mandy chung wrote: +1 Mandy On 2/14/18 1:45 PM, sangheon.kim wrote: Hi all, Could I have some reviews for CMM removal? This is closed CR but some public codes also need small modifications. This CR is for removing stuff

Re: RFR (JDK11) 8137326: Methods for comparing CharSequence, StringBuilder, and StringBuffer

2018-02-14 Thread Stuart Marks
Hi Joe, Overall, looks good. Are there any tests of AbstractStringBuilder.compareTo() that exercise comparisons of the Latin1 vs UTF16 coders? A couple people have raised the issue of the SB's implementing Comparable but not overriding equals(). This is unusual but well-defined. I do think i

Re: RFR: JDK-8190187: C++ code calling JNI_CreateJavaVM can be killed by Java

2018-02-14 Thread David Holmes
On 15/02/2018 12:13 AM, Adam Farley8 wrote: Hi All, -- Short version -- Could a committer please take the fix for JDK-8190187 (full code included in the bug) and: 1) Complete the CSR process for the new JNI Return code. 2) Commit the changes that contain (a) the new return code, and (b) the no

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Martin Buchholz
Embarrassed by my failure to take runFinalizersOnExit into account, I tried to redo the patch to be actually correct. http://cr.openjdk.java.net/~martin/webrevs/jdk/Finalizer-data-race/ even though we may succeed in making runFinalizersOnExit go away, and I'm not planning to submit any time soon.

Re: RFR: 8197812: (ref) Data race in Finalizer

2018-02-14 Thread Peter Levart
Hi Martin, On 02/15/2018 06:47 AM, Martin Buchholz wrote: Embarrassed by my failure to take runFinalizersOnExit into account, I tried to redo the patch to be actually correct. I think that your patch was correct the first time too, but in view of future removing the need for "pollFirst" from

RFR: JDK-8197988, mach5 T2 test javax/net/ssl/interop/ClientHelloChromeInterOp.java failed after JDK-8164278

2018-02-14 Thread Xueming Shen
Hi, Please help review the change for JDK-819798 issue: https://bugs.openjdk.java.net/browse/JDK-8197988 webrev: http://cr.openjdk.java.net/~sherman/8197988/webrev The change for JDK-8164278 caused a decoding regression. A Tier2 test case, javax/net/ssl/interop/ClientHelloChromeInterOp.java,