[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR 8062070: com/sun/jdi/DoubleAgentTest.java.DoubleAgentTest fails intermittently after 8056143
From:       Jaroslav Bachorik <jaroslav.bachorik () oracle ! com>
Date:       2014-10-27 18:00:27
Message-ID: 544E883B.4020703 () oracle ! com
[Download RAW message or body]

On 10/26/2014 11:53 PM, David Holmes wrote:
> On 25/10/2014 3:06 AM, Jaroslav Bachorik wrote:
>> Please, review this change to the test library.
>>
>> Issue : https://bugs.openjdk.java.net/browse/JDK-8062070
>> Webrev: http://cr.openjdk.java.net/~jbachorik/8062070/webrev.00
>>
>> The test started failing after the process.destroyForcibly() was added
>> to ProcessTools.executeProcess() to make sure there were no orphaned
>> processes once the test was finished. The destruction call was placed to
>> the finally block and was called for running or terminated processes
>> indiscriminately.
>>
>> It turns out that Process.get*Stream() methods will get confused when
>> calling Process.destroy/Forcibly/() on an already exited process - the
>> streams will get closed and any attempt to read or write to them will
>> end with a SocketException.
>>
>> The failure is timing related - when the stream manages to buffer data
>> before destroying the process from another thread the test passes.
>> Otherwise it just fails.
>>
>> The solution is to forcibly close the external process (started by
>> ProcessTools.executeProcess()) only in cases when eg. Process.waitFor()
>> throws an exception - moving it from the finally block to the catch
>> block.
>
> I can't help but think we are simply shifting the failure window around.
> But the proof of this is in the testing.

I don't know whether we can do any better while keeping the 
post-condition of having the process exited before leaving this method.

But with this change we would hit this problem in cases when the test 
would be already failing anyway.

I ran JPRT on all the available platforms and the test didn't fail once.

Thanks for the reviews.

-JB-
>
> Reviewed.
>
> David
>
>
>> Thanks,
>>
>> -JB-

[prev in list] [next in list] [prev in thread] [next in thread]