Re: RFR [14] 8217606: LdapContext#reconnect always opens a new connection

2019-08-07 Thread Chris Yin
Many thanks Pavel for the changes, and thanks Lance, Roger for the reviewing. Yep, let me try to handle some questions from Roger > On 7 Aug 2019, at 11:52 PM, Roger Riggs wrote: > > BaseLdapServer: > > 158: Is printing the stack trace diagnostic or an error?, the exception is > not rethrown

Re: ProcessHandle.Info startInstant() drifts with system time on Linux

2019-08-07 Thread Duft Markus
Hey, Thanks for the answer and information. OK - I understand that Linux does not provide a stable absolute timestamp per process to read, so the JVM tries to come up with one. Would it be possible somehow to provide the user with another API to uniquely identify a process (just any arbitrary "

RE: RFR: 8158880: test/java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java fail with zh_CN locale

2019-08-07 Thread Thejasvi Voniadka
Hi Naoto, Thank you for the review. This change has now been pushed. -Original Message- From: Naoto Sato Sent: Thursday, August 01, 2019 9:43 PM To: Thejasvi Voniadka ; i18n-...@openjdk.java.net; core-libs-dev@openjdk.java.net Subject: Re: RFR: 8158880: test/java/time/tck/java/time/f

RFR (XS) 8221307 : String.substring() OOB exception on start index reports improper information

2019-08-07 Thread Ivan Gerasimov
Hello! The exception thrown by substring(int) may look confusing. For example it produces "String index out of range: -1" when the index is length+1. It is proposed to make the error message more clear, similar to what we have for substring(int, int). BUGURL: https://bugs.openjdk.java.net/b

Re: RFR: JDK-8229252 : Add descriptions to Windows jtreg tests

2019-08-07 Thread Andy Herrick
this is fine. /Andy On 8/7/2019 12:47 PM, Alexey Semenyuk wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). - Add comments to Windows jtreg tests to help SQE setup their testing. [1] https://

Re: [14] RFR: 8215181: Accounting currency format support

2019-08-07 Thread naoto . sato
Thanks, Roger. I will replace the string length check as you suggested before push. Naoto On 8/7/19 2:14 PM, Roger Riggs wrote: +1, Bundle.java:226:  there is a slight preference for String.isEmpty() over  String.length() == 0 Thanks, Roger On 8/7/19 4:26 PM, Lance Andersen wrote: awesome

Re: [14] RFR: 8215181: Accounting currency format support

2019-08-07 Thread Roger Riggs
+1, Bundle.java:226:  there is a slight preference for String.isEmpty() over  String.length() == 0 Thanks, Roger On 8/7/19 4:26 PM, Lance Andersen wrote: awesome, I must have missed it thinking it was a bug when I reviewed the bug itself (long day :-) ) …. On Aug 7, 2019, at 4:24 PM, naot

Re: RFR: 8226297: Dual-pivot quicksort improvements

2019-08-07 Thread Laurent Bourgès
Hi Brent, I made a manual comparison of your webrev.4 and my previous patch. - I noticed plenty of javadoc / comment changes that seem totally appropriate in Arrays class: ok for me. - You also fixed the problem with the ForkJoinPool field: good. - No change in the DualPivotQuickSort class except

Re: [14] RFR: 8215181: Accounting currency format support

2019-08-07 Thread Lance Andersen
awesome, I must have missed it thinking it was a bug when I reviewed the bug itself (long day :-) ) …. > On Aug 7, 2019, at 4:24 PM, naoto.s...@oracle.com wrote: > > Thanks for the review, Lance. > > Yes, CSR is needed and it is already approved: > > https://bugs.openjdk.java.net/browse/JDK-82

Re: [14] RFR: 8215181: Accounting currency format support

2019-08-07 Thread naoto . sato
Thanks for the review, Lance. Yes, CSR is needed and it is already approved: https://bugs.openjdk.java.net/browse/JDK-8218770 Naoto On 8/7/19 1:20 PM, Lance Andersen wrote: Hi again Naoto, I meant to ask is there a CSR due to the change in the javadoc for NumberFormat?  If not, there probab

