Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Wed, Jun 5, 2019 at 8:37 AM Martin Buchholz wrote: > > > On Tue, Jun 4, 2019 at 9:50 PM Thomas Stüfe > wrote: > >> >> I would vote for keeping the name as it is: I would have to change a >> number of places, and when in doubt I'd like to keep the changes minimal to >> make backporting future

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi David, You are right. I am pretty sure there are ancient historical reasons for doing this by hand instead of letting exec() deal with scripts itself. Martin or Roger may know more. But I think right now this is more of a compatibility concern - changing this behavior has the risk of breaking

Re: Review Request JDK-8221368: Error message when module main class cannot be loaded is missing exception details

On 04/06/2019 19:34, Mandy Chung wrote: Alan, Sundar, Thanks for the review. I further clean up the test and rename jdk.test.Main2 class to a new test2 module.  No change to the fix. http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221368/webrev.01/ Looks okay.

Re: RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe

Thanks Martin for the second review! I'll fix the close call and then I'll push. Cheers, Thomas On Wed, Jun 5, 2019 at 8:35 AM Martin Buchholz wrote: > Huh, looks like an ancient bug of mine that doesn't trigger in practice. > > Looks good, except change FAIL_FILENO to fail_pipe_fd below (th

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 9:50 PM Thomas Stüfe wrote: > > I would vote for keeping the name as it is: I would have to change a > number of places, and when in doubt I'd like to keep the changes minimal to > make backporting future changes easier. > OK - author gets to cast deciding vote!

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

Looks okay. Just to mention the alternativ: we could add "@requires !vm.aot.enabled" in those tests if only a few. Mandy On 6/4/19 7:29 PM, Mikael Vidstedt wrote: I, too, agree. :) New webrev which adds a new ProblemList-aot.txt to be used when running tests with AOT. Webrev: http://cr

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Wed, Jun 5, 2019 at 5:06 AM Martin Buchholz wrote: > > > On Tue, Jun 4, 2019 at 2:01 PM Roger Riggs wrote: > >> Hmm. Can_JOHNNY_EXEC naming is a bit quirky, not intuitive... >> > > I'm the original author of that joke^H deeply evocative symbol name, so I > might be biased. > > I like the na

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 2:01 PM Roger Riggs wrote: > Hmm. Can_JOHNNY_EXEC naming is a bit quirky, not intuitive... > I'm the original author of that joke^H deeply evocative symbol name, so I might be biased.

RFR(s): 8205131: remove Runtime trace methods

Hi all, Please review this changeset and CSR request that remove the traceInstructions() and traceMethodCalls() methods from java.lang.Runtime. These methods have been deprecated for removal since Java 9, and they do nothing. I've also removed a couple mentions of these methods from some tests

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

LGTM! — Igor > On Jun 4, 2019, at 7:29 PM, Mikael Vidstedt > wrote: > > > I, too, agree. :) > > New webrev which adds a new ProblemList-aot.txt to be used when running tests > with AOT. > > Webrev: > http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.01/open/webrev/ > > Cheers, >

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

I, too, agree. :) New webrev which adds a new ProblemList-aot.txt to be used when running tests with AOT. Webrev: http://cr.openjdk.java.net/~mikael/webrevs/8225305/webrev.01/open/webrev/ Cheers, Mikael > On Jun 4,

Re: RFR(xs): 8225315 test java/util/ArrayDeque/WhiteBox.java isn't part of the jdk_collections test group

On 6/4/19 5:56 PM, Joseph D. Darcy wrote: Should the test be removed from the catch-all group as part of this change? Ah, good question. But the answer is that it doesn't need to be. The "catch-all" group jdk_util_other is defined thus: -- # All util components not part of some other util c

Re: RFR: JDK-8225213: Backport jsr166 tck tests to jdk8u

On Tue, Jun 4, 2019 at 6:07 PM Andrew John Hughes wrote: > > I don't think this is the best approach. JDK-8146467 looks like a pretty > clean snapshot of the tests from not long after 8u GA. It doesn't > include any *9Test.java files for a start. It would seem better to start > with backporting t

