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-boun...@openjdk.java.net> On Behalf > Of Volker Simonis > Sent: Friday, December 7, 2018 9:05 AM > To: Steve Groeger <groe...@uk.ibm.com>; Java Core Libs <core-libs- > d...@openjdk.java.net> > Cc: ppc-aix-port-...@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 <groe...@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: groe...@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