Re: [14] RFR: 8215181: Accounting currency format support

2019-08-07 Thread Lance Andersen
Hi again Naoto, I meant to ask is there a CSR due to the change in the javadoc for NumberFormat? If not, there probably should > On Aug 7, 2019, at 4:17 PM, Lance Andersen wrote: > > Hi Naoto, > > I looked over the proposed changes and the bug. They seem reasonable to me. > > Best > Lance

Re: [14] RFR: 8215181: Accounting currency format support

2019-08-07 Thread Lance Andersen
Hi Naoto, I looked over the proposed changes and the bug. They seem reasonable to me. Best Lance > On Aug 7, 2019, at 4:12 PM, naoto.s...@oracle.com wrote: > > Ping. > > Naoto > > On 7/31/19 5:57 PM, naoto.s...@oracle.com wrote: >> Hi, >> Please review the fix to the following issue: >> https

Re: [14] RFR: 8215181: Accounting currency format support

2019-08-07 Thread naoto . sato
Ping. Naoto On 7/31/19 5:57 PM, naoto.s...@oracle.com wrote: Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8215181 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8215181/webrev.00/ This is to enable "accounting" style c

Re: RFR 8226530: ZipFile reads wrong entry size from ZIP64 entries

2019-08-07 Thread Martin Buchholz
Thanks for joining the small club of zip implementers. No one likes the ZIP file format, but the power of a legacy data interchange format is great. On Wed, Aug 7, 2019 at 11:54 AM Lance Andersen wrote: > Hi Martin, > > On Aug 7, 2019, at 2:37 PM, Martin Buchholz wrote: > > Thanks for fixing th

Re: RFR: JDK-8229252 : Add descriptions to Windows jtreg tests

2019-08-07 Thread Alexander Matveev
Hi Alexey, Looks good. Thanks, Alexander On 8/7/2019 9:47 AM, Alexey Semenyuk wrote: Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). - Add comments to Windows jtreg tests to help SQE setup their t

Re: RFR 8226530: ZipFile reads wrong entry size from ZIP64 entries

2019-08-07 Thread Lance Andersen
Hi Martin, > On Aug 7, 2019, at 2:37 PM, Martin Buchholz wrote: > > Thanks for fixing this. Looks good. Thank you. > The APPNOTE.TXT spec is very confusing and surprising - it is not at all > obvious why the size fields are required in the LOC ZIP64 header but not in > the CEN ZIP64 header.

Re: RFR [14] 8217606: LdapContext#reconnect always opens a new connection

2019-08-07 Thread Pavel Rappo
> On 7 Aug 2019, at 18:49, Roger Riggs wrote: > > > > It seemed unnecessary/redundant since except for isRunning it is updated > inside a synchronize(lock). Pardon, what seemed unnecessary? > The synchronized(socketList) is also unnecessary since it is already > synchronized on lock. You

Re: RFR 8226530: ZipFile reads wrong entry size from ZIP64 entries

2019-08-07 Thread Martin Buchholz
Thanks for fixing this. Looks good. The APPNOTE.TXT spec is very confusing and surprising - it is not at all obvious why the size fields are required in the LOC ZIP64 header but not in the CEN ZIP64 header. I might include a reference to the sections in the PKWARE spec - 4.5.3 and 4.4.9 It seems t

Re: RFR [14] 8217606: LdapContext#reconnect always opens a new connection

2019-08-07 Thread Roger Riggs
Hi Pavel, You are correct, state is volatile. It seemed unnecessary/redundant since except for isRunning it is updated inside a synchronize(lock). The synchronized(socketList) is also unnecessary since it is already synchronized on lock. Roger On 8/7/19 1:18 PM, Pavel Rappo wrote: Roger,

Re: ProcessHandle.Info startInstant() drifts with system time on Linux

