JDK-8211844 [aix] ProcessBuilder: Piping between created processes does not work.

[Roger Riggs]

Hi,

I'd be interested in how PipelineTest failed on AIX?

It looks like PipelineTest is flawed in that verifyProcesses() does not 
check
all of the 1..n Processes getInputStreams for being a null stream, only 
the nth Process.
And the test is incorrectly applied to the last Process, which returns a 
null
stream not because of the pipeline but because the output is redirected 
to a file.

Thanks, Roger


On 12/07/2018 09:52 AM, Roger Riggs wrote:
> Hi,
>
> I notice that the Solaris case also does not include "|| 
> forceNullOutputStream".
> I'll have to investigate why the Pipeline test didn't fail on Solaris.
>
> Please add that to the patch and I'll run it through our tests.
>
> Thanks, Roger
>
> On 12/07/2018 03:05 AM, Volker Simonis wrote:
>> 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 at 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 at 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
>