On 1/07/2014 6:54 PM, Staffan Larsen wrote:

On 1 jul 2014, at 10:29, Jaroslav Bachorik <[email protected]> wrote:

On 07/01/2014 08:17 AM, Staffan Larsen wrote:
Jaroslav,

Great cleanup!

How about using Process.destroyForcibly() instead of sending the “shutdown” 
message? Maybe not as “nice”, but much less code.

The target process needs to hang around for a while till the test tries to shut 
it down. We would need to put a monitor wait there and rely on the OS being 
able to shut the process down.

Not sure what you mean. Why can’t we terminate the process at the same place we 
call RunnerUtil.stopApplication()?

The Process.destroyForcibly() spec leaves it to the OS specific implementations 
to do the right thing. For the major OSes it seems to force kill the process 
but I'm not sure about eg. embedded devices.

I think the idea of Process.destroyForcibly() is that we /can/ rely on it. If 
we can’t rely on our own implementation, then the API is not very usable.

Right. It is the OS that is key not the device. Linux/Solaris will send a SIGKILL. Windows does terminateProcess for both destroy and destroyForcibly.

David
-----




test/sun/tools/jstatd/JstatdTest.java:
  323                     port = Integer.toString(31111); 
//Utils.getFreePort());
  Looks like a mistake?

Definitely a mistake :( Thanks for spotting it!

Updated webrev: http://cr.openjdk.java.net/~jbachorik/8048193/webrev.01

Looks good!

Thanks,
/Staffan



-JB-



/Staffan


On 30 jun 2014, at 18:43, Jaroslav Bachorik <[email protected]> 
wrote:

Please, review the following test change.

Issue : https://bugs.openjdk.java.net/browse/JDK-8048193
Webrev: http://cr.openjdk.java.net/~jbachorik/8048193/webrev.00

Intricate log parsing in order to get an application PID is replaced with the 
new Process.getPid() API call. While doing this cleanup it also become obvious 
that it was unnecessary to start a socket server for each launched test 
application just in order to shut it down when the same functionality can be 
achieved through the usage of stdin/stdout provided by the Process instance.

Thanks,

-JB-



Reply via email to