Re: RFR: 8225179: (regex) Minor Pattern cleanup

Hi Claes! Looks good to me, thanks! LookBehindNode may be better named LookBehindEndNode, as it should only match at the very end of the look-behind token. With kind regards, Ivan On 6/4/19 1:58 AM, Claes Redestad wrote: Hi, please review this j.u.regex.Pattern cleanup. - refactor BitCl

Re: RFR: JDK-8225213: Backport jsr166 tck tests to jdk8u

On 05/06/2019 01:15, Martin Buchholz wrote: > (Trying to correct my mistake of not having cc'ed jdk8u-dev) > > On Mon, Jun 3, 2019 at 5:15 PM Martin Buchholz > wrote: > > > > -- Forwarded message - > From: *Martin Buchholz (JBS)*

Re: RFR(xs): 8225315 test java/util/ArrayDeque/WhiteBox.java isn't part of the jdk_collections test group

Looks okay. Should the test be removed from the catch-all group as part of this change? -Joe On 6/4/2019 5:53 PM, Stuart Marks wrote: Hi all, This test was added some time ago, but it wasn't added to the jdk_collections test group. Please review the patch appended below, which adds it to th

Re: RFR(xs): 8225315 test java/util/ArrayDeque/WhiteBox.java isn't part of the jdk_collections test group

Looks good to me! On Tue, Jun 4, 2019 at 5:55 PM Stuart Marks wrote: > Hi all, > > This test was added some time ago, but it wasn't added to the > jdk_collections > test group. Please review the patch appended below, which adds it to the > right > group. (The test was still run as part of the ca

RFR(xs): 8225315 test java/util/ArrayDeque/WhiteBox.java isn't part of the jdk_collections test group

Hi all, This test was added some time ago, but it wasn't added to the jdk_collections test group. Please review the patch appended below, which adds it to the right group. (The test was still run as part of the catch-all jdk_util_other group though.) Bug link: https://bugs.openjdk.java.net/b

Re: RFR: JDK-8225213: Backport jsr166 tck tests to jdk8u

(Trying to correct my mistake of not having cc'ed jdk8u-dev) On Mon, Jun 3, 2019 at 5:15 PM Martin Buchholz wrote: > > > -- Forwarded message - > From: Martin Buchholz (JBS) > Date: Mon, Jun 3, 2019 at 5:11 PM > Subject: [JBS] {Commented} (JDK-8225213) Backport jsr166 tck tests

Re: RFR 8225198 : Optimize regex tree for greedy quantifiers of type {N, }

Thank you Claes and Brent for reviewing! I'll edit the comment as suggested before pushing. And yes, we do have a few testcases to verify this optimization in RegExTest.java / BMPTestCases.txt. With kind regards, Ivan On 6/4/19 2:30 PM, Brent Christian wrote: Hi, Ivan The change looks fi

Re: RFR XS,docs JDK-8225309 HTML issues in jdk.jdi module

Hi Jon, Looks good to me. Thanks, Serguei On 6/4/19 2:42 PM, Jonathan Gibbons wrote: Please review another small patch for the jdk.jdi module, to fix a missing heading in one file and to adjust a heading in another. No webrev; patch below. That being said, the file src/jdk.jdi/share/class

Re: RFR: XXS,docs JDK-8225324 Bad HTML in jdk.jfr module-info.java

+1 > On Jun 4, 2019, at 7:47 PM, Jonathan Gibbons > wrote: > > Please review a fix to remove an unnecessary trailing from the > module-info file for jdk.jfr > > Patch below. > > -- Jon > > JBS: https://bugs.openjdk.java.net/browse/JDK-8225324 > > > $ hg diff -R open > diff -r e079a4cfad75

RFR: XXS,docs JDK-8225324 Bad HTML in jdk.jfr module-info.java

Please review a fix to remove an unnecessary trailing from the module-info file for jdk.jfr Patch below. -- Jon JBS: https://bugs.openjdk.java.net/browse/JDK-8225324 $ hg diff -R open diff -r e079a4cfad75 src/jdk.jfr/share/classes/module-info.java --- a/src/jdk.jfr/share/classes/module-info

Re: RFR: XS,docs JDK-8225314 broken links in java.base

Looks good; thanks Jon, -Joe On 6/4/2019 3:28 PM, Jonathan Gibbons wrote: Please review a small fix for a couple of broken links in java.base. Both look like the same copy/paste error, trying to refer to a target id in another package. No webrev: patch below. -- Jon JBS: https://bugs.open

RFR: XS,docs JDK-8225314 broken links in java.base

Please review a small fix for a couple of broken links in java.base. Both look like the same copy/paste error, trying to refer to a target id in another package. No webrev: patch below. -- Jon JBS: https://bugs.openjdk.java.net/browse/JDK-8225314 $ hg diff -R open diff -r 64fe51ee940e src/

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

I agree with Igor.  The best is to keep running these tests except the AOT, perhaps + fastdebug, run only. Mandy On 6/4/19 2:06 PM, Igor Ignatyev wrote: Hi Mikael, as it looks like 8222445 isn't going to be fixed for a long time (as it's "targeted" to tbd), and the defect seems to affect only

Re: RFR XS,docs JDK-8225309 HTML issues in jdk.jdi module

+1 > On Jun 4, 2019, at 5:42 PM, Jonathan Gibbons > wrote: > > Please review another small patch for the jdk.jdi module, to fix a missing > heading in one file and to adjust a heading in another. > > No webrev; patch below. > > That being said, the file > src/jdk.jdi/share/classes/com/sun/jd

RFR XS,docs JDK-8225309 HTML issues in jdk.jdi module

Please review another small patch for the jdk.jdi module, to fix a missing heading in one file and to adjust a heading in another. No webrev; patch below. That being said, the file src/jdk.jdi/share/classes/com/sun/jdi/doc-files/signature.html barely warrants being its own file, but even what

Re: RFR 8225198 : Optimize regex tree for greedy quantifiers of type {N, }

Hi, Ivan The change looks fine. I would call this "noreg-cleanup". Tests are needed to confirm the correctness of the new code, which I imagine we already have. One small suggestion: src/java.base/share/classes/java/util/regex/Pattern.java 4271 * and unlimited maximum, for *, + and

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

Hi Mikael, as it looks like 8222445 isn't going to be fixed for a long time (as it's "targeted" to tbd), and the defect seems to affect only AOT run, I don't think it's a good idea to put these tests into a general problem list. I'd suggest to either create aot-specific problem list or put them

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hmm.  Can_JOHNNY_EXEC naming is a bit quirky, not intuitive... On 06/04/2019 04:52 PM, Martin Buchholz wrote: On Tue, Jun 4, 2019 at 10:40 AM Thomas Stüfe > wrote: On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz mailto:marti...@google.com>> wrote:

