RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails intermittently

[shanliang]

Jaroslav Bachorik wrote:
> Hi Shanliang,
>
> On 06/26/2014 03:15 PM, shanliang wrote:
>> Hi,
>>
>> Today ProcessTools.executeProcess has the code:
>>      new OutputAnalyzer(pb.start());
>>
>> and OutputAnalyzer constructor calls immediately:
>>      exitValue = process.exitValue();
>>
>> the test got exception because the process did not end.
>
> Are you sure about this?
>
> The OutputAnalyzer constructor, before calling process.exitValue(), 
> calls ProcessTools.getOutput(process) which actually does 
> process.waitFor()
Good catch!
>
> A probable explanation would be that process.waitFor() gets 
> interrupted without the target process actually ending.
>
> Either the result of ProcessTools.getOutput() should be checked for 
> null to detect this situation or ProcessTools.getOutput() should throw 
> a more aggressive exception when waitFor() gets interrupted.
It was possible beacause of an InterruptedException, but maybe a Process 
issue too.

process.waitFor() was called by the test main thread, I am wondering who 
wanted to interrupt this thread?

Not know why ProcessTools.getOutput() catches InterruptedException and 
then returns null, are there some other tests dependent to this 
behavior? otherwise better to not catch InterruptedException.

I think to keep this modification, it will allow us to get more 
information if the bug is reproduced, if getting an InterruptedException 
then we need to do more investigation for why, if no exception then we 
may rise a question to process.waitFor().

Shanliang
>
> -JB-
>
>>
>> So a direct solution for the test is not to call:
>>         ProcessTools.executeTestJvm(args);
>>
>> but:
>>          ProcessBuilder pb =
>> ProcessTools.createJavaProcessBuilder(Utils.addTestJavaOpts(args));
>>          Process process = pb.start();
>>          process.waitFor();
>>          OutputAnalyzer output = new OutputAnalyzer(process);
>>
>> here we do waiting:
>>          process.waitFor();
>> before constructing an OutputAnalyzer.
>>
>> ProcessTools is a tool class we may have same issue for other tests
>> using this class. So we may need to improve the test library.
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8031554
>> webrev: http://cr.openjdk.java.net/~sjiang/JDK-8031554/00/
>>
>>
>> Thanks,
>> Shanliang
>>
>