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. > >> >> 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- >> >