Re: RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

Looks good. Thanks Vladimir > On Jun 4, 2019, at 1:38 PM, Mikael Vidstedt > wrote: > > > The following java/lang/invoke/VarHandles tests frequently fail when run with > AOT. Until https://bugs.openjdk.java.net/browse/JDK-8222445 has been fixed > they should be problem listed. > > java/lan

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 10:40 AM Thomas Stüfe wrote: > > > On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz > wrote: > >> >> Unfortunately, it does make the subprocess implementation even >> more complicated, since now "fail" is used to communicate success as well >> as failure. Which probably res

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi Florian, Interesting, but in this case it is not posix_spawn, but plain fork(). The VM does this, the exec calls come from us, not the libc. See childproc.c . Cheers, Thomas’ On Tue 4. Jun 2019 at 22:42, Florian Weimer wrote: > * Thomas Stüfe: > > > Then I ran an strace over it and saw this

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

* Thomas Stüfe: > Then I ran an strace over it and saw this: > > 5332 [pid 3911] execve("./sleep2", ["./sleep2"], [/* 78 vars */] ...> > > .. > 5342 [pid 3911] <... execve res

Re: RFR: XS,doc JDK-8225306 bad headings in java.sql.rowset SyncProvider.java

a Tiny +1 ;-) > On Jun 4, 2019, at 4:33 PM, Jonathan Gibbons > wrote: > > Please review a tiny doc fix to make the headings in this file consistent. > > The headings are currently: > > $ grep 'h[1-6]' > open/src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java > * 1.0 N

RFR (XS): 8225305: ProblemList java/lang/invoke/VarHandles tests

The following java/lang/invoke/VarHandles tests frequently fail when run with AOT. Until https://bugs.openjdk.java.net/browse/JDK-8222445 has been fixed they should be problem listed. java/lang/invoke/VarHandles/VarHandleTestByteArrayAsShort

RFR: XS,doc JDK-8225306 bad headings in java.sql.rowset SyncProvider.java

Please review a tiny doc fix to make the headings in this file consistent. The headings are currently: $ grep 'h[1-6]' open/src/java.sql.rowset/share/classes/javax/sql/rowset/spi/SyncProvider.java  * 1.0 Naming Convention for Implementations  * 2.0 How a RowSet Object Gets Its Provider  * 3.0

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

In this case, going from what the execve(2) Linux man page says (and Mac seems to agree), if the script were executable it would have a '#!' interpreter string and the kernel would execute it anyway, would it not? What case would be solved by explicitly starting a shell? On Tue, Jun 4, 2019 at 2:1

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Thank you for clarifying, and for the review. Cheers, Thomas On Tue, Jun 4, 2019 at 9:14 PM Roger Riggs wrote: > Hi Thomas, > > If the argument is not an .exe, then it may be a command shell script > (.sh, etc.) > Only the system knows it is not an exe (errno == ENOEXEC), so it then > passes it

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi Thomas, If the argument is not an .exe, then it may be a command shell script  (.sh, etc.) Only the system knows it is not an exe (errno == ENOEXEC), so it then passes it as the first argument to /bin/sh that will handle the shell files. And yes, count me as a Reviewer and reviewed. Roger

Re: Review Request JDK-8221368: Error message when module main class cannot be loaded is missing exception details

Alan, Sundar, Thanks for the review. I further clean up the test and rename jdk.test.Main2 class to a new test2 module.  No change to the fix. http://cr.openjdk.java.net/~mchung/jdk13/webrevs/8221368/webrev.01/ Mandy On 6/4/19 12:10 AM, Alan Bateman wrote: On 04/06/2019 05:00, Mandy Chung

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 7:25 PM Martin Buchholz wrote: > > > On Tue, Jun 4, 2019 at 8:09 AM Roger Riggs wrote: > >> Hi Thomas, >> >> A minor concern is the impact of the extra write and read that can cause >> rescheduling >> of the parent and child processes. But that's probably in the noise >>

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 7:21 PM Martin Buchholz wrote: > Hi Thomas, > > Thanks - I agree with the approach. > > Unfortunately, it does make the subprocess implementation even > more complicated, since now "fail" is used to communicate success as well > as failure. Which probably results in some c

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

On Tue, Jun 4, 2019 at 8:09 AM Roger Riggs wrote: > Hi Thomas, > > A minor concern is the impact of the extra write and read that can cause > rescheduling > of the parent and child processes. But that's probably in the noise > compared to the > real work of exec. It would raise the complexity q

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi Thomas, Thanks - I agree with the approach. Unfortunately, it does make the subprocess implementation even more complicated, since now "fail" is used to communicate success as well as failure. Which probably results in some comments becoming stale. Can we come up with a better name for "fail

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi Roger, thanks for the feedback. Please find my answers inline. On Tue, Jun 4, 2019 at 5:09 PM Roger Riggs wrote: > Hi Thomas, > > A minor concern is the impact of the extra write and read that can cause > rescheduling > of the parent and child processes. But that's probably in the noise > c

Re: RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe

