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

[Christian Tornqvist]

I can't remember if there was a reason for doing it like this, but it seems
reasonable to not catch the InterruptedException in getOutput().

 

Thanks,

Christian

 

From: Staffan Larsen [mailto:staffan.larsen at oracle.com] 
Sent: Friday, June 27, 2014 4:49 AM
To: shanliang
Cc: Jaroslav Bachorik; serviceability-dev at openjdk.java.net
serviceability-dev at openjdk.java.net; Christian Tornqvist
Subject: Re: RFR JDK-8031554: com/sun/tools/attach/BasicTests.java fails
intermittently

 

It does look suspicious to catch and ignore the InterruptedException,
especially since the OutputAnalyzer constructor will fail in this case. 

 

Christian is the original author of these classes: do you know if there is a
good rationale for doing this? Or should we propagate the
InterruptedException?

 

Thanks,

/Staffan

 

On 26 jun 2014, at 17:24, shanliang <shanliang.jiang at oracle.com
<mailto:shanliang.jiang at oracle.com> > wrote:





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

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20140627/8e36787b/attachment-0001.html>