2019-08-07 Thread Roger Riggs
Hi Markus, core-libs-dev@openjdk.java.net is the more appropriate list for this question. The ProcessHandle info is based directly on the OS information about a process, there is no separate information stored or kept. If Linux had a stable way to represent the start time it would be reflect

Re: RFR [14] 8217606: LdapContext#reconnect always opens a new connection

2019-08-07 Thread Pavel Rappo
Roger, thank you for looking at this. While we might hear (on some of your questions) from Chris soon, I just have to ask about this one > On 7 Aug 2019, at 16:52, Roger Riggs wrote: > > 238: isRunning should be synchronized(lock) to make sure each Thread sees the > same value threads that upda

RFR: JDK-8229252 : Add descriptions to Windows jtreg tests

2019-08-07 Thread Alexey Semenyuk
Please review the jpackage fix for bug [1] at [2]. This is a fix for the JDK-8200758-branch branch of the open sandbox repository (jpackage). - Add comments to Windows jtreg tests to help SQE setup their testing. [1] https://bugs.openjdk.java.net/browse/JDK-8229252 [2] http://cr.openjdk.java

Re: RFR 8226530: ZipFile reads wrong entry size from ZIP64 entries

2019-08-07 Thread Brian Burkhalter
Hi Lance, > On Aug 7, 2019, at 9:33 AM, Lance Andersen wrote: > > I thought about it but decided it was not needed at least now given the files > are only used by this test and the scratch directory will get cleaned up > anyways. I thought that might be the case. > If you think I should add

Re: RFR 8226530: ZipFile reads wrong entry size from ZIP64 entries

2019-08-07 Thread Lance Andersen
Hi Brian, > On Aug 7, 2019, at 12:21 PM, Brian Burkhalter > wrote: > > Hi Lance, > > Looks good. thank you > I was wondering whether you might want to delete the files using > jdk.test.lib.util.FileUtils.deleteFileIfExistsWithRetry() instead. I thought about it but decided it was not needed

Re: RFR 8226530: ZipFile reads wrong entry size from ZIP64 entries

2019-08-07 Thread Brian Burkhalter
Hi Lance, Looks good. I was wondering whether you might want to delete the files using jdk.test.lib.util.FileUtils.deleteFileIfExistsWithRetry() instead. Brian > On Aug 6, 2019, at 1:31 PM, Lance Andersen wrote: > > Please review the fix for https://bugs.openjdk.java.net/browse/JDK-8226530 >

Re: RFR [14] 8217606: LdapContext#reconnect always opens a new connection

2019-08-07 Thread Roger Riggs
Hi, Nicely structured server framework, some details to cleanup. BaseLdapServer: 100: The new exception should have a message "Unexpected exception" or "server should no be running"... 158: Is printing the stack trace diagnostic or an error?, the exception is not rethrown and no message clari

Re: RFR: 8224974: Implement JEP 352

2019-08-07 Thread Andrew Dinn
On 07/08/2019 14:32, Dmitry Chuyko wrote: > The test fails on some machines but does not fail on others, all 4.x > kernels. The possible problem may be when build host and run host are > different machines. This seems to be related to map0() implementation in > FileChannelImpl.c in case MAP_SYNC an

Re: RFR: 8224974: Implement JEP 352