Thank you Roger! On Tue, Jun 4, 2019 at 5:13 PM Roger Riggs wrote: > H Thomas, > > Looks ok. > > Roger > > > On 06/04/2019 09:23 AM, Thomas Stüfe wrote: > > Hi all, > > > > may I please have reviews for this small fix: > > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8224181 > > cr: > > > h

Re: RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe

H Thomas, Looks ok. Roger On 06/04/2019 09:23 AM, Thomas Stüfe wrote: Hi all, may I please have reviews for this small fix: Bug: https://bugs.openjdk.java.net/browse/JDK-8224181 cr: http://cr.openjdk.java.net/~stuefe/webrevs/8224181-on-child-process-spawn--child-may-write-to-wrong-file-desc

Re: PING: RFR(s): (new approach) 8223777: In posix_spawn mode, failing to exec() jspawnhelper does not result in an error

Hi Thomas, A minor concern is the impact of the extra write and read that can cause rescheduling of the parent and child processes.  But that's probably in the noise compared to the real work of exec.  It would raise the complexity quite a bit/too much to code a single read in the parent that

RE: RFR 8213031: (zipfs) Add support for POSIX file permissions

Hi Alan, I made some updates to the CSR. The main update was that I transferred the new documentation part from module-info.java to the CSR's Specification section. Can you please review the CSR? Thanks Christoph From: Alan Bateman Sent: Montag, 3. Juni 2019 20:42 To: Langer, Christoph ; Lanc

RFR(xs): 8224181: On child process spawn, child may write to random file descriptor instead of the fail pipe

Hi all, may I please have reviews for this small fix: Bug: https://bugs.openjdk.java.net/browse/JDK-8224181 cr: http://cr.openjdk.java.net/~stuefe/webrevs/8224181-on-child-process-spawn--child-may-write-to-wrong-file-descriptor-instead-of-the-fail-pipe/webrev.00/webrev/ In sub process error hand

Re: RFR: 8224974: Implement JEP 352

On 03/06/2019 15:37, Andrew Dinn wrote: : The CSR and JEP have been updated accordingly CSR: https://bugs.openjdk.java.net/browse/JDK-8224975 JEP: https://bugs.openjdk.java.net/browse/JDK-8207851 The specification section in the CSR was missing the module definition so I added that. The res

RFR: 8225179: (regex) Minor Pattern cleanup

Hi, please review this j.u.regex.Pattern cleanup. - refactor BitClass to be a BmpCharPredicate (which allows removing two identical(!) lambdas), which improves startup and reduces allocations when compiling Patterns. - remove unused GroupRef class - made anonymous lookbehindEnd Node instance int

Re: RFR 8225198 : Optimize regex tree for greedy quantifiers of type {N, }

Hi Ivan, looks good to me. /Claes On 2019-06-03 21:54, Ivan Gerasimov wrote: Hello! When building a match-tree, the regex engine optimizes '*' and '+' greedy quantifiers by using special node of type CharPropertyGreedy. This later class can be used unmodified for other greedy quantifiers

Re: RFR: 6394787: Typos in javadoc of OIS.readObjectOverride and OOS.writeObjectOverride

+1 I can sponsor this for you Andrey. -Chris. > On 4 Jun 2019, at 07:56, Alan Bateman wrote: > > This looks right, I assume a cut&paste issue when the methods were originally > added. > > -Alan > > On 02/06/2019 12:23, Andrey Turbanov wrote: >> Hello. >> I would like to contribute a small

Re: Review Request JDK-8221368: Error message when module main class cannot be loaded is missing exception details

On 04/06/2019 05:00, Mandy Chung wrote: The launcher prints out the error message when the main class fails to initialize but java.launcher.module.error5 resource that intends to print the Caused-by exception message when running in a named module but prints an incorrect parameter. Very simple f

Re: Review Request JDK-8222448: java/lang/reflect/PublicMethods/PublicMethodsTest.java times out

On 04/06/2019 06:06, David Holmes wrote: Hi Mandy, Functional fix looks good, but layout and indentation appears off in the diff. I think the formatting got lost when it was pasted into a JIRA comment. Anyway, I think this is a good start. It would not surprise me if we eventually have to add