JDK-8211844 [aix] ProcessBuilder: Piping between created processes does not work.
Hi all, I have been investigating the issue https://bugs.openjdk.java.net/browse/JDK-8211844 raised by Goetz Lindenmaier which is related to the jdk/java/lang/ProcessBuilder/PipelineTest.java JTReg test failing on AIX. Having done some investigation I have a potential fix fore the issue: diff -r 9501a7b59111 src/java.base/unix/classes/java/lang/ProcessImpl.java --- a/src/java.base/unix/classes/java/lang/ProcessImpl.java Mon Dec 03 14:28:19 2018 +0300 +++ b/src/java.base/unix/classes/java/lang/ProcessImpl.java Thu Dec 06 15:01:03 2018 +0000 @@ -446,7 +446,7 @@ ProcessBuilder.NullOutputStream.INSTANCE : new ProcessPipeOutputStream(fds[0]); - stdout = (fds[1] == -1) ? + stdout = (fds[1] == -1 || forceNullOutputStream) ? ProcessBuilder.NullInputStream.INSTANCE : new DeferredCloseProcessPipeInputStream(fds[1]); I would appreciate any feedback please, and for someone to be a sponsor for this and to contributute it to OpenJDK. Steve Groeger IBM Runtime Technologies Hursley, Winchester Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129 Fax (44) 1962 816800 Lotus Notes: Steve Groeger/UK/IBM Internet: groeges@uk.ibm.com Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Hi Steve, thanks a lot for this fix. I'm forwarding this to core-libs-dev as well, because that's where the review has to take place. The "ppc-aix-port-dev" list is mostly a marker for the port maintainers to get their attention on relevant changes (so cross-posting is fine in this case :) On Thu, Dec 6, 2018 at 4:26 PM Steve Groeger <GROEGES@uk.ibm.com> wrote:
Hi all,
I have been investigating the issue https://bugs.openjdk.java.net/browse/JDK-8211844 raised by Goetz Lindenmaier which is related to the jdk/java/lang/ProcessBuilder/PipelineTest.java JTReg test failing on AIX. Having done some investigation I have a potential fix fore the issue:
diff -r 9501a7b59111 src/java.base/unix/classes/java/lang/ProcessImpl.java --- a/src/java.base/unix/classes/java/lang/ProcessImpl.java Mon Dec 03 14:28:19 2018 +0300 +++ b/src/java.base/unix/classes/java/lang/ProcessImpl.java Thu Dec 06 15:01:03 2018 +0000 @@ -446,7 +446,7 @@ ProcessBuilder.NullOutputStream.INSTANCE : new ProcessPipeOutputStream(fds[0]);
- stdout = (fds[1] == -1) ? + stdout = (fds[1] == -1 || forceNullOutputStream) ? ProcessBuilder.NullInputStream.INSTANCE : new DeferredCloseProcessPipeInputStream(fds[1]);
Your change looks good and I can sponsor it. Just as a hint for other reviewers I'd like to mention that this change, albeit in a shared Java file, is still AIX-only because it is in the "AIX" case of a switch statement. @Steve: can you please verify, that your change introduces no regression by running the complete jtreg test suite. Thank you and best regards, Volker
I would appreciate any feedback please, and for someone to be a sponsor for this and to contributute it to OpenJDK.
Steve Groeger IBM Runtime Technologies Hursley, Winchester Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129 Fax (44) 1962 816800 Lotus Notes: Steve Groeger/UK/IBM Internet: groeges@uk.ibm.com
Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Hi, To me, the change looks good, too. I just wondered whether solaris needs to be fixed, too. But I don’t see the test failing on solaris. The bug needs to be removed from the ProblemList in the change, though. See also http://cr.openjdk.java.net/~goetz/wr18/sgroeger/8211844-aix_pipe_proc/01/ I would sponsor this issue. Best regards, Goetz.
-----Original Message----- From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> On Behalf Of Volker Simonis Sent: Friday, December 7, 2018 9:05 AM To: Steve Groeger <GROEGES@uk.ibm.com>; Java Core Libs <core-libs- dev@openjdk.java.net> Cc: ppc-aix-port-dev@openjdk.java.net Subject: Re: JDK-8211844 [aix] ProcessBuilder: Piping between created processes does not work.
Hi Steve,
thanks a lot for this fix. I'm forwarding this to core-libs-dev as well, because that's where the review has to take place. The "ppc-aix-port-dev" list is mostly a marker for the port maintainers to get their attention on relevant changes (so cross-posting is fine in this case :)
On Thu, Dec 6, 2018 at 4:26 PM Steve Groeger <GROEGES@uk.ibm.com> wrote:
Hi all,
I have been investigating the issue
https://bugs.openjdk.java.net/browse/JDK-8211844 raised by Goetz Lindenmaier which is related to the
jdk/java/lang/ProcessBuilder/PipelineTest.java JTReg test failing on AIX. Having done some investigation I have a potential fix fore the issue:
diff -r 9501a7b59111 src/java.base/unix/classes/java/lang/ProcessImpl.java --- a/src/java.base/unix/classes/java/lang/ProcessImpl.java Mon Dec 03 14:28:19 2018 +0300 +++ b/src/java.base/unix/classes/java/lang/ProcessImpl.java Thu Dec 06 15:01:03 2018 +0000 @@ -446,7 +446,7 @@ ProcessBuilder.NullOutputStream.INSTANCE : new ProcessPipeOutputStream(fds[0]);
- stdout = (fds[1] == -1) ? + stdout = (fds[1] == -1 || forceNullOutputStream) ? ProcessBuilder.NullInputStream.INSTANCE : new DeferredCloseProcessPipeInputStream(fds[1]);
Your change looks good and I can sponsor it. Just as a hint for other reviewers I'd like to mention that this change, albeit in a shared Java file, is still AIX-only because it is in the "AIX" case of a switch statement.
@Steve: can you please verify, that your change introduces no regression by running the complete jtreg test suite.
Thank you and best regards, Volker
I would appreciate any feedback please, and for someone to be a sponsor for this and to contributute it to OpenJDK.
Steve Groeger IBM Runtime Technologies Hursley, Winchester Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129 Fax (44) 1962 816800 Lotus Notes: Steve Groeger/UK/IBM Internet: groeges@uk.ibm.com
Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Hi Steve, Thanks for this fix! I made a webrev for it, and also would sponsor it: http://cr.openjdk.java.net/~goetz/wr18/sgroeger/8211844-aix_pipe_proc/01/ I also added a patch to remove the test from the problem list. The ProblemList is used to exclude tests known to be failing. I think this needs to be reviewed on hotspot-runtime-dev. But I'm not sure, maybe core-libs-dev is the better list. If you post there, point me to it (in case I miss it) and I'll send a formal review. I also put it into our testing. Should this be fixed for Solaris, too? I didn't see the problem there, though. Best regards, Goetz. From: Steve Groeger <GROEGES@uk.ibm.com> Sent: Thursday, December 6, 2018 4:26 PM To: ppc-aix-port-dev <ppc-aix-port-dev@openjdk.java.net> Cc: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Subject: JDK-8211844 [aix] ProcessBuilder: Piping between created processes does not work. Hi all, I have been investigating the issue https://bugs.openjdk.java.net/browse/JDK-8211844 raised by Goetz Lindenmaier which is related to the jdk/java/lang/ProcessBuilder/PipelineTest.java JTReg test failing on AIX. Having done some investigation I have a potential fix fore the issue: diff -r 9501a7b59111 src/java.base/unix/classes/java/lang/ProcessImpl.java --- a/src/java.base/unix/classes/java/lang/ProcessImpl.java Mon Dec 03 14:28:19 2018 +0300 +++ b/src/java.base/unix/classes/java/lang/ProcessImpl.java Thu Dec 06 15:01:03 2018 +0000 @@ -446,7 +446,7 @@ ProcessBuilder.NullOutputStream.INSTANCE : new ProcessPipeOutputStream(fds[0]); - stdout = (fds[1] == -1) ? + stdout = (fds[1] == -1 || forceNullOutputStream) ? ProcessBuilder.NullInputStream.INSTANCE : new DeferredCloseProcessPipeInputStream(fds[1]); I would appreciate any feedback please, and for someone to be a sponsor for this and to contributute it to OpenJDK. Steve Groeger IBM Runtime Technologies Hursley, Winchester Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129 Fax (44) 1962 816800 Lotus Notes: Steve Groeger/UK/IBM Internet: groeges@uk.ibm.com<mailto:groeges@uk.ibm.com> Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
Hi Goetz, Thanks for creating the webrev. If there are reviewers that can look at your webrev that would be great. Volker had posted this onto core-libs-dev here: http://mail.openjdk.java.net/pipermail/core-libs-dev/2018-December/057184.ht... I am currently running the full set of JTReg tests, as per Volker's request, and will post the results when complete. Thanks Steve Groeger IBM Runtime Technologies Hursley, Winchester Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129 Fax (44) 1962 816800 Lotus Notes: Steve Groeger/UK/IBM Internet: groeges@uk.ibm.com Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU From: "Lindenmaier, Goetz" <goetz.lindenmaier@sap.com> To: 'Steve Groeger' <GROEGES@uk.ibm.com>, ppc-aix-port-dev <ppc-aix-port-dev@openjdk.java.net> Date: 07/12/2018 13:51 Subject: RE: JDK-8211844 [aix] ProcessBuilder: Piping between created processes does not work. Hi Steve, Thanks for this fix! I made a webrev for it, and also would sponsor it: http://cr.openjdk.java.net/~goetz/wr18/sgroeger/8211844-aix_pipe_proc/01/ I also added a patch to remove the test from the problem list. The ProblemList is used to exclude tests known to be failing. I think this needs to be reviewed on hotspot-runtime-dev. But I’m not sure, maybe core-libs-dev is the better list. If you post there, point me to it (in case I miss it) and I’ll send a formal review. I also put it into our testing. Should this be fixed for Solaris, too? I didn’t see the problem there, though. Best regards, Goetz. From: Steve Groeger <GROEGES@uk.ibm.com> Sent: Thursday, December 6, 2018 4:26 PM To: ppc-aix-port-dev <ppc-aix-port-dev@openjdk.java.net> Cc: Lindenmaier, Goetz <goetz.lindenmaier@sap.com> Subject: JDK-8211844 [aix] ProcessBuilder: Piping between created processes does not work. Hi all, I have been investigating the issue https://bugs.openjdk.java.net/browse/JDK-8211844 raised by Goetz Lindenmaier which is related to the jdk/java/lang/ProcessBuilder/PipelineTest.java JTReg test failing on AIX. Having done some investigation I have a potential fix fore the issue: diff -r 9501a7b59111 src/java.base/unix/classes/java/lang/ProcessImpl.java --- a/src/java.base/unix/classes/java/lang/ProcessImpl.java Mon Dec 03 14:28:19 2018 +0300 +++ b/src/java.base/unix/classes/java/lang/ProcessImpl.java Thu Dec 06 15:01:03 2018 +0000 @@ -446,7 +446,7 @@ ProcessBuilder.NullOutputStream.INSTANCE : new ProcessPipeOutputStream(fds[0]); - stdout = (fds[1] == -1) ? + stdout = (fds[1] == -1 || forceNullOutputStream) ? ProcessBuilder.NullInputStream.INSTANCE : new DeferredCloseProcessPipeInputStream(fds[1]); I would appreciate any feedback please, and for someone to be a sponsor for this and to contributute it to OpenJDK. Steve Groeger IBM Runtime Technologies Hursley, Winchester Tel: (44) 1962 816911 Mobex: 279990 Mobile: 07718 517 129 Fax (44) 1962 816800 Lotus Notes: Steve Groeger/UK/IBM Internet: groeges@uk.ibm.com Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU Unless stated otherwise above: IBM United Kingdom Limited - Registered in England and Wales with number 741598. Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
participants (3)
-
Lindenmaier, Goetz
-
Steve Groeger
-
Volker Simonis