2019-08-07 Thread Dmitry Chuyko
On 8/7/19 4:29 PM, Andrew Dinn wrote: On 07/08/2019 13:04, Dmitry Chuyko wrote: On 8/7/19 2:02 PM, Dmitry Chuyko wrote: Andrew, New code is buildable and new MapFail test passes on different platforms except it fails on linux i386: Ah, that even was x86_64 (sorry, mixed up results from auto

Re: RFR: 8224974: Implement JEP 352

2019-08-07 Thread Dmitry Chuyko
The test fails on some machines but does not fail on others, all 4.x kernels. The possible problem may be when build host and run host are different machines. This seems to be related to map0() implementation in FileChannelImpl.c in case MAP_SYNC and MAP_SHARED_VALIDATE are not defined (or defi

Re: RFR [14] 8217606: LdapContext#reconnect always opens a new connection

2019-08-07 Thread Lance Andersen
Hi Pavel, The change looks good. Nice job on the tests :-) > On Aug 7, 2019, at 8:47 AM, Pavel Rappo wrote: > > Hello, > > Please review the following change: > > http://cr.openjdk.java.net/~prappo/8217606/webrev.00/ > > The original issue seems to be an unintended consequence of JDK-805

Re: RFR: 8224974: Implement JEP 352

2019-08-07 Thread Andrew Dinn
On 07/08/2019 13:04, Dmitry Chuyko wrote: > On 8/7/19 2:02 PM, Dmitry Chuyko wrote: >> Andrew, >> >> New code is buildable and new MapFail test passes on different >> platforms except it fails on linux i386: > > Ah, that even was x86_64 (sorry, mixed up results from automated > system). I'll try t

RFR [14] 8217606: LdapContext#reconnect always opens a new connection

2019-08-07 Thread Pavel Rappo
Hello, Please review the following change: http://cr.openjdk.java.net/~prappo/8217606/webrev.00/ The original issue seems to be an unintended consequence of JDK-8059009 [1]. The proposed change is based on the private patch by Chris Yin. I skimmed through the history of the `com.sun.jndi.ldap

Re: RFR: 8224974: Implement JEP 352

2019-08-07 Thread Dmitry Chuyko
On 8/7/19 2:02 PM, Dmitry Chuyko wrote: Andrew, New code is buildable and new MapFail test passes on different platforms except it fails on linux i386: Ah, that even was x86_64 (sorry, mixed up results from automated system). I'll try to reproduce it by hand to see if there are any addition

Re: RFR: 8224974: Implement JEP 352

2019-08-07 Thread Dmitry Chuyko
Andrew, New code is buildable and new MapFail test passes on different platforms except it fails on linux i386: --System.err:(12/712)-- java.lang.Exception: unexpected message for IOExceptionInvalid argument     at MapFail.main(MapFail.java:60)     at java.base/jdk.internal.re

Re: RFR 8226530: ZipFile reads wrong entry size from ZIP64 entries

2019-08-07 Thread Aleksey Shipilev
On 8/6/19 10:31 PM, Lance Andersen wrote: > The webrev can be found at > http://cr.openjdk.java.net/~lancea/8226530/webrev.00/index.html Looks good to me. I spot-tested my test cases that used to fail, and they all pass with this patch. Thanks! -Aleksey

RE: RFR 8226530: ZipFile reads wrong entry size from ZIP64 entries

2019-08-07 Thread Langer, Christoph
Hi Lance, this looks good to me. Best regards Christoph > -Original Message- > From: core-libs-dev On Behalf > Of Lance Andersen > Sent: Dienstag, 6. August 2019 22:32 > To: core-libs-dev > Subject: RFR 8226530: ZipFile reads wrong entry size from ZIP64 entries > > Hi > > Please revi

Re: RFR: 8224974: Implement JEP 352

2019-08-07 Thread Alan Bateman
On 06/08/2019 09:09, Andrew Dinn wrote: : The unmapper code is not strictly 'new' as regards its reliance on synchronization. It merely follows and repeats the pattern employed in the prior code that it has generalized (by splitting the original Unmapper into two distinct flavours of subclass).

Re: RFR: 8224974: Implement JEP 352

2019-08-07 Thread Dmitry Chuyko
On 8/6/19 7:09 PM, Andrew Dinn wrote: Hello Dmitry, On 06/08/2019 15:25, Dmitry Chuyko wrote: One quick question about synchronization in unmappers. One of preliminary steps for Loom was to replace monitor usage by j.u.c locks for I/O to let fibers release carrier threads. For instance see JDK-

Re: RFR: 8224974: Implement JEP 352

2019-08-07 Thread Dmitry Chuyko
On 8/6/19 6:58 PM, Andrew Dinn wrote: .. No this behaviour is not currently tested. However, the only client at present will never exercise that path so it is not critical to test it now. I'd be happy to address testing of this behaviour as part of a follow-up JIRA issue if yo