RFR 8056143: interrupted java/lang/management/MemoryMXBean/LowMemoryTest.java leaves running process

[Jaroslav Bachorik]

On 10/15/2014 02:10 AM, David Holmes wrote:
> On 14/10/2014 8:46 PM, Jaroslav Bachorik wrote:
>> Please, review the following test change
>>
>> Issue : https://bugs.openjdk.java.net/browse/JDK-8056143
>> Webrev: http://cr.openjdk.java.net/~jbachorik/8056143/webrev.00
>>
>> The method jdk.testlibrary.ProcessTools.getOutput(process) waits for the
>> given process to finish (process.waitFor()) before grabbing its outputs.
>> However, the code does not handle the process.waitFor() being
>> interrupted correctly - it just goes ahead and tries to obtain the exit
>> code which will fail and leave the tested process running.
>>
>> The correct way is to forcibly destroy the process when
>> process.waitFor() is interrupted or throws ExecutionException to make
>> sure the process has actually exited before checking its exit code.
>
> Why is this correct? What gives the thread calling getOutput the right
> to terminate the target process just because that thread was interrupted
> while waiting? If the interrupting thread intended the interrupt to mean
> "forcibly terminate the process and interrupt all threads waiting on it"
> then that thread should be doing the termination _not_ the one that was
> interrupted!

Process.waitFor() gets interrupted by a thread unknown to the actual 
test case - probably the JTreg timeout thread. The interrupting thread 
doesn't know that it is supposed to destroy a process. Once JTreg can 
take care of cleaning up process tree upon exit this code wouldn't be 
needed.

I was contemplating adding the check for "null" returned from 
ProcessTools.getOutput() and destroying the process inside the caller 
code - but this would have the same results as doing it in 
ProcessTools.getOutput() with the drawback of duplicating the same check 
everywhere ProcessTools.getOutput() would be used.

A silent postcondition of ProcessTools.getOuptut() is that the target 
process has finished - and it holds for all the code paths except the 
InterruptedException handler.

-JB-



>
> David
>
>> Thanks,
>>
>> -